FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[v10.x] backport #26599 (process: add --unhandled-rejections flag) by BridgeAR · Pull Request #29036 · nodejs/node · GitHub

/ node Public

[v10.x] backport #26599 (process: add --unhandled-rejections flag) - #29036

Closed
BridgeAR wants to merge 2 commits into
nodejs:v10.x-stagingfrom
BridgeAR:v10.x-backport-26599
ConversationCommits (2)ChecksFiles changed
Closed

[v10.x] backport #26599 (process: add --unhandled-rejections flag)#29036
BridgeAR wants to merge 2 commits into
nodejs:v10.x-stagingfrom
BridgeAR:v10.x-backport-26599

Conversation

BridgeAR commented Aug 7, 2019

Copy link
Copy Markdown
Member

Backporting #26599 was not completely trivial, since there where so many changes that required a work around. Functionality wise it should now be identical.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

process: add --unhandled-rejections flag
e7ff5ea 
This adds a flag to define the default behavior for unhandled
rejections. Three modes exist: `none`, `warn` and `strict`. The first
is going to silence all unhandled rejection warnings. The second
behaves identical to the current default with the excetion that no
deprecation warning will be printed and the last is going to throw
an error for each unhandled rejection, just as regular exceptions do.
It is possible to intercept those with the `uncaughtException` hook
as with all other exceptions as well.

This PR has no influence on the existing `unhandledRejection` hook.
If that is used, it will continue to function as before.

PR-URL: nodejs#26599
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. v10.x labels Aug 7, 2019

nodejs-github-bot commented Sep 20, 2019

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Sep 24, 2019

Copy link
Copy Markdown
Collaborator

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

BridgeAR commented Sep 24, 2019

Copy link
Copy Markdown
Member Author

@addaleax would you be so kind and have a look at this? Seems like one test fails when run inside of a worker (in the freestyle test suite) and I am not able to reproduce this locally and I don't see any issue with the code as it is :/.

addaleax added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 24, 2019

addaleax commented Sep 24, 2019

Copy link
Copy Markdown
Member

@BridgeAR The assert.strictEqual(origin, 'unhandledRejection', err); line in the uncaughtException handler is failing because origin === 'unhandledException' in the second call. Having err as the third argument to assert.strictEqual is what’s masking the issue here.

As for reproducing, ./node --experimental-worker --unhandled-rejections=strict tools/run-worker.js test/parallel/test-promise-unhandled-error.js works for me.

BridgeAR commented Sep 24, 2019

Copy link
Copy Markdown
Member Author

@addaleax the test itself should work though. It works fine on v12 and master but fails with this backport while being executed in a worker.

addaleax reviewed Sep 24, 2019
View reviewed changes
Comment thread lib/internal/process/promises.js
worker: add missing return value in case of fatal exceptions
26d3b0a 
This adds a missing return value for the worker specific fatal
exception handler.

nodejs-github-bot commented Sep 24, 2019

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Sep 25, 2019

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Sep 25, 2019

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Sep 25, 2019

Copy link
Copy Markdown
Collaborator

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

BethGriggs pushed a commit that referenced this pull request Oct 15, 2019
worker: add missing return value in case of fatal exceptions
2eae030 
This adds a missing return value for the worker specific fatal
exception handler.

PR-URL: #29036
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>

BethGriggs commented Oct 15, 2019

Copy link
Copy Markdown
Member

Landed on v10.x-staging

BethGriggs closed this Oct 15, 2019
BethGriggs mentioned this pull request Oct 18, 2019
Merged
BridgeAR deleted the v10.x-backport-26599 branch January 20, 2020 12:05
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

addaleax addaleax left review comments
BethGriggs BethGriggs approved these changes

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL