Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[v14.x backport] async_hooks: eliminate native PromiseHook #39742

Closed
wants to merge 4 commits into from
Closed

[v14.x backport] async_hooks: eliminate native PromiseHook #39742

wants to merge 4 commits into from

Conversation

Qard
Copy link
Member

@Qard Qard commented Aug 12, 2021

This is a backport of #39135.

@Qard Qard added c++ Issues and PRs that require attention from people who are familiar with C++. async_hooks Issues and PRs related to the async hooks subsystem. v14.x labels Aug 12, 2021
@github-actions github-actions bot added domain Issues and PRs related to the domain subsystem. needs-ci PRs that need a full CI run. labels Aug 12, 2021
@Qard Qard added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 12, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 12, 2021
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

Stephen Belanger added 4 commits August 12, 2021 15:53
async_hooks: eliminate native PromiseHook
d6cfbb6 
PR-URL: nodejs#39135
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
async_hooks: emit promise trace events from JS
2f88a81 
PR-URL: nodejs#39135
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
domain: do not add domain to promise from other context
0cb2c24 
PR-URL: nodejs#39135
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
lib: comment explaining special-case handling of promises
bbb6259 
Promise handling is special-cased for domains and trace_events.
Domains must not add the `domain` property to promises in other
contexts, and trace_events must be emitted from JavaScript as
promises no longer have an AsyncWrap type attached to them.

PR-URL: nodejs#39135
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
@Qard Qard force-pushed the backport-39135-to-14 branch from b34cbd9 to bbb6259 Compare August 12, 2021 22:53
@Qard Qard added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 16, 2021
@targos
Copy link
Member

targos commented Aug 29, 2021

Landed in f8b4dc1...c41466f

@targos targos closed this Aug 29, 2021
targos pushed a commit that referenced this pull request Aug 29, 2021
@targos
async_hooks: eliminate native PromiseHook
e6b7eb1 
PR-URL: #39135
Backport-PR-URL: #39742
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
targos pushed a commit that referenced this pull request Aug 29, 2021
@targos
async_hooks: emit promise trace events from JS
22d95f1 
PR-URL: #39135
Backport-PR-URL: #39742
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
targos pushed a commit that referenced this pull request Aug 29, 2021
@targos
domain: do not add domain to promise from other context
6e58915 
PR-URL: #39135
Backport-PR-URL: #39742
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
targos pushed a commit that referenced this pull request Aug 29, 2021
@targos
lib: comment explaining special-case handling of promises
c41466f 
Promise handling is special-cased for domains and trace_events.
Domains must not add the `domain` property to promises in other
contexts, and trace_events must be emitted from JavaScript as
promises no longer have an AsyncWrap type attached to them.

PR-URL: #39135
Backport-PR-URL: #39742
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 31, 2021
@MylesBorins
async_hooks: eliminate native PromiseHook
13296d1 
PR-URL: #39135
Backport-PR-URL: #39742
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 31, 2021
@MylesBorins
async_hooks: emit promise trace events from JS
fc29ddb 
PR-URL: #39135
Backport-PR-URL: #39742
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 31, 2021
@MylesBorins
domain: do not add domain to promise from other context
5500ae9 
PR-URL: #39135
Backport-PR-URL: #39742
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 31, 2021
@MylesBorins
lib: comment explaining special-case handling of promises
1737352 
Promise handling is special-cased for domains and trace_events.
Domains must not add the `domain` property to promises in other
contexts, and trace_events must be emitted from JavaScript as
promises no longer have an AsyncWrap type attached to them.

PR-URL: #39135
Backport-PR-URL: #39742
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
@foxxyz
async_hooks: eliminate native PromiseHook
5a1841c 
PR-URL: nodejs#39135
Backport-PR-URL: nodejs#39742
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
@foxxyz
async_hooks: emit promise trace events from JS
7114224 
PR-URL: nodejs#39135
Backport-PR-URL: nodejs#39742
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
@foxxyz
domain: do not add domain to promise from other context
0dc51c1 
PR-URL: nodejs#39135
Backport-PR-URL: nodejs#39742
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
@foxxyz
lib: comment explaining special-case handling of promises
8f24614 
Promise handling is special-cased for domains and trace_events.
Domains must not add the `domain` property to promises in other
contexts, and trace_events must be emitted from JavaScript as
promises no longer have an AsyncWrap type attached to them.

PR-URL: nodejs#39135
Backport-PR-URL: nodejs#39742
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
@Qard Qard deleted the backport-39135-to-14 branch June 18, 2022 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Flarna Flarna Flarna approved these changes

Assignees
No one assigned
Labels
async_hooks Issues and PRs related to the async hooks subsystem. author ready PRs 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++. domain Issues and PRs related to the domain subsystem. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Qard @nodejs-github-bot @targos @Flarna