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

src: allow preventing SetPromiseRejectCallback by codebytere · Pull Request #34387 · nodejs/node · GitHub

/ node Public

src: allow preventing SetPromiseRejectCallback - #34387

Closed
codebytere wants to merge 1 commit into
nodejs:masterfrom
codebytere:allow-preventing-SetPromiseRejectCallback
ConversationCommits (1)ChecksFiles changed
Closed

src: allow preventing SetPromiseRejectCallback#34387
codebytere wants to merge 1 commit into
nodejs:masterfrom
codebytere:allow-preventing-SetPromiseRejectCallback

Conversation

codebytere commented Jul 16, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

This PR slightly augments the existing IsolateSettings struct for embedder purposes.

Electron does not want to use the promise rejection callback that Node.js uses,
because it does not send PromiseRejectionEvents to the global script context in the browser (for fairly obvious reasons).

Therefore, if we want to use Node.js setup logic in the renderer process (as opposed to standalone embedded mode, where we already do this) we would not be able to allow users to do things such as:

window.addEventListener('unhandledrejection', function (event) {
  // ...your code here to handle the unhandled rejection...

  // Prevent the default handling (such as outputting the
  // error to the console)

  event.preventDefault();
});

since the 'unhandledrejection' event would never be fired.

We need to use the one Blink already provides and sets on the Isolate we're given, and so we need to slightly augment IsolateSettings to allow for that.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

codebytere added the embedding Issues and PRs related to embedding Node.js in another project. label Jul 16, 2020
nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jul 16, 2020
codebytere mentioned this pull request Jul 16, 2020
Merged
5 tasks

nodejs-github-bot commented Jul 16, 2020

Copy link
Copy Markdown
Collaborator

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

addaleax added the semver-major PRs that contain breaking changes and should be released in the next major version. label Jul 16, 2020
addaleax approved these changes Jul 16, 2020
View reviewed changes

addaleax left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM, but I think it makes sense to use the inverse flag (i.e. SHOULD_NOT_SET_PROMISE_REJECTION_CALLBACK), because then this becomes semver-minor instead :)

codebytere force-pushed the allow-preventing-SetPromiseRejectCallback branch from 0e4958e to 8862256 Compare July 16, 2020 17:43
codebytere added semver-minor PRs that contain new features and should be released in the next minor version. and removed semver-major PRs that contain breaking changes and should be released in the next major version. labels Jul 16, 2020
addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jul 17, 2020

nodejs-github-bot commented Jul 17, 2020

Copy link
Copy Markdown
Collaborator

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

This was referenced Jul 18, 2020
Open
Open
Open
codebytere force-pushed the allow-preventing-SetPromiseRejectCallback branch from 8862256 to 5eec3ac Compare July 20, 2020 15:48
devsnek reviewed Jul 20, 2020
View reviewed changes
Comment thread src/node.h Outdated
codebytere force-pushed the allow-preventing-SetPromiseRejectCallback branch from 5eec3ac to 267c4c9 Compare July 20, 2020 16:11
addaleax reviewed Jul 20, 2020
View reviewed changes
Comment thread src/api/environment.cc Outdated
nodejs deleted a comment from nodejs-github-bot Jul 20, 2020
nodejs deleted a comment from nodejs-github-bot Jul 20, 2020
codebytere force-pushed the allow-preventing-SetPromiseRejectCallback branch from 267c4c9 to a7a5972 Compare July 20, 2020 17:03

nodejs-github-bot commented Jul 20, 2020

Copy link
Copy Markdown
Collaborator

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

github-actions Bot mentioned this pull request Jul 21, 2020
Open
10 tasks
codebytere requested a review from addaleax July 21, 2020 01:19
This was referenced Jul 24, 2020
Open
Open
Open
Open
MylesBorins pushed a commit that referenced this pull request Jul 27, 2020
src: allow preventing SetPromiseRejectCallback
bd9ab00 
PR-URL: #34387
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
github-actions Bot mentioned this pull request Jul 28, 2020
Open
3 tasks
ruyadorno added a commit that referenced this pull request Jul 28, 2020
2020-07-29, Version 14.7.0 (Current)
b04e1d7 
Notable changes:

dgram:
  * (SEMVER-MINOR) add IPv6 scope id suffix to received udp6 dgrams (Pekka Nikander) #14500
doc:
  * add AshCripps to collaborators (AshCripps) #34494
  * add HarshithaKP to collaborators (Harshitha K P) #34417
  * add rexagod to collaborators (Pranshu Srivastava) #34457
  * add release key for Richard Lau (Richard Lau) #34397
events:
  * (SEMVER-MINOR) expand NodeEventTarget functionality (Anna Henningsen) #34057
src:
  * (SEMVER-MINOR) allow preventing SetPromiseRejectCallback (Shelley Vohr) #34387
  * (SEMVER-MINOR) allow setting a dir for all diagnostic output (AshCripps) #33584
worker:
  * (SEMVER-MINOR) make MessagePort inherit from EventTarget (Anna Henningsen) #34057

PR-URL: TODO
ruyadorno mentioned this pull request Jul 28, 2020
Merged
ruyadorno added a commit that referenced this pull request Jul 28, 2020
2020-07-29, Version 14.7.0 (Current)
44a6478 
Notable changes:

dgram:
  * (SEMVER-MINOR) add IPv6 scope id suffix to received udp6 dgrams (Pekka Nikander) #14500
doc:
  * add AshCripps to collaborators (AshCripps) #34494
  * add HarshithaKP to collaborators (Harshitha K P) #34417
  * add rexagod to collaborators (Pranshu Srivastava) #34457
  * add release key for Richard Lau (Richard Lau) #34397
events:
  * (SEMVER-MINOR) expand NodeEventTarget functionality (Anna Henningsen) #34057
src:
  * (SEMVER-MINOR) allow preventing SetPromiseRejectCallback (Shelley Vohr) #34387
  * (SEMVER-MINOR) allow setting a dir for all diagnostic output (AshCripps) #33584
worker:
  * (SEMVER-MINOR) make MessagePort inherit from EventTarget (Anna Henningsen) #34057
zlib:
  * switch to lazy init for zlib streams (Andrey Pechkurov) #34048

PR-URL: #34542
ruyadorno added a commit that referenced this pull request Jul 29, 2020
2020-07-29, Version 14.7.0 (Current)
6171265 
Notable changes:

deps:
  * upgrade npm to 6.14.7 (claudiahdz) #34468
dgram:
  * (SEMVER-MINOR) add IPv6 scope id suffix to received udp6 dgrams (Pekka Nikander) #14500
doc:
  * add AshCripps to collaborators (AshCripps) #34494
  * add HarshithaKP to collaborators (Harshitha K P) #34417
  * add rexagod to collaborators (Pranshu Srivastava) #34457
  * add release key for Richard Lau (Richard Lau) #34397
events:
  * (SEMVER-MINOR) expand NodeEventTarget functionality (Anna Henningsen) #34057
src:
  * (SEMVER-MINOR) allow preventing SetPromiseRejectCallback (Shelley Vohr) #34387
  * (SEMVER-MINOR) allow setting a dir for all diagnostic output (AshCripps) #33584
worker:
  * (SEMVER-MINOR) make MessagePort inherit from EventTarget (Anna Henningsen) #34057
zlib:
  * switch to lazy init for zlib streams (Andrey Pechkurov) #34048

PR-URL: #34542
MylesBorins pushed a commit that referenced this pull request Jul 29, 2020
2020-07-29, Version 14.7.0 (Current)
168b22b 
Notable changes:

deps:
  * upgrade npm to 6.14.7 (claudiahdz) #34468
dgram:
  * (SEMVER-MINOR) add IPv6 scope id suffix to received udp6 dgrams (Pekka Nikander) #14500
doc:
  * add AshCripps to collaborators (AshCripps) #34494
  * add HarshithaKP to collaborators (Harshitha K P) #34417
  * add rexagod to collaborators (Pranshu Srivastava) #34457
  * add release key for Richard Lau (Richard Lau) #34397
events:
  * (SEMVER-MINOR) expand NodeEventTarget functionality (Anna Henningsen) #34057
src:
  * (SEMVER-MINOR) allow preventing SetPromiseRejectCallback (Shelley Vohr) #34387
  * (SEMVER-MINOR) allow setting a dir for all diagnostic output (AshCripps) #33584
worker:
  * (SEMVER-MINOR) make MessagePort inherit from EventTarget (Anna Henningsen) #34057
zlib:
  * switch to lazy init for zlib streams (Andrey Pechkurov) #34048

PR-URL: #34542
codebytere added a commit to electron/electron that referenced this pull request Sep 1, 2020
Remove redundant promise rejection callback
9bd811f 
nodejs/node#34387
codebytere added a commit to electron/electron that referenced this pull request Sep 1, 2020
Remove redundant promise rejection callback
73c2d55 
nodejs/node#34387
codebytere added a commit to electron/electron that referenced this pull request Sep 2, 2020
Remove redundant promise rejection callback
723fa67 
nodejs/node#34387
codebytere added a commit to electron/electron that referenced this pull request Sep 4, 2020
Remove redundant promise rejection callback
b529ee0 
nodejs/node#34387
codebytere added a commit to electron/electron that referenced this pull request Sep 14, 2020
Remove redundant promise rejection callback
73a844e 
nodejs/node#34387
codebytere added a commit to electron/electron that referenced this pull request Sep 14, 2020
Remove redundant promise rejection callback
bf81d63 
nodejs/node#34387
codebytere added a commit to electron/electron that referenced this pull request Sep 14, 2020
Remove redundant promise rejection callback
1e6f14e 
nodejs/node#34387
codebytere added a commit to electron/electron that referenced this pull request Sep 16, 2020
Remove redundant promise rejection callback
a128148 
nodejs/node#34387
addaleax pushed a commit that referenced this pull request Sep 22, 2020
src: allow preventing SetPromiseRejectCallback
18e8687 
PR-URL: #34387
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
codebytere mentioned this pull request Sep 28, 2020
Merged
mhdawson mentioned this pull request Jan 18, 2022
Closed
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

jasnell jasnell approved these changes
addaleax addaleax approved these changes
tniessen tniessen approved these changes
joyeecheung Awaiting requested review from joyeecheung
+1 more reviewer
devsnek devsnek approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. embedding Issues and PRs related to embedding Node.js in another project. 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.

6 participants

Back | FazBrowse Home | New Git URL