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.

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

lib: properly processing JavaScript exceptions on async_hooks fatal error #38106

Closed
wants to merge 3 commits into from

Conversation

legendecas
Copy link
Member

@legendecas legendecas commented Apr 6, 2021

JavaScript exceptions could be arbitrary values.

Status-quo of the behavior would result in TypeError in node internals

TypeError: Cannot read property 'stack' of null
    at fatalError (node:internal/async_hooks:161:16)

@nodejs-github-bot nodejs-github-bot added async_hooks Issues and PRs related to the async hooks subsystem. needs-ci PRs that need a full CI run. labels Apr 6, 2021
@legendecas legendecas force-pushed the async_hooks/fatal_error branch 2 times, most recently from bdc1aa2 to 4f5bead Compare April 6, 2021 04:09
lib/internal/async_hooks.js Outdated Show resolved Hide resolved
lib/internal/async_hooks.js Outdated Show resolved Hide resolved
…rror

JavaScript exceptions could be arbitrary values.
@legendecas legendecas force-pushed the async_hooks/fatal_error branch from 4f5bead to 56f756a Compare April 6, 2021 11:21
@nodejs-github-bot

This comment has been minimized.

@Flarna Flarna added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Apr 7, 2021
@Flarna
Copy link
Member

Flarna commented Apr 7, 2021

@legendecas Unfortunately it seems your test fails on windows (see https://ci.nodejs.org/job/node-test-binary-windows-js-suites/9219/)

@legendecas
Copy link
Member Author

@Flarna thanks, fixed.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@Flarna Flarna added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 7, 2021
@jasnell jasnell requested a review from addaleax April 7, 2021 13:58
Co-authored-by: Anna Henningsen <github@addaleax.net>
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

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

Flarna pushed a commit that referenced this pull request Apr 8, 2021
JavaScript exceptions could be arbitrary values.

PR-URL: #38106
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Flarna
Copy link
Member

Flarna commented Apr 8, 2021

Landed in d861324

@Flarna Flarna closed this Apr 8, 2021
@legendecas legendecas deleted the async_hooks/fatal_error branch April 9, 2021 17:13
targos pushed a commit that referenced this pull request May 1, 2021
JavaScript exceptions could be arbitrary values.

PR-URL: #38106
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@danielleadams danielleadams mentioned this pull request May 3, 2021
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 pending requests for changes, and a CI started. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants