Skip to content
This repository has been archived by the owner on Jan 6, 2021. It is now read-only.

cross-env should exit with non-zero code when terminated by Ctrl-C (SIGINT) #228

Closed
jsnajdr opened this issue Feb 18, 2020 · 5 comments
Closed

Comments

@jsnajdr
Copy link

jsnajdr commented Feb 18, 2020

Steps to reproduce:
Run a shell command like this:

./node_modules/.bin/cross-env cat && echo success

The cat process is waiting for stdin and therefore keeps running. Now terminate the command with Ctrl-C.

Expected result:
The cross-env cat command didn't succeed -- it was terminated before it has a chance to finish. Therefore, the echo success command shouldn't run and shouldn't write success to console.

Actual result:
The cross-env's child process exits because it was terminated by a signal, and cross-env then exits itself with error code 0. That means success and the echo success command gets to run.

Proposed solution:
Basically, #181 by @jnielson94 should be reverted. On SIGINT, the exit code should be 128 + signal_number, i.e., 130. Shell will do such a translation, as documented for example here. And it can be verified by a simple experiment in your shell:

$ cat
<terminate with ctrl-c>
$ echo $?
130

I'm not sure I understand what exactly is reported as wrong in @jnielson94's #180 report. That the script errors when terminated with Ctrl-C and that npm run logs an error? However, I believe that's actually the correct behavior 🙂 It's what makes shell conditional chains (cmd1 && cmd2, cmd3 || cmd4) work.

@kentcdodds
Copy link
Owner

Does this PR fix this problem? #227

@jsnajdr
Copy link
Author

jsnajdr commented Feb 20, 2020

Does this PR fix this problem? #227

Yes, it should fix this problem. The code in #227 works when SIGINT is triggered by pressing Ctrl-C in shell, where the shell sends the signal to both the parent and child processes. Then the parent exits with "I died by the SIGINT signal" exit status, instead of exit code 0. Problem solved.

However, when only the parent process receives the signal (e.g., after kill -s SIGINT <parent_pid>), the signal is ignored and nothing terminates. That's suspicious.

I commented about this on the PR, too.

@baerrach
Copy link
Contributor

On Windows, using Git for Windows bash:

$ node src/bin/cross-env.js cat && echo success
# In another Git for Windows bash terminal, `ps -ef` then `kill -s SIGINT <cat pid>`
success

$ echo $?
0

It depends on what cat does when receiving a SIGINT as to what cross-env will do.

@baerrach
Copy link
Contributor

baerrach commented Feb 20, 2020

I wasn't able to work out how to emulate https://unix.stackexchange.com/questions/149741/why-is-sigint-not-propagated-to-child-process-when-sent-to-its-parent-process on windows.

The ppids aren't making sense to me.

I've aliased node to alias node='winpty node.exe' so when I kill the winpty process:

$ node src/bin/cross-env.js cat && echo success

# kill -s SIGINT <the `winpty` process>

$ echo $?
130

If I use node directly, the kill -s SIGINT doesn't get passed to the program at all.
And when I Ctrl-C I get $? === 127

$ \node src/bin/cross-env.js cat && echo success

# kill -s SIGINT <node> doesn't work
# Ctrl-C to interrupt

$ echo $?
127

@kentcdodds
Copy link
Owner

This should be fixed now :)

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

No branches or pull requests

3 participants