open-cli-tools / concurrently

Run commands concurrently. Like `npm run watch-js & npm run watch-less` but better.
https://www.npmjs.com/package/concurrently
MIT License
7.13k stars 233 forks source link

Multiple processes exiting with code 0 or SIGTERM produce a non-zero exit code #187

Open jcollum-nike opened 5 years ago

jcollum-nike commented 5 years ago

4.1.0

This is my output. All the processes are exiting with 0 or SIGTERM. But the final exit code is non-zero.

[test  ] npm run test:docker:sleep exited with code 0
--> Sending SIGTERM to other processes..
[server] [mock   ] npm run start:mockserver:nobunyan exited with code SIGTERM
[server] [nes    ] DOCKER=1 nessie start exited with code 0
[server] [webpack] npm run webpack:server exited with code 0
[server] npm ERR! code ELIFECYCLE

Here are my npm scripts:

  "start:docker": "concurrently --n  'nes    ,mock   ,webpack' -s 'first' 'DOCKER=1 nessie start' 'npm run start:mockserver:nobunyan' 'npm run webpack:server'",

  "test:docker:sleep":  "sleep 10 && npm run --prefix functional-tests test:docker",
  "test:functional:docker:ra": "npm-run-all --parallel -lr 'test:docker:sleep' 'start:docker'",
  "test:functional:docker": "concurrently -n 'test  ,server' -s 'first' --kill-others 'npm run test:docker:sleep' 'npm run start:docker' ",

test:functional:docker:ra runs the same commands but exits with 0 (the desired output).

I need this exit code for my build pipeline.

JasonGhent commented 4 years ago

Seconding that this is still an issue. saw this manifest more consistently on linux than macos.

✗ npx concurrently -k 'node -e "process.on(\"SIGTERM\", (...args) => { console.log(\"args\", args); process.exit(0) }); setInterval(()=>{});"' 'exit 0'; echo $?

[1] exit 0 exited with code 0
--> Sending SIGTERM to other processes..
[0] node -e "process.on(\"SIGTERM\", (...args) => { console.log(\"args\", args); process.exit(0) }); setInterval(()=>{});" exited with code SIGTERM

1

It may be some internal race condition? I saw this output a single time when trying all of this. Note the 0 exit code for the same command on the same machine:

✗ npx concurrently -k 'node -e "process.on(\"SIGTERM\", (...args) => { console.log(\"args\", args); process.exit(0) }); setInterval(()=>{});"' 'exit 0'
[1] exit 0 exited with code 0
--> Sending SIGTERM to other processes..
[0] args [ 'SIGTERM', 15 ]
[0] node -e "process.on(\"SIGTERM\", (...args) => { console.log(\"args\", args); process.exit(0) }); setInterval(()=>{});" exited with code 0
mscottnelson commented 3 years ago

I found @JasonGhent's example helpful as a starting point to explore, but ultimately a touch confusing (at least to me) since it is catching SIGTERM, and thus not allowing the child process respond to the command signal. I recommend anyone running into this double check their own signal handlers. If you are registering a SIGTERM, then your node process will defer to that handler.

You can see the behavior exhibited this way: concurrently -k 'exit 0' 'sleep 5'; echo $?

Based on the way the concurrently tests are written, at least, I would expect this to exit with 0, but it exits with 1. However, at least in my own testing, after controlling for application error handling, it does at least behave consistently.

I believe what's happening, although I haven't verified, is that the child process's own error handler responds to the SIGTERM (and exits with 1) before the previous error handler (which exited with 0) finishes registering this as a successful (0) kill. Please let me know if an MR might be considered for this behavior or not.

coolvasanth commented 3 years ago

I'm also facing the same problem, any solution to fix this ?

taxilian commented 3 years ago

This is very frustrating as it means I can't use --kill-others in any reasonable way without making it erroneously think there is an error.

Th3S4mur41 commented 2 years ago

Seing the same issue when trying to kill vite preview after hint ended with code 0

concurrently -k -s first 'npm run start:prod' 'npx hint http://localhost:3000/'

vite_preview_hint_exit-1

abjrcode commented 2 years ago

I am having the same issue, a typical ExpressJS server setup and I have a handler for SIGTERM. I even tried to just process.exit(0) in the SIGTERM handler without avail. I keep getting process exited with code SIGTERM. Trying to kill the process with kill $PID or with any other signal and I see a successful exit code of 0 in the terminal.

xenoterracide commented 2 years ago

yep, I think this is what I came here to report, anyone have a workaround?

xenoterracide commented 2 years ago

I guess putting this at the end of my test express app is sufficient in my case

process.once('SIGTERM', () => process.exit(0))
gustavohenke commented 2 years ago

Please remember to add the --success flag folks 🙂 default behaviour is to exit with success when all commands have also exited with success.

Perhaps --kill-others should integrate with --success tightly, but that'd probably be a breaking change.

Keen to hear the thoughts of y'all.

mscottnelson commented 2 years ago

Thanks for this. I see now that --kill-others --success first probably would have covered my issue at the time. I think it would be worthwhile to either integrate --kill-others with --success or maybe explicitly document the interaction between these, with a clear reference from --kill-others short description to --success first. I think my expectation, that a programming ending successfully ends everything with success when --kill-others is flagged, is a natural one, but that having --kill-others change the default of a separate flag would also be surprising.

Th3S4mur41 commented 2 years ago

Although this would be a breaking change. I would definitely expect the exit code to be the one from the first process with --kill-others In that case I most likely don't care about the other processes' exit code since I decide to kill them before they are done.

The most prominent use case is killing a (web) server when the tests you wanted to run on it are done. The exit code of the tests is definitely relevant, but the one from the server is not.

FreeYourMindToProgress commented 2 years ago

I agree with you @Th3S4mur41 but I don't know how to get the result of the first script. I'm executing a script in jenkins pipeline

script{ bat "npm run concurrently cypress run-- \"--kill-others\" \"ng serve \" \"--success first\"" }

and this generates :

[0] cypress run exited with code 2 --> Sending SIGTERM to other processes.. [1] ng serve exited with code 1

FreeYourMindToProgress commented 2 years ago
process.once('SIGTERM', () => process.exit(0))

is ng serve exited in this case?

yarinsa commented 1 year ago

Any solution?

mscottnelson commented 1 year ago

@yarinsa Many cases are covered by some combination of --kill-others, --success, and --kill-signal (which must be used with --kill-others or --kill-others-on-fail).

@gustavohenke It would be nice to be able to exit concurrently with the exit code of the sequentially-first exiting process, whatever it is: "If any of command A, command B, or command C exits with $EXITCODE, kill-others and exit with $EXITCODE." That is, "race these commands, and if the first to finish succeeds, exit 0, but if it fails, exit with its exit code (1 could also probably work for most cases)." I don't think there's a way to cover that scenario currently; or at least, I couldn't figure out how to do it. What you can do is say, "if all of one or more particular commands succeed, then exit with 0, but if any command fails, exit with 1."