Remove async_hooks runInAsyncIdScope API · Issue #14328 · nodejs/node (original) (raw)

• Version: master (016d81c)
• Platform: all
• Subsystem: async_hooks

As I mentioned a few months ago I'm not comfortable exposing the low-level async_hooks JS API. In the documentation PR #12953 we agreed to keep the low-level JS API undocumented. Some time has now passed and I think we are in a better position to discuss this.

The low-level JS API is quite large, thus I would like to separate the discussion into:

• A discussion about setInitTriggerId and triggerIdScope. (Remove async_hooks setTrigger API #14238)
• A discussion about runInAsyncIdScope. (this)
• A discussion about newUid, emitInit, emitBefore, emitAfter and emitDestroy (Remove async_hooks Sensitive/low-level Embedder API #15572).

Note: There is some overlap between emitInit and setInitTriggerId in terms of initTriggerId. Hopefully, that won't be an issue.

Background

runInAsyncIdScope(asyncId, cb) creates a new scope with the asyncId as
the executionAsyncId and with the current executionAsyncId as
the triggerAsyncId. It does so without invoking the before and after hooks.

runInAsyncIdScope was not part of the original async_hooks EP but
was included in the async_hooks PR.

runInAsyncIdScope is not used anywhere in node-core and as such, it purpose
is not well documented. @trevnorris mentions it only a single time:

This could also be used by something like a database module, where multiple queries should be treated as their own async operations, but the actual operation is combined into a single query internally. This is the reason for async_hooks.runInAsyncIdScope():

// A pooled resource on top of a native resource class MyResource inherits async_hooks.AsyncResource { constructor() { super('MyResource'); } query(val, cb) { // query() will pool the actual request then call each callback with // the specific data it requested. nativeResource.query(val, (er, data) => { async_hooks.runInAsyncIdScope(this.asyncId(), () => cb(er, data)); }); } }

const mr = new MyResource(); mr.query(val, (er, data) => { // Now when init() fires for writeFile() it will have both the currentId and // triggerId of the JS instance. fs.writeFile(path, data.toString()); });

Although, later @trevnorris says it is a bad example.

Issues

• The primary issue is that emitBefore and emitAfter are not used, thus
  the before and after hooks are never invoked. This can be an issue if the user
  depends on the executionAsyncId() to match the asyncId in the before hook.

For example, in trace I at some point used:

let stack = [];

createHook({ before(asyncId) { stack.push(states.get(asyncId)); }, after(asyncId) { stack.pop(); } })

However, because of runInAsyncIdScope this is actually invalid code.

Solution

• We remove runInAsyncIdScope
• The user should use AsyncResource and emitBefore/emitAfter to change
  executionAsyncId() and triggerAsyncId().

Note: We may want to just deprecate the API in node 8 and remove it in a future version.

For example, the DB resource example should be implemented as:

// A pooled resource on top of a native resource class MyResource inherits async_hooks.AsyncResource { constructor() { super('MyResource'); } query(val, cb) { // query() will pool the actual request then call each callback with // the specific data it requested. nativeResource.query(val, (er, data) => { this.emitBefore(); cb(er, data); this.emitAfter(); }); } }

/cc @nodejs/async_hooks