-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
async_wrap,src: promise hook integration #13000
Closed
Closed
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
ef35de1
async_wrap,src: promise hook integration
563c1b0
Reintroduce promise hook sharing
4aaadc0
Cleanup
70f9675
Clean up comments
0afbcfc
remove unused variables
addaleax 833ad39
use .As<>() for casting
addaleax 5717d75
align arguments (trevnorris review)
addaleax 34ee31f
add CHECK_NE() (trevnorris review)
addaleax e0c194b
re-untangle domain/async_hooks promise support
addaleax e1c11e7
async_hooks: only set up hooks if used
addaleax d47e97a
Clarify use of additional object to track promise lifespan.
012171a
Clarifying comment about destroy hooks in test-promise.js
4e38bda
Remove redundant domain tracking
13313bb
Fix lint errors
9b9c310
Performance improvements
81ff7f6
Appease linter
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FatalException()
always exits the application in this case becauseClearFatalException()
removed allunhandledException
callbacks. So I'll assume thisreturn
is so the compiler doesn't emit a warning.I decided this would be the safest course of action when first implementing
AsyncWrap
because it was difficult to properly clean up if an async hook threw. It was difficult enough to properly cleanup if a user's callback threw (seeprocess._fatalException()
inlib/internal/bootstrap_node.js
). If we can properly show it's safe to recover from an async hook throwing then I'm all for changing this behavior.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, maybe we can look into that more in a follow on pr.