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

async_hooks: proper handling for AH in runInAsyncScope by apapirovski · Pull Request #30965 · nodejs/node · GitHub

/ node Public

async_hooks: proper handling for AH in runInAsyncScope - #30965

Closed
apapirovski wants to merge 1 commit into
nodejs:masterfrom
apapirovski:simplify-run-in-async-scope
Closed

async_hooks: proper handling for AH in runInAsyncScope#30965
apapirovski wants to merge 1 commit into
nodejs:masterfrom
apapirovski:simplify-run-in-async-scope

Conversation

Copy link
Copy Markdown
Contributor

We should never try to manually run emitAfter in case of an error,
the exception handler will do it for us, if we're going to recover.

This PR expands on the fix in #30087

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

We should never try to manually run emitAfter in case of an error,
the exception handler will do it for us, if we're going to recover.
apapirovski requested a review from addaleax December 14, 2019 19:08
nodejs-github-bot added the async_hooks Issues and PRs related to the async hooks subsystem. label Dec 14, 2019

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

Trott added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Dec 15, 2019
Trott pushed a commit that referenced this pull request Dec 17, 2019
We should never try to manually run emitAfter in case of an error,
the exception handler will do it for us, if we're going to recover.

PR-URL: #30965
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>

Trott commented Dec 17, 2019

Copy link
Copy Markdown
Member

Landed in 503900b

Trott closed this Dec 17, 2019
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
We should never try to manually run emitAfter in case of an error,
the exception handler will do it for us, if we're going to recover.

PR-URL: #30965
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins mentioned this pull request Dec 17, 2019
targos pushed a commit that referenced this pull request Jan 14, 2020
We should never try to manually run emitAfter in case of an error,
the exception handler will do it for us, if we're going to recover.

PR-URL: #30965
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
We should never try to manually run emitAfter in case of an error,
the exception handler will do it for us, if we're going to recover.

PR-URL: #30965
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins mentioned this pull request Feb 8, 2020

Copy link
Copy Markdown
Contributor

This seems to have broken some things, more specifically it seems related to #31783.

I'm reverting it from v12.x and we should reland when the fix has been on current and the reported bugs are fixed

addaleax pushed a commit that referenced this pull request Feb 14, 2020
The exception handler user-defined will not automatically emit after
for the async resource.

Also removes a duplicated case
`test-emit-after-uncaught-exception-runInAsyncScope.js`
which is identical to test-emit-after-uncaught-exception.js.

Refs: #30965
PR-URL: #31784
Fixes: #31783
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax pushed a commit that referenced this pull request Feb 14, 2020
The exception handler user-defined will not automatically emit after
for the async resource.

Also removes a duplicated case
`test-emit-after-uncaught-exception-runInAsyncScope.js`
which is identical to test-emit-after-uncaught-exception.js.

Refs: #30965
PR-URL: #31784
Fixes: #31783
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
codebytere pushed a commit that referenced this pull request Feb 17, 2020
The exception handler user-defined will not automatically emit after
for the async resource.

Also removes a duplicated case
`test-emit-after-uncaught-exception-runInAsyncScope.js`
which is identical to test-emit-after-uncaught-exception.js.

Refs: #30965
PR-URL: #31784
Fixes: #31783
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins mentioned this pull request Feb 18, 2020
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

async_hooks Issues and PRs related to the async hooks subsystem. author ready PRs that have at least one approval, no outstanding review comments, and a CI started.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants


Back | FazBrowse Home | New Git URL