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

async_hooks: use CHECK instead of throwing error #17832

Closed
wants to merge 1 commit into from

Conversation

maclover7
Copy link
Contributor

SetupHooks is only available via process.binding('async_wrap'), so
there's no reason it shouldn't be called with the appropriate arguments,
since it is an internal-only function. The only place this function is
used is lib/internal/async_hooks.js.

cc @jasnell

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

async_hooks

SetupHooks is only available via `process.binding('async_wrap')`, so
there's no reason it shouldn't be called with the appropriate arguments,
since it is an internal-only function. The only place this function is
used is `lib/internal/async_hooks.js`.
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Dec 23, 2017
@maclover7
Copy link
Contributor Author

@maclover7
Copy link
Contributor Author

Landing...

@maclover7 maclover7 self-assigned this Dec 25, 2017
@maclover7 maclover7 added the async_hooks Issues and PRs related to the async hooks subsystem. label Dec 25, 2017
maclover7 added a commit to maclover7/node that referenced this pull request Dec 25, 2017
SetupHooks is only available via `process.binding('async_wrap')`, so
there's no reason it shouldn't be called with the appropriate arguments,
since it is an internal-only function. The only place this function is
used is `lib/internal/async_hooks.js`.

PR-URL: nodejs#17832
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@maclover7
Copy link
Contributor Author

Landed in 81c2b59

@maclover7 maclover7 closed this Dec 25, 2017
@maclover7 maclover7 deleted the jm-async-erors branch December 25, 2017 03:00
MylesBorins pushed a commit that referenced this pull request Jan 8, 2018
SetupHooks is only available via `process.binding('async_wrap')`, so
there's no reason it shouldn't be called with the appropriate arguments,
since it is an internal-only function. The only place this function is
used is `lib/internal/async_hooks.js`.

PR-URL: #17832
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
SetupHooks is only available via `process.binding('async_wrap')`, so
there's no reason it shouldn't be called with the appropriate arguments,
since it is an internal-only function. The only place this function is
used is `lib/internal/async_hooks.js`.

PR-URL: #17832
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
SetupHooks is only available via `process.binding('async_wrap')`, so
there's no reason it shouldn't be called with the appropriate arguments,
since it is an internal-only function. The only place this function is
used is `lib/internal/async_hooks.js`.

PR-URL: #17832
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 10, 2018
gibfahn pushed a commit to gibfahn/node that referenced this pull request Jan 17, 2018
SetupHooks is only available via `process.binding('async_wrap')`, so
there's no reason it shouldn't be called with the appropriate arguments,
since it is an internal-only function. The only place this function is
used is `lib/internal/async_hooks.js`.

PR-URL: nodejs#17832
Backport-PR-URL: nodejs#18179
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
SetupHooks is only available via `process.binding('async_wrap')`, so
there's no reason it shouldn't be called with the appropriate arguments,
since it is an internal-only function. The only place this function is
used is `lib/internal/async_hooks.js`.

Backport-PR-URL: #18179
PR-URL: #17832
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
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. c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants