async_hooks: deprecate undocumented API #16972

AndreasMadsen · 2017-11-12T17:50:17Z

This deprecates the undocumented async_hooks API and moves the internal parts to internal/async_hooks.js. Since the API is undocumented async_hooks is experimental, I think it is fine to deprecate them in semver-minor and remove them in semver-major.

I'm not sure if we should land this on v8.x, thoughts?

Ref: #14328
Ref: #15572

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

Affected core subsystem(s)

async_hooks

AndreasMadsen · 2017-11-12T17:51:03Z

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

/cc @nodejs/async_hooks

vsemozhetbyt · 2017-11-12T18:03:46Z

doc/api/deprecations.md

@@ -747,6 +747,24 @@ be set to `false` to disable ECDH entirely on the server only. This mode is
 deprecated in preparation for migrating to OpenSSL 1.1.0 and consistency with
 the client. Use the `ciphers` parameter instead.

+<a id="DEP0084"></a>


A nit: should this be DEP00XX at this stage?

vsemozhetbyt · 2017-11-12T18:04:00Z

doc/api/deprecations.md

+API instead.
+
+
+<a id="DEP0085"></a>


vsemozhetbyt · 2017-11-12T18:04:36Z

doc/api/deprecations.md

+
+Type: Runtime
+
+`runInAsyncIdScope` doesn't emit the before or after event and can thus cause


A nit: before or after -> 'before' or 'after' ?

jasnell · 2017-11-12T18:15:07Z

doc/api/deprecations.md

@@ -747,6 +747,24 @@ be set to `false` to disable ECDH entirely on the server only. This mode is
 deprecated in preparation for migrating to OpenSSL 1.1.0 and consistency with
 the client. Use the `ciphers` parameter instead.

+<a id="DEP0084"></a>


These should use the form DEP00XX until the PR lands.

AndreasMadsen · 2017-11-14T08:43:40Z

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

AndreasMadsen · 2017-11-14T11:20:52Z

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

AndreasMadsen · 2017-11-16T10:56:07Z

Can I get some LGTMs? /cc @nodejs/async_hooks

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

addaleax

LGTM, with the assumption that the “moving code around” bits were really not doing more than moving code around

addaleax · 2017-11-16T11:35:53Z

I'm not sure if we should land this on v8.x, thoughts?

I think that would be a good idea.

AndreasMadsen · 2017-11-16T11:55:50Z

LGTM, with the assumption that the “moving code around” bits were really not doing more than moving code around

I hope so :)

addaleax · 2017-11-18T19:55:44Z

@AndreasMadsen This needs a rebase :)

AndreasMadsen · 2017-11-20T09:28:14Z

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

AndreasMadsen · 2017-11-20T11:04:28Z

Landed in 07a4fa3

MylesBorins · 2017-12-11T22:23:56Z

This is not landing cleanly on v9.x as there are deprecations on master that are not in v9.x

Are we good landing this in a non semver-major release? AFAIK async_hooks are experimental so it shouldn't be an issue (theoretically not even semver-minor).

If we do backport I'm assume it should come with the exact same error code. @jasnell any advice?

jasnell · 2017-12-11T22:25:52Z

Sgtm.

PR-URL: #16972 Refs: #14328 Refs: #15572 Reviewed-By: Anna Henningsen <[email protected]>

Notable change: * async_hooks: - deprecate AsyncHooks Sensitive API and runInAsyncIdScope. Neither API were documented. (Andreas Madsen) #16972 * deps: - update nghttp2 to 1.29.0 (James M Snell) #17908 - upgrade npm to 5.6.0 (Kat Marchán) #17535 - cherry-pick 50f7455 from upstream V8 (Michaël Zasso) #16591 * events: - remove reaches into _events internals (Anatoli Papirovski) #17440 * http: - add rawPacket in err of `clientError` event (XadillaX) #17672 * http2: - implement maxSessionMemory (James M Snell) #17967 - add initial support for originSet (James M Snell) #17935 - add altsvc support (James M Snell) #17917 - perf_hooks integration (James M Snell) #17906 * net: - remove Socket.prototype.write (Anna Henningsen) #17644 - remove Socket.prototype.listen (Ruben Bridgewater) #13735 * repl: - show lexically scoped vars in tab completion (Michaël Zasso) #16591 * stream: - rm {writeable/readable}State.length (Calvin Metcalf) #12857 - add flow and buffer properties to streams (Calvin Metcalf) #12855 * util: - allow wildcards in NODE_DEBUG variable (Tyler) #17609 * zlib: - add ArrayBuffer support (Jem Bezooyen) #16042 * Addedew collaborator** - [starkwang](https://github.com/starkwang) Weijia Wang * Addedew TSC member** - [danbev](https://github.com/danbev) Daniel Bevenius PR-URL: #18069

Notable change: * async_hooks: - deprecate AsyncHooks Sensitive API and runInAsyncIdScope. Neither API were documented. (Andreas Madsen) #16972 * deps: - update nghttp2 to 1.29.0 (James M Snell) #17908 - upgrade npm to 5.6.0 (Kat Marchán) #17535 - cherry-pick 50f7455 from upstream V8 (Michaël Zasso) #16591 * events: - remove reaches into _events internals (Anatoli Papirovski) #17440 * http: - add rawPacket in err of `clientError` event (XadillaX) #17672 * http2: - implement maxSessionMemory (James M Snell) #17967 - add initial support for originSet (James M Snell) #17935 - add altsvc support (James M Snell) #17917 - perf_hooks integration (James M Snell) #17906 - Refactoring and cleanup of Http2Session and Http2Stream destroy (James M Snell) #17406 * net: - remove Socket.prototype.write (Anna Henningsen) #17644 - remove Socket.prototype.listen (Ruben Bridgewater) #13735 * repl: - show lexically scoped vars in tab completion (Michaël Zasso) #16591 * stream: - rm {writeable/readable}State.length (Calvin Metcalf) #12857 - add flow and buffer properties to streams (Calvin Metcalf) #12855 * util: - allow wildcards in NODE_DEBUG variable (Tyler) #17609 * zlib: - add ArrayBuffer support (Jem Bezooyen) #16042 * Addedew collaborator** - [starkwang](https://github.com/starkwang) Weijia Wang * Addedew TSC member** - [danbev](https://github.com/danbev) Daniel Bevenius PR-URL: #18069

PR-URL: nodejs#16972 Backport-PR-URL: nodejs#18179 Refs: nodejs#14328 Refs: nodejs#15572 Reviewed-By: Anna Henningsen <[email protected]>

Backport-PR-URL: #18179 PR-URL: #16972 Refs: #14328 Refs: #15572 Reviewed-By: Anna Henningsen <[email protected]>

Notable changes: * deps: * update V8 to 6.2.414.46 (Michaël Zasso) [#16413](#16413) * revert ABI breaking changes in V8 6.2 (Anna Henningsen) [#16413](#16413) * upgrade libuv to 1.19.1 (cjihrig) [#18260](#18260) * re land npm 5.6.0 (Myles Borins) [#18625](#18625) * ICU 60 bump (Steven R. Loomis) [#16876](#16876) * crypto: * Support both OpenSSL 1.1.0 and 1.0.2 (David Benjamin) [#16130](#16130) * warn on invalid authentication tag length (Tobias Nießen) [#17566](#17566) * async_hooks: * update defaultTriggerAsyncIdScope for perf (Anatoli Papirovski) [#18004](#18004) * use typed array stack as fast path (Anna Henningsen) [#17780](#17780) * use scope for defaultTriggerAsyncId (Andreas Madsen) [#17273](#17273) * separate missing from default context (Andreas Madsen) [#17273](#17273) * rename initTriggerId (Andreas Madsen) [#17273](#17273) * deprecate undocumented API (Andreas Madsen) [#16972](#16972) * add destroy event for gced AsyncResources (Sebastian Mayr) [#16998](#16998) * add trace events to async_hooks (Andreas Madsen) [#15538](#15538) * set HTTPParser trigger to socket (Andreas Madsen) [#18003](#18003) * add provider types for net server (Andreas Madsen) [#17157](#17157) * n-api: * add helper for addons to get the event loop (Anna Henningsen) [#17109](#17109) * cli: * add --stack-trace-limit to NODE_OPTIONS (Anna Henningsen) [#16495](#16495) * console: * add support for console.debug (Benjamin Zaslavsky) [#17033](#17033) * module: * add builtinModules (Jon Moss) [#16386](#16386) * replace default paths in require.resolve() (cjihrig) [#17113](#17113) * src: * add helper for addons to get the event loop (Anna Henningsen) [#17109](#17109) * add process.ppid (cjihrig) [#16839](#16839) * http: * support generic `Duplex` streams (Anna Henningsen) [#16267](#16267) * add rawPacket in err of `clientError` event (XadillaX) [#17672](#17672) * better support for IPv6 addresses (Mattias Holmlund) [#14772](#14772) * net: * remove ADDRCONFIG DNS hint on Windows (Bartosz Sosnowski) [#17662](#17662) * process: * fix reading zero-length env vars on win32 (Anna Henningsen) [#18463](#18463) * tls: * unconsume stream on destroy (Anna Henningsen) [#17478](#17478) * process: * improve unhandled rejection message (Madara Uchiha) [#17158](#17158) * stream: * remove usage of *State.highWaterMark (Calvin Metcalf) [#12860](#12860) * trace_events: * add executionAsyncId to init events (Andreas Madsen) [#17196](#17196) PR-URL: #18336

Notable changes: * deps: * update V8 to 6.2.414.46 (Michaël Zasso) [nodejs#16413](nodejs#16413) * revert ABI breaking changes in V8 6.2 (Anna Henningsen) [nodejs#16413](nodejs#16413) * upgrade libuv to 1.19.1 (cjihrig) [nodejs#18260](nodejs#18260) * re land npm 5.6.0 (Myles Borins) [nodejs#18625](nodejs#18625) * ICU 60 bump (Steven R. Loomis) [nodejs#16876](nodejs#16876) * crypto: * Support both OpenSSL 1.1.0 and 1.0.2 (David Benjamin) [nodejs#16130](nodejs#16130) * warn on invalid authentication tag length (Tobias Nießen) [nodejs#17566](nodejs#17566) * async_hooks: * update defaultTriggerAsyncIdScope for perf (Anatoli Papirovski) [nodejs#18004](nodejs#18004) * use typed array stack as fast path (Anna Henningsen) [nodejs#17780](nodejs#17780) * use scope for defaultTriggerAsyncId (Andreas Madsen) [nodejs#17273](nodejs#17273) * separate missing from default context (Andreas Madsen) [nodejs#17273](nodejs#17273) * rename initTriggerId (Andreas Madsen) [nodejs#17273](nodejs#17273) * deprecate undocumented API (Andreas Madsen) [nodejs#16972](nodejs#16972) * add destroy event for gced AsyncResources (Sebastian Mayr) [nodejs#16998](nodejs#16998) * add trace events to async_hooks (Andreas Madsen) [nodejs#15538](nodejs#15538) * set HTTPParser trigger to socket (Andreas Madsen) [nodejs#18003](nodejs#18003) * add provider types for net server (Andreas Madsen) [nodejs#17157](nodejs#17157) * n-api: * add helper for addons to get the event loop (Anna Henningsen) [nodejs#17109](nodejs#17109) * cli: * add --stack-trace-limit to NODE_OPTIONS (Anna Henningsen) [nodejs#16495](nodejs#16495) * console: * add support for console.debug (Benjamin Zaslavsky) [nodejs#17033](nodejs#17033) * module: * add builtinModules (Jon Moss) [nodejs#16386](nodejs#16386) * replace default paths in require.resolve() (cjihrig) [nodejs#17113](nodejs#17113) * src: * add helper for addons to get the event loop (Anna Henningsen) [nodejs#17109](nodejs#17109) * add process.ppid (cjihrig) [nodejs#16839](nodejs#16839) * http: * support generic `Duplex` streams (Anna Henningsen) [nodejs#16267](nodejs#16267) * add rawPacket in err of `clientError` event (XadillaX) [nodejs#17672](nodejs#17672) * better support for IPv6 addresses (Mattias Holmlund) [nodejs#14772](nodejs#14772) * net: * remove ADDRCONFIG DNS hint on Windows (Bartosz Sosnowski) [nodejs#17662](nodejs#17662) * process: * fix reading zero-length env vars on win32 (Anna Henningsen) [nodejs#18463](nodejs#18463) * tls: * unconsume stream on destroy (Anna Henningsen) [nodejs#17478](nodejs#17478) * process: * improve unhandled rejection message (Madara Uchiha) [nodejs#17158](nodejs#17158) * stream: * remove usage of *State.highWaterMark (Calvin Metcalf) [nodejs#12860](nodejs#12860) * trace_events: * add executionAsyncId to init events (Andreas Madsen) [nodejs#17196](nodejs#17196) PR-URL: nodejs#18336

AndreasMadsen added async_hooks Issues and PRs related to the async hooks subsystem. semver-minor PRs that contain new features and should be released in the next minor version. labels Nov 12, 2017

nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Nov 12, 2017

AndreasMadsen added dont-land-on-v4.x labels Nov 12, 2017

vsemozhetbyt reviewed Nov 12, 2017

View reviewed changes

doc/api/deprecations.md Outdated

API instead.

<a id="DEP0085"></a>

Copy link

Contributor

vsemozhetbyt Nov 12, 2017

Choose a reason for hiding this comment

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

Ditto.

vsemozhetbyt reviewed Nov 12, 2017

View reviewed changes

jasnell reviewed Nov 12, 2017

View reviewed changes

addaleax approved these changes Nov 16, 2017

View reviewed changes

addaleax added the lts-watch-v8.x label Nov 16, 2017

async_hooks: deprecate undocumented API

34a470a

AndreasMadsen closed this Nov 20, 2017

This was referenced Nov 20, 2017

async_hooks: remove deprecated API #17147

Closed

[async_hooks] stable API - tracking issue nodejs/diagnostics#124

Closed

MylesBorins added the backport-requested-v9.x label Dec 11, 2017

AndreasMadsen added backport-requested-v8.x and removed lts-watch-v8.x labels Dec 11, 2017

MylesBorins pushed a commit that referenced this pull request Jan 8, 2018

async_hooks: deprecate undocumented API

23f4433

PR-URL: #16972 Refs: #14328 Refs: #15572 Reviewed-By: Anna Henningsen <[email protected]>

AndreasMadsen added backported-to-v9.x and removed backport-requested-v9.x labels Jan 8, 2018

MylesBorins mentioned this pull request Jan 10, 2018

v9.4.0 proposal #18069

Merged

yocontra mentioned this pull request Jan 10, 2018

async hooks deprecated :( naugtur/blocked-at#12

Closed

This was referenced Jan 10, 2018

Remove async_hooks setTrigger API #14238

Closed

Semver Minor Tracking nodejs/Release#298

Closed

Backport async hooks to v8.x #18179

Closed

gibfahn pushed a commit to gibfahn/node that referenced this pull request Jan 17, 2018

async_hooks: deprecate undocumented API

e066706

PR-URL: nodejs#16972 Backport-PR-URL: nodejs#18179 Refs: nodejs#14328 Refs: nodejs#15572 Reviewed-By: Anna Henningsen <[email protected]>

MylesBorins pushed a commit that referenced this pull request Jan 19, 2018

async_hooks: deprecate undocumented API

025b9f2

Backport-PR-URL: #18179 PR-URL: #16972 Refs: #14328 Refs: #15572 Reviewed-By: Anna Henningsen <[email protected]>

kissofire mentioned this pull request Mar 7, 2021

[Snyk] Fix for 1 vulnerabilities kissofire/node#28

Open

meghasfdc mentioned this pull request Mar 8, 2021

[Snyk] Fix for 1 vulnerabilities meghasfdc/node#28

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

async_hooks: deprecate undocumented API #16972

async_hooks: deprecate undocumented API #16972

AndreasMadsen commented Nov 12, 2017 •

edited

Loading

AndreasMadsen commented Nov 12, 2017

vsemozhetbyt Nov 12, 2017

vsemozhetbyt Nov 12, 2017

vsemozhetbyt Nov 12, 2017

jasnell Nov 12, 2017

AndreasMadsen commented Nov 14, 2017

AndreasMadsen commented Nov 14, 2017

AndreasMadsen commented Nov 16, 2017

addaleax left a comment

addaleax commented Nov 16, 2017

AndreasMadsen commented Nov 16, 2017 •

edited

Loading

addaleax commented Nov 18, 2017

AndreasMadsen commented Nov 20, 2017

AndreasMadsen commented Nov 20, 2017

MylesBorins commented Dec 11, 2017

jasnell commented Dec 11, 2017


		Type: Runtime

		`runInAsyncIdScope` doesn't emit the before or after event and can thus cause

async_hooks: deprecate undocumented API #16972

async_hooks: deprecate undocumented API #16972

Conversation

AndreasMadsen commented Nov 12, 2017 • edited Loading

Checklist

Affected core subsystem(s)

AndreasMadsen commented Nov 12, 2017

vsemozhetbyt Nov 12, 2017

Choose a reason for hiding this comment

vsemozhetbyt Nov 12, 2017

Choose a reason for hiding this comment

vsemozhetbyt Nov 12, 2017

Choose a reason for hiding this comment

jasnell Nov 12, 2017

Choose a reason for hiding this comment

AndreasMadsen commented Nov 14, 2017

AndreasMadsen commented Nov 14, 2017

AndreasMadsen commented Nov 16, 2017

addaleax left a comment

Choose a reason for hiding this comment

addaleax commented Nov 16, 2017

AndreasMadsen commented Nov 16, 2017 • edited Loading

addaleax commented Nov 18, 2017

AndreasMadsen commented Nov 20, 2017

AndreasMadsen commented Nov 20, 2017

MylesBorins commented Dec 11, 2017

jasnell commented Dec 11, 2017

AndreasMadsen commented Nov 12, 2017 •

edited

Loading

AndreasMadsen commented Nov 16, 2017 •

edited

Loading