Draft
Conversation
This is helpful to observe promise resolution without considering the promise as handled. This allows the unhandledRejection event to still be produced if no other resolution handling occurs.
Collaborator
|
Review requested:
|
Member
|
What's the runtime overhead? |
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.
I'm looking for feedback on this experiment. This is helpful to observe promise resolution without considering the promise as handled. This allows the unhandledRejection event to still be produced if no other resolution handling occurs. This includes a very small V8 change. Assuming people find the intent of this change reasonable, I would try to get that upstreamed to V8. I just don't want to put in all the effort to make that happen if the thing itself is not likely to be accepted.
The particular use case I have in mind for this is the
tracePromisemethod inTracingChannel. Currently it wraps thenables in aPromise.resolve(...)and then calls thethen(...)on it. This is problematic though as the thenable could have other methods which disappear as the return value becomes a native promise instead. Rather than using chaining there which produces a new and crucially different promise, I would like to return the original promise. The issue is that if we just branch internally for our observation and then return the original that original is considered handled and so would no longer trigger unhandledRejection if no other continuation was attached. This change would provide an interface such that we can observe that result but opt-out of the handled transition.I imagine there might be other use cases for this in other parts of the codebase. This is intended purely as an internal utility, I don't expect we'd actually expose it in the public API anywhere.
What do folks think? Is this a reasonable solution we should be applying?
cc @nodejs/diagnostics @nodejs/v8