Skip to content

Conversation

@codebytere
Copy link
Member

@codebyterecodebytere commented Jul 16, 2020

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

@codebyterecodebytere added the embedding Issues and PRs related to embedding Node.js in another project. label Jul 16, 2020
@nodejs-github-botnodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jul 16, 2020
@nodejs-github-bot
Copy link
Collaborator

@addaleaxaddaleax added the semver-major PRs that contain breaking changes and should be released in the next major version. label Jul 16, 2020
Copy link
Member

@addaleaxaddaleax left a comment

Choose a reason for hiding this comment

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

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 :)

@codebyterecodebytereforce-pushed the allow-preventing-SetPromiseRejectCallback branch from 0e4958e to 8862256CompareJuly 16, 2020 17:43
@codebyterecodebytere 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
@addaleaxaddaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 17, 2020
@nodejs-github-bot
Copy link
Collaborator

@codebyterecodebytereforce-pushed the allow-preventing-SetPromiseRejectCallback branch from 5eec3ac to 267c4c9CompareJuly 20, 2020 16:11
@nodejsnodejs deleted a comment from nodejs-github-botJul 20, 2020
@nodejsnodejs deleted a comment from nodejs-github-botJul 20, 2020
@codebyterecodebytereforce-pushed the allow-preventing-SetPromiseRejectCallback branch from 267c4c9 to a7a5972CompareJuly 20, 2020 17:03
@nodejs-github-bot
Copy link
Collaborator

MylesBorins pushed a commit that referenced this pull request Jul 27, 2020
PR-URL: #34387 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
ruyadorno added a commit that referenced this pull request Jul 28, 2020
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
@ruyadornoruyadorno mentioned this pull request Jul 28, 2020
ruyadorno added a commit that referenced this pull request Jul 28, 2020
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
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
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
codebytere added a commit to electron/electron that referenced this pull request Sep 1, 2020
codebytere added a commit to electron/electron that referenced this pull request Sep 2, 2020
codebytere added a commit to electron/electron that referenced this pull request Sep 4, 2020
codebytere added a commit to electron/electron that referenced this pull request Sep 14, 2020
codebytere added a commit to electron/electron that referenced this pull request Sep 14, 2020
codebytere added a commit to electron/electron that referenced this pull request Sep 14, 2020
codebytere added a commit to electron/electron that referenced this pull request Sep 16, 2020
addaleax pushed a commit that referenced this pull request Sep 22, 2020
PR-URL: #34387 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
@codebyterecodebytere mentioned this pull request Sep 28, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.c++Issues and PRs that require attention from people who are familiar with C++.embeddingIssues and PRs related to embedding Node.js in another project.semver-minorPRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@codebytere@nodejs-github-bot@jasnell@addaleax@tniessen@devsnek