fixes #860: failure to exit on SIGINT race condition #1157
Conversation
Codecov Report
@@ Coverage Diff @@ ## master #1157 +/- ## ====================================== Coverage 76.8% 76.8% ====================================== Files 5 5 Lines 470 470 Branches 151 151 ====================================== Hits 361 361 Misses 109 109Continue to review full report at Codecov.
|
| process.on(sig, () => { | ||
| server.close(); | ||
| process.exit(); // eslint-disable-line no-process-exit | ||
| server.close(() => { |
There was a problem hiding this comment.
process.nextTick() ? But not 💯
There was a problem hiding this comment.
@michael-ciniawsky hm? please elaborate :)
There was a problem hiding this comment.
Hmm I think it is bananas from my side tbh :), the intend was
server.close() process.nextTick(() => process.exit())to ensure server.close() finishes before process.exit() is called by pushing it to the next EventLoop 'tick'. But a callback to server.close() is definitely cleaner and more expressive/logical, so forget what I said 😛
There was a problem hiding this comment.
haha no worries. something good to keep in mind though!
2 participants
What kind of change does this PR introduce?
Properly calls
process.exitonly afterserver.closehas completed. Probable fix for reported edge case whereby the server process doesn't actually exit when SIGINT is sent.Did you add or update the
examples/?No need
Summary
See above
Does this PR introduce a breaking change?
No
Other information
Added new test to confirm SIGINT detection works properly.