process: unhandledRejection support more errors #41682

benjamingr · 2022-01-24T18:51:28Z

Support cross realm errors where instanceof errors in our
unhandledRejection warning to print better error with stack traces.

This means that unhandled rejection warnings will print better cross-realm errors with --unhandled-rejections=warn

Refs: #41676

nodejs-github-bot · 2022-01-24T19:36:53Z

CI: https://ci.nodejs.org/job/node-test-pull-request/42123/

lib/internal/process/promises.js

Support cross realm errors where `instanceof` errors in our unhandledRejection warning to print better error with stack traces.

nodejs-github-bot · 2022-01-24T20:03:02Z

CI: https://ci.nodejs.org/job/node-test-pull-request/42124/

nodejs-github-bot · 2022-01-25T12:47:18Z

CI: https://ci.nodejs.org/job/node-test-pull-request/42134/

benjamingr · 2022-01-25T14:29:16Z

OMG I think CI just passed on the first try this has to be telling me something I'm going to go buy a lottery ticket 🤩

ItamarGronich · 2022-01-26T16:07:15Z

test/parallel/test-promise-unhandled-warn.js

 // Unhandled rejections trigger two warning per rejection. One is the rejection
 // reason and the other is a note where this warning is coming from.
-process.on('warning', common.mustCall(4));
+process.on('warning', common.mustCall((reason) => {


What will happen if --unhandled-rejections=throw (which is the default nowadays)
does this test cover it? the title says unhandled-warn so maybe not this test, maybe need to add to modify another test which handles the throw value?

@ItamarGronich there is a separate test for test-promise-unhandled-throw though that's harder to test given it's process output.

benjamingr · 2022-01-27T14:18:15Z

@aduh95 can I get a review/lgtm I want to do more work on this area of the code and don't wanna float :)?

bnb · 2022-01-27T17:00:46Z

lib/internal/process/promises.js

+function isErrorLike(o) {
+  return typeof o === 'object' &&
+         o !== null &&
+         ObjectPrototypeHasOwnProperty(o, 'stack');


might want to use Object.hasOwn() :)

@bnb I wanted to but then we can't backport it to older versions of Node - see discussion here: #41682 (comment)

nodejs-github-bot · 2022-01-28T18:55:23Z

Commit Queue failed

- Loading data for nodejs/node/pull/41682 ✔ Done loading data for nodejs/node/pull/41682 ----------------------------------- PR info ------------------------------------ Title process: unhandledRejection support more errors (#41682) Author Benjamin Gruenbaum (@benjamingr) Branch benjamingr:unhandled-rejection-cross-realm-stack -> nodejs:master Labels process, promises, errors, needs-ci Commits 2 - process: unhandledRejection support more errors - add test Committers 1 - Benjamin Gruenbaum PR-URL: https://github.com/nodejs/node/pull/41682 Refs: https://github.com/nodejs/node/issues/41676 Reviewed-By: Nitzan Uziely Reviewed-By: Tierney Cyren ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/41682 Refs: https://github.com/nodejs/node/issues/41676 Reviewed-By: Nitzan Uziely Reviewed-By: Tierney Cyren -------------------------------------------------------------------------------- ℹ This PR was created on Mon, 24 Jan 2022 18:51:28 GMT ✔ Approvals: 2 ✔ - Nitzan Uziely (@linkgoron): https://github.com/nodejs/node/pull/41682#pullrequestreview-863862772 ✔ - Tierney Cyren (@bnb): https://github.com/nodejs/node/pull/41682#pullrequestreview-866484825 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2022-01-25T12:47:18Z: https://ci.nodejs.org/job/node-test-pull-request/42134/ - Querying data for job/node-test-pull-request/42134/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/master up to date... From https://github.com/nodejs/node * branch master -> FETCH_HEAD ✔ origin/master is now up-to-date - Downloading patch for 41682 From https://github.com/nodejs/node * branch refs/pull/41682/merge -> FETCH_HEAD ✔ Fetched commits as 0172d1d48cb6..9a234a525fda -------------------------------------------------------------------------------- [master 1764276c66] process: unhandledRejection support more errors Author: Benjamin Gruenbaum Date: Mon Jan 24 20:47:11 2022 +0200 1 file changed, 12 insertions(+), 4 deletions(-) [master 8833fb3264] add test Author: Benjamin Gruenbaum Date: Tue Jan 25 14:33:05 2022 +0200 2 files changed, 26 insertions(+), 5 deletions(-) ✔ Patches applied There are 2 commits in the PR. Attempting autorebase. Rebasing (2/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
process: unhandledRejection support more errors

Support cross realm errors where instanceof errors in our
unhandledRejection warning to print better error with stack traces.

PR-URL: #41682
Refs: #41676
Reviewed-By: Nitzan Uziely linkgoron@gmail.com
Reviewed-By: Tierney Cyren hello@bnb.im

[detached HEAD 647511a414] process: unhandledRejection support more errors
Author: Benjamin Gruenbaum benjamingr@gmail.com
Date: Mon Jan 24 20:47:11 2022 +0200
1 file changed, 12 insertions(+), 4 deletions(-)
Rebasing (3/4)
Rebasing (4/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
add test

PR-URL: #41682
Refs: #41676
Reviewed-By: Nitzan Uziely linkgoron@gmail.com
Reviewed-By: Tierney Cyren hello@bnb.im

[detached HEAD e6c16e2773] add test
Author: Benjamin Gruenbaum benjamingr@gmail.com
Date: Tue Jan 25 14:33:05 2022 +0200
2 files changed, 26 insertions(+), 5 deletions(-)

Successfully rebased and updated refs/heads/master.

ℹ Use --fixupAll option, squash the PR manually or land the PR from the command line.

https://github.com/nodejs/node/actions/runs/1763130093

nodejs-github-bot · 2022-01-28T19:11:28Z

Landed in 64c4b55

Support cross realm errors where `instanceof` errors in our unhandledRejection warning to print better error with stack traces. PR-URL: nodejs#41682 Refs: nodejs#41676 Reviewed-By: Nitzan Uziely <linkgoron@gmail.com> Reviewed-By: Tierney Cyren <hello@bnb.im>

Support cross realm errors where `instanceof` errors in our unhandledRejection warning to print better error with stack traces. PR-URL: #41682 Refs: #41676 Reviewed-By: Nitzan Uziely <linkgoron@gmail.com> Reviewed-By: Tierney Cyren <hello@bnb.im>

benjamingr mentioned this pull request Jan 24, 2022

No correct stack trace when reject value isn't an instanceof Error #41676

Closed

benjamingr force-pushed the unhandled-rejection-cross-realm-stack branch from c5093df to a03aed8 Compare January 24, 2022 18:53

nodejs-github-bot added needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. labels Jan 24, 2022

benjamingr added errors Issues and PRs related to JavaScript errors originated in Node.js core. promises Issues and PRs related to ECMAScript promises. request-ci Add this label to start a Jenkins CI on a PR. labels Jan 24, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 24, 2022

aduh95 reviewed Jan 24, 2022

View reviewed changes

lib/internal/process/promises.js Outdated Show resolved Hide resolved

process: unhandledRejection support more errors

0f493e9

Support cross realm errors where `instanceof` errors in our unhandledRejection warning to print better error with stack traces.

benjamingr force-pushed the unhandled-rejection-cross-realm-stack branch from a03aed8 to 0f493e9 Compare January 24, 2022 19:56

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 24, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 24, 2022

benjamingr mentioned this pull request Jan 24, 2022

Do not filter out stack trace for unhandled rejections jestjs/jest#10784

Closed

This comment has been minimized.

# to view

github-actions bot mentioned this pull request Jan 25, 2022

CI Reliability 2022-01-25 nodejs/reliability#182

Open

28 tasks

add test

9a234a5

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2022

github-actions bot mentioned this pull request Jan 26, 2022

CI Reliability 2022-01-26 nodejs/reliability#183

Open

26 tasks

ItamarGronich reviewed Jan 26, 2022

View reviewed changes

Linkgoron approved these changes Jan 26, 2022

View reviewed changes

github-actions bot mentioned this pull request Jan 27, 2022

CI Reliability 2022-01-27 nodejs/reliability#184

Open

30 tasks

bnb reviewed Jan 27, 2022

View reviewed changes

github-actions bot mentioned this pull request Jan 28, 2022

CI Reliability 2022-01-28 nodejs/reliability#185

Open

22 tasks

bnb approved these changes Jan 28, 2022

View reviewed changes

benjamingr added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 28, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 28, 2022

nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jan 28, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 28, 2022

nodejs-github-bot merged commit 64c4b55 into nodejs:master Jan 28, 2022

github-actions bot mentioned this pull request Jan 29, 2022

CI Reliability 2022-01-29 nodejs/reliability#186

Open

22 tasks

benjamingr deleted the unhandled-rejection-cross-realm-stack branch January 29, 2022 17:17

ruyadorno mentioned this pull request Feb 8, 2022

v17.5.0 release proposal #41897

Merged

danielleadams mentioned this pull request Mar 3, 2022

v16.14.1 release proposal #42200

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

process: unhandledRejection support more errors #41682

process: unhandledRejection support more errors #41682

benjamingr commented Jan 24, 2022

nodejs-github-bot commented Jan 24, 2022

nodejs-github-bot commented Jan 24, 2022

This comment has been minimized.

nodejs-github-bot commented Jan 25, 2022

benjamingr commented Jan 25, 2022

ItamarGronich Jan 26, 2022 •

edited

Loading

benjamingr Jan 26, 2022

benjamingr commented Jan 27, 2022

bnb Jan 27, 2022 •

edited

Loading

benjamingr Jan 27, 2022 •

edited

Loading

nodejs-github-bot commented Jan 28, 2022

PR-URL: #41682
Refs: #41676
Reviewed-By: Nitzan Uziely linkgoron@gmail.com
Reviewed-By: Tierney Cyren hello@bnb.im

PR-URL: #41682
Refs: #41676
Reviewed-By: Nitzan Uziely linkgoron@gmail.com
Reviewed-By: Tierney Cyren hello@bnb.im

Successfully rebased and updated refs/heads/master.

nodejs-github-bot commented Jan 28, 2022

process: unhandledRejection support more errors #41682

process: unhandledRejection support more errors #41682

Conversation

benjamingr commented Jan 24, 2022

nodejs-github-bot commented Jan 24, 2022

nodejs-github-bot commented Jan 24, 2022

This comment has been minimized.

nodejs-github-bot commented Jan 25, 2022

benjamingr commented Jan 25, 2022

ItamarGronich Jan 26, 2022 • edited Loading

Choose a reason for hiding this comment

benjamingr Jan 26, 2022

Choose a reason for hiding this comment

benjamingr commented Jan 27, 2022

bnb Jan 27, 2022 • edited Loading

Choose a reason for hiding this comment

benjamingr Jan 27, 2022 • edited Loading

Choose a reason for hiding this comment

nodejs-github-bot commented Jan 28, 2022

PR-URL: #41682 Refs: #41676 Reviewed-By: Nitzan Uziely linkgoron@gmail.com Reviewed-By: Tierney Cyren hello@bnb.im

PR-URL: #41682 Refs: #41676 Reviewed-By: Nitzan Uziely linkgoron@gmail.com Reviewed-By: Tierney Cyren hello@bnb.im

Successfully rebased and updated refs/heads/master.

nodejs-github-bot commented Jan 28, 2022

ItamarGronich Jan 26, 2022 •

edited

Loading

bnb Jan 27, 2022 •

edited

Loading

benjamingr Jan 27, 2022 •

edited

Loading

PR-URL: #41682
Refs: #41676
Reviewed-By: Nitzan Uziely linkgoron@gmail.com
Reviewed-By: Tierney Cyren hello@bnb.im

PR-URL: #41682
Refs: #41676
Reviewed-By: Nitzan Uziely linkgoron@gmail.com
Reviewed-By: Tierney Cyren hello@bnb.im