Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gulp does not wait for gulpfile.js to complete when SIGTERM is sent to gulp-cli #222

Open
kozak-codes opened this issue Oct 7, 2020 · 6 comments

Comments

@kozak-codes
Copy link

What were you expecting to happen?

When I click CTRL+C, a sigterm is sent to gulp. I have created a watcher which spawns a child_process when anything under src/** is triggered. to ensure that the instance has fully exited before we restart a new instance. My instance has some async cleanup (~50 ms). I've added an exitHandle for SIGINT and SIGTERM handler to my gulpfile.js to prevent the process from exiting immediately. When this completes properly, there are some console logs, and then the user should be given the terminal after a short delay.

What actually happened?

Gulpfile completes, but gulp-cli has exited. This results in losing the current process losing the terminal input. (eg KleptoKat@Computer:~/app$) with the console outputs after this line. It requires an extra enter keypress which is not a huge deal but it's a little annoying.

Please give us a sample of your gulpfile

Here is a smaller gulpfile:

const gulp = require('gulp');
const { spawn } = require('child_process');
const { EventEmitter } = require('events');

let instance;
let instanceRunning = false;
let instanceChanged = new EventEmitter();
let watchCb;

function startAll(cb) {
	instance = spawn('node', [ '-e', '"setTimeout(() => {}, 20000)"'], {
		stdio: [undefined, process.stdout, process.stderr],
		env: process.env,
		detached: true,
	});
	instanceRunning = true;
	instanceChanged.emit('started');

	instance.addListener('exit', (code) => {
		console.log(`Instance exited with code ${ code }`);
		instanceRunning = false;
		instance = undefined;
		instanceChanged.emit('stopped');

		if (code === 10) {
			startAll();
		}
	})

	if (cb) { cb() };
}

function kill(cb) {
	if (instance) {
		console.log(`Killing instance...`);
		const onStopped = () => {
			instanceChanged.removeListener('stopped', onStopped);
			cb();
		}

		instanceChanged.addListener('stopped', onStopped);

		instance.kill();
	} else {
		cb();
	}
}

function watch(cb) {
	// TODO: restart individual services w/hot reload  :O
	gulp.watch('src/**/*', gulp.series(
		'build',
		'kill',
		'startAll'
	));

	watchCb = cb;
}

gulp.task('kill', kill);
gulp.task('startAll', startAll);
gulp.task('build', buildSrc);
gulp.task('watch', watch);
gulp.task('default', gulp.series(
	'build',
	'startAll',
	'watch',
	'kill'
));



function exitHandle(signal) {
	console.log('Exit handle', signal. instanceRunning);

	if (watchCb) {
		watchCb();
	} else {
		process.exit(0);
	}
	setTimeout(() => {}, 50000);

	if (instance) {
		instance.kill();
	} else {
		
	}
}

process.on('SIGINT', exitHandle);
process.on('SIGTERM', exitHandle);
process.on('SIGHUP', exitHandle);

Terminal output / screenshots

When CTRL C is entered:

INFO  SERVER: Gateway started on port 1337
^CExit handle undefined
[18:09:20] Finished 'watch' after 2 min
[18:09:20] Starting 'kill'...
Killing instance...
INFO  BrokerManager: Stopping

KleptoKat@Computer:~/app$ INFO  broker: ServiceBroker is stopped. Good bye.
INFO  BrokerManager: stopped
Instance exited with code 0
[18:09:20] Finished 'kill' after 26 ms
[18:09:20] Finished 'default' after 2.1 min
            [ ENTER KEY IS PRESSED HERE ]
KleptoKat@Computer:~/app$

Please provide the following information:

  • OS & version [e.g. MacOS Catalina 10.15.4]: Ubuntu 20
  • node version (run node -v): v10.16.3
  • npm version (run npm -v): 6.9.0
  • gulp version (run gulp -v): CLI 2.2, local 4.0.2

I can help provide a more replicable environment if need be. It's just slightly annoying. A possible fix may be to add an argument, like --waitForGulpfileToExit which will enable this kind of waiting behaviour.

@phated
Copy link
Member

phated commented Oct 14, 2020

@sttk Can you help here? I'm not sure what is being requested.

@sttk
Copy link
Contributor

sttk commented Oct 15, 2020

Yes, I'll address this weekend.

@sttk
Copy link
Contributor

sttk commented Oct 18, 2020

@KleptoKat I suppose your aim in this issue is to make enable to run post-tasks of a watch task and finish them normally by gulp-cli when CTRL+C is entered.

And I changed your code as follows. Does this fit your aim?

const gulp = require('gulp')                                                    
let watchCb;
function startAll(cb) {
  ... Spawn `instance`
  cb();
}
async function kill(cb) {
  await ... Clean up `instance`
  cb();
}
function watch(cb) {
  gulp.watch('src/**', gulp.series(kill, startAll));
  watchCb = cb;
}
gulp.task('default', gulp.series(startAll, watch, kill, end));
function end(cb) {
  cb();
  process.exit(0);
}
function exitHandle() {
  if (watchCb) watchCb();
}
process.on('SIGHUP', exitHandle);
process.on('SIGINT', exitHandle);
process.on('SIGTERM', exitHandle);

@sttk
Copy link
Contributor

sttk commented Oct 18, 2020

@phated Should we implement the solution for this issue? The solution would be like the following:

const gulp = require('gulp')

// 1) Override the current `watch` function
const origWatch = gulp.watch;
gulp.watch = function(globs, task, endCb) {

  if (endCb) {
    function exitHandle() {
      if (endCb) endCb();
    }
    process.on('SIGHUP', exitHandle);
    process.on('SIGINT', exitHandle);
    process.on('SIGTERM', exitHandle);
    gulp.__DOEXIT = true;  // 2) do `exit(0)` in gulp-cli/lib/versioned/^4.0.0/index.js
  }

  return origWatch(globs, task);
}

function startAll(cb) {
  ... Spawn `instance`
  cb();
}
async function kill(cb) {
  await ... Clean up `instance`
  cb();
}
function watch(cb) {
  gulp.watch('src/**', gulp.series(kill, startAll), cb);
}
gulp.task('default', gulp.series(startAll, watch, kill));

@phated
Copy link
Member

phated commented Oct 19, 2020

@sttk I think making the change you suggest is out of scope. People have all the flexibility of node at their disposal and can even hook into the watcher instance if they need to (we return Chokidar from watch), so I don't think I want to make that change.

@sttk
Copy link
Contributor

sttk commented Oct 19, 2020

@phated Got it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants