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 4 returns with exit code 0 even if there are unfinished tasks #2081

Closed
barbogast opened this issue Jan 3, 2018 · 6 comments
Closed

Comments

@barbogast
Copy link

What were you expecting to happen?
Gulp 4 returns a non-zero exit code if tasks are unfinished

What actually happened?
Gulp 4 returned exit code zero

Please post a sample of your gulpfile (preferably reduced to just the bit that's not working)

const gulp = require('gulp')

gulp.task('aaa', () => {
  
})

When running the task gulp correctly reports that the task has not completed. But the return code of gulp seems to be 0. I would prefer it to be 1.

~/website$ node_modules/.bin/gulp aaa
[10:54:26] Using gulpfile ~/website/gulpfile.js
[10:54:26] Starting 'aaa'...
[10:54:26] The following tasks did not complete: aaa
[10:54:26] Did you forget to signal async completion?
~/website$ echo $?
0

What version of gulp are you using?

~/website$ gulp -v
[10:53:13] CLI version 1.4.0
[10:53:13] Local version 4.0.0-alpha.2

What versions of npm and node are you using?

~/website$ node -v
v8.9.3
~/website$ npm -v
5.5.1
@aduh95
Copy link

aduh95 commented Jan 3, 2018

For the record, I tested with the latest version, and got the same result:

$ gulp -v                                                                                                                                               
[16:56:02] CLI version 2.0.0
[16:56:02] Local version 4.0.0
$ gulp aaa
[16:59:25] Using gulpfile .../gulpfile.js
[16:59:25] Starting 'aaa'...
[16:59:25] The following tasks did not complete: aaa
[16:59:25] Did you forget to signal async completion?
$ echo $?
0

@phated
Copy link
Member

phated commented Jan 4, 2018

@barbogast I like this idea (and I think I might have tried to do it before) but the way node handles process.on('exit') (used at https://github.com/gulpjs/gulp-cli/blob/master/lib/versioned/%5E4.0.0/log/sync-task.js#L38) doesn't allow you to specify a new error code (see https://nodejs.org/api/process.html#process_event_exit) AFAIK. If you want to submit a PR to gulp-cli with something that makes this work, I'll take a look.

/cc @sttk in case he knows something more than me.

@phated phated closed this as completed Jan 4, 2018
@sttk
Copy link
Contributor

sttk commented Jan 8, 2018

@phated Since node v0.11, exit code can be specified by setting to process.exitCode.

In v0.10, exit code can be specified by using process.exit(code), but there is a bad point that 'exit' event handlers registered after warn() in sync-task.js are not executed.

Should we do this?

@phated
Copy link
Member

phated commented Jan 8, 2018

@sttk maybe we should only support this with process.exitCode - this will mean that the exit code will be incorrect on node 0.10 (but that's been our default behavior so far)

@sttk
Copy link
Contributor

sttk commented Jan 9, 2018

@phated I got it. I'll send a pr about this after I research exit code when gulp-cli is respawned.

@barbogast
Copy link
Author

I just tried setting process.exitCode and it seemed to work for me. See the PR above.

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

4 participants