worker: fix exit code for error thrown in uncaughtException handler#38012
Closed
Linkgoron wants to merge 3 commits intonodejs:masterfrom
Closed
worker: fix exit code for error thrown in uncaughtException handler#38012Linkgoron wants to merge 3 commits intonodejs:masterfrom
Linkgoron wants to merge 3 commits intonodejs:masterfrom
Conversation
Change worker exit code when the unhandled exception handler throws from 0 to 7 fixes: nodejs#37996
ddcff99 to
7cf8425
Compare
This comment has been minimized.
This comment has been minimized.
|
Thanks for fixing this so quickly. I think the docs need an update as well if we're going with 7 and not 1
https://nodejs.org/api/worker_threads.html#worker_threads_event_exit |
Contributor
Author
|
I've changed the exit code to |
This comment has been minimized.
This comment has been minimized.
287f22e to
7779d71
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
@nodejs/workers |
benjamingr
approved these changes
Apr 3, 2021
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Collaborator
jasnell
approved these changes
Apr 6, 2021
Member
|
Landed in 6986fa0 |
|
thanks!
The 7 slipped in there again "from 0 to 7" |
Contributor
Author
Yeah, it looks like I missed amending the commit message. The PR is good though, and correctly exits with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When an
uncaughtExceptionhandler itself throws in a worker, the worker exits with an error code of 0 instead of71, which happens because the worker thread global handler catches the error, and exitCode stays 0.My fix only emits
exitfor "regular" unhandled exceptions (which shouldn't actually reach that code anyway, as it should set_exitingcorrectly in the "inner" handler) to emulate the same behaviour that happens in non-workers, whereexitis not emitted when an error is thrown from a handler.The
process._exitinglogic was essentially taken from here: https://github.com/nodejs/node/blob/master/lib/internal/process/execution.js#L167Fixes: #37996