GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||
GeezerVison is showing github.com — doc: clarify `uncaughtException` `origin` for ESM by aduh95 · Pull Request #41339 · nodejs/node Conversation
Contributor
Fixes: #41328 /cc @nodejs/modules
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
GeoffreyBooth
approved these changes
bmeck
reviewed
Uh oh!
There was an error while loading. Please reload this page.
bmeck
suggested changes
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should workshop wording to make it clear that it isn't causes necessarily by ESM entrypoints but just having things wrapped in a Promise async context in general, see a purely CJS duplication: // node --unhandled-rejections=strict entry.cjs new Promise(() => require('./throws.cjs')); // throws.cjs process.on('uncaughtExceptionMonitor', (...args) => { console.log(args); }) throw new Error('thrown'); might mean just more clearly defining what an
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
This comment has been minimized.
Contributor
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
bmeck
reviewed
Uh oh!
There was an error while loading. Please reload this page.
bmeck
reviewed
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we just simplify this to
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have a purely CJS example when this happen when
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aduh95 I do not for that case, but it does happen outside of ES module static loading phase.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Any chance we can list those edge cases as well? I don't think
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Didn’t the default for
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It changed in v15.0.0. But that's beside the point I think, this section of the docs is currently wrong, and if this PR doesn't do a good enough job to document when users should expect
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aduh95 roughly anything that creates a Promise job causes that, so IDK where to start. If an uncaughtException would occur inside of an async function / inside of a Promise constructor/handler callbacks / inside ESM are all the real times that this happens. So uncaughtException isn't really about the source file / throwing at the top level, but it is about how it was invoked, always. Listing async functions and Promise methods might be enough?
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. async functions, promise then/catch/finally, new Promise executor functions, and soon, Array.fromAsync
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@bmeck I think that is true when What is more tricky is that a rejection in the ES loader will always trigger an
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't think of another off the top of my head. I think we've done enough workshopping though that whatever is clarified seems good at this point.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
bmeck
approved these changes
Collaborator
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Contributor
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
targos
pushed a commit
that referenced
this pull request
Fixes: #41328 PR-URL: #41339 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Bradley Farias <[email protected]>
danielleadams
pushed a commit
that referenced
this pull request
Fixes: #41328 PR-URL: #41339 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Bradley Farias <[email protected]>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers+1 more reviewerUh oh!
There was an error while loading. Please reload this page. AssigneesNo one assignedLabels
author ready
ProjectsMilestoneNo milestoneDevelopmentSuccessfully merging this pull request may close these issues. uncaughtException origin is always unhandledRejection when package.json type is moduleUh oh!
There was an error while loading. Please reload this page. 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. | ||||||||||||||||
| You are visitor 01402503 — GeezerVison does not store the pages you look up. Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required. |