Smarter handling of exit code 2

[joeytwiddle]

This PR removes the special handling of exit code 2.

Not only mocha but also stylelint will sometimes exit with code 2, resulting in unexpected behaviour from nodemon. (It exits, and stops monitoring.)

The || true workaround is unusual and easy to forget. I only ever remember to add it after I have been bitten!

If there is no special reason to keep the special handling of code 2, then I think it would be best to remove it.

(In the case when a developer does provide a non-parseable command, which exits with code 2, an error is already reported to the console. So we won't be keeping developers in the dark by not quitting. After this PR, the developer will need to hit Ctrl-C to exit nodemon, in order to fix their command. For an example syntax error, try: nodemon -x 'ls <')

Fixes #496 #627

This could perhaps be part of a major release, just in case some people are using it as a way to break out of nodemon intentionally: #751 (comment)

[remy]

I'm not convinced this is the right change. I think the messaging in the exit should be clearer, but that's all.

With your changes, here is the workflow:

❯ nodemon -x 'ls <'
[nodemon] to restart at any time, enter `rs`
[nodemon] watching: *.*
[nodemon] starting `ls <`
sh: -c: line 0: syntax error near unexpected token `newline'
sh: -c: line 0: `ls <'
[nodemon] app crashed - waiting for file changes before starting...

Since the actual exec command is wrong, the developer has to ctrl+c nodemon and correct the exec command.

Automatically restarting on file changes will never make that command work.

I think the right change is to update the error message that's shown, perhaps linking to the FAQ (which can be added to with links and even more context).

I can't see a reason for nodemon not to exit on code 2 - unless I'm missing something?

[joeytwiddle]

The argument not to exit on code 2 is that mocha will sometimes exit with code 1, 2, 3, 4, ... (and stylelint too), and usually in those cases we still want nodemon to keep watching and running.

Currently behaviour with mocha is somewhat inconsistent. nodemon will happily watch and run passing and failing tests for a while, but then it will surprisingly stop running entirely (when exactly 2 mocha tests fail). It appears random, until you find the GitHub issue explaining that code 2 is special, and you are advised to put || true at the end of your mocha command.

On the other hand, yes sometimes code 2 means you wrote a bad shell command. And in that case exiting nodemon is slightly preferable.

So I guess we have to decide which is more bothersome: nodemon stops running mocha tests unexpectedly, or having to hit Ctrl-C after typing a bad command.

Anyway I do agree, in either case, some messaging may help to guide the developer. I will look into that.

Other options:

1. We could try harder to detect whether the code 2 is from a bad command or from mocha/stylelint. For example:
   1a. we could read stderr for things like sh: -c: line 0: syntax error, or
   1b. we could measure if the command fails immediately the first time it is executed.
2. We could raise issues on mocha and stylelint packages, explaining that they are using the reserved code 2 incorrectly. (But I suspect they will not want to change the behaviour.)

I would like to look into 1b, and situational messaging.

[joeytwiddle]

OK, I have changed the behaviour:

• If the exit code is 2, and the time taken for the child to fail is < 500ms, then we assume the command had a syntax error, and nodemon exits as usual.
• Otherwise we assume the command ran ok, but the process exited with code 2 for some other reason. In this case we keep watching.
• I have improved the messaging a bit (perhaps verbose, but hopefully clear).
• (I have not checked if this is the first run or not. It doesn't seem necessary. Similarly, I am not reading stderr.)

To test, try:

./bin/nodemon.js -x 'ls <'
# exits with logging

# Now simulate mocha failing with two tests
./bin/nodemon.js -x 'sleep 1 ; exit 2'
# nodemon keeps running

---

Possible concerns:

• If the command exits with code 2 after more than 500ms, there is no special messaging about it. (nodemon acts as if it exited with code 1.)

  This situation could occur if the command given to nodemon was fine, but the called process later runs a script or command with a bad command in it.

  I actually think this behaviour (no special messaging) is acceptable, but it is a change from the past.

• Is bus.emit('error', code); really necessary now? It logs a stack-trace which I don't think is interesting to most users.

• I kept the logged lines quite short, to satisfy the linter. Some of the logging could theoretically squeeze onto fewer lines.

• The 500ms is of course an arbitrary figure. Hopefully all shells will fail faster than that, and all mocha tests will fail slower than that! (If mocha does fail too quickly, then at least the logging should help the user. But if the shell starts too slowly, there is currently no helpful message for the user. We could add one.)

[remy]

Can you add: Read more here: https://git.io/fNOAG after line 169.

[remy]

Two comments I've left, and once these are in, I'm happy with the PR - thank you and great work 👍

[joeytwiddle]

Remy, I only saw one comment from you! I have added the link to the FAQ as you asked.

But what was the second comment? I have taken a guess, and changed the || true into || exit 1 like the FAQ suggests. Did I guess right?

Thank you for nodemon, an invaluable productivity tool! 😄

[remy]

I don't understand what github is doing, but I've taken a screenshot of the comments - sorry, bit naff!

[joeytwiddle]

My apologies. I probably made a rebase/amend and a force push yesterday, just after you commented, so the comment got lost.

I have added a code comment with the fNOAR link now.

If you want to make adjustments to this branch directly, I will not be offended. 🙇‍♂️

[remy]

If you want to make adjustments to this branch directly, I will not be offended. 🙇

I've never really known how do that cleanly. Once I clone the branch locally, (from past experience) I couldn't push back into the PR and had to make my own branch and new PR - really odd. Anyway, this looks good - thanks!