Skip to content

Remove async_hooks runInAsyncIdScope API #14328

Closed
@AndreasMadsen

Description

@AndreasMadsen
  • 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:

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    async_hooksIssues and PRs related to the async hooks subsystem.discussIssues opened for discussions and feedbacks.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions