Merged
Conversation
mhofman
approved these changes
Aug 29, 2022
Contributor
mhofman
left a comment
There was a problem hiding this comment.
This is safe and can be merged as is, my review is mostly stylistic nits.
Sorry for not writing the correct check in the first place. Could you please open issue for me to look into unit testing this somehow?
Contributor
Author
|
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.
Our attempt to safely tolerate Node async_hooks was safe, but missed a case we could handle safely and which is provoked at least by the vscode debugger.
Node's async_hooks currently contains the following code, which we can also safely tolerate
However, before this PR we did not know to expect this. I just encountered this failure when trying to debug some of our code in the vscode debugger. The additional record that Node adds here is one we can tolerate safely. This PR's expectations includes this additional record, so that we can safely tolerate it in environments, like the vscode debugger, that provoke it.
Reviews: How do I test this? I have already tried it experimentally on the debugger experience that was blocking me. This code got me past that problem.