async_hooks,doc: some async_hooks improvements

Update docs and type checking for AsyncResource type

PR-URL: https://github.com/nodejs/node/pull/15103
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

doc/api/async_hooks.md

@@ -73,7 +73,11 @@ function destroy(asyncId) { }
 added: REPLACEME
 -->
 
-* `callbacks` {Object} the callbacks to register
+* `callbacks` {Object} the [Hook Callbacks][] to register
+  * `init` {Function} The [`init` callback][].
+  * `before` {Function} The [`before` callback][].
+  * `after` {Function} The [`after` callback][].
+  * `destroy` {Function} The [`destroy` callback][].
 * Returns: `{AsyncHook}` instance used for disabling and enabling hooks
 
 Registers functions to be called for different lifetime events of each async
@@ -87,6 +91,31 @@ be tracked then only the `destroy` callback needs to be passed. The
 specifics of all functions that can be passed to `callbacks` is in the section
 [`Hook Callbacks`][].
 
+```js
+const async_hooks = require('async_hooks');
+
+const asyncHook = async_hooks.createHook({
+  init(asyncId, type, triggerAsyncId, resource) { },
+  destroy(asyncId) { }
+});
+```
+
+Note that the callbacks will be inherited via the prototype chain:
+
+```js
+class MyAsyncCallbacks {
+  init(asyncId, type, triggerAsyncId, resource) { }
+  destroy(asyncId) {}
+}
+
+class MyAddedCallbacks extends MyAsyncCallbacks {
+  before(asyncId) { }
+  after(asyncId) { }
+}
+
+const asyncHook = async_hooks.createHook(new MyAddedCallbacks());
+```
+
 ##### Error Handling
 
 If any `AsyncHook` callbacks throw, the application will print the stack trace
@@ -187,11 +216,12 @@ require('net').createServer().listen(function() { this.close(); });
 clearTimeout(setTimeout(() => {}, 10));
 ```
 
-Every new resource is assigned a unique ID.
+Every new resource is assigned an ID that is unique within the scope of the
+current process.
 
 ###### `type`
 
-The `type` is a string that represents the type of resource that caused
+The `type` is a string identifying the type of resource that caused
 `init` to be called. Generally, it will correspond to the name of the
 resource's constructor.
 
@@ -214,8 +244,8 @@ when listening to the hooks.
 
 ###### `triggerId`
 
-`triggerAsyncId` is the `asyncId` of the resource that caused (or "triggered") the
-new resource to initialize and that caused `init` to call. This is different
+`triggerAsyncId` is the `asyncId` of the resource that caused (or "triggered")
+the new resource to initialize and that caused `init` to call. This is different
 from `async_hooks.executionAsyncId()` that only shows *when* a resource was
 created, while `triggerAsyncId` shows *why* a resource was created.
 
@@ -253,26 +283,27 @@ propagating what resource is responsible for the new resource's existence.
 
 ###### `resource`
 
-`resource` is an object that represents the actual resource. This can contain
-useful information such as the hostname for the `GETADDRINFOREQWRAP` resource
-type, which will be used when looking up the ip for the hostname in
-`net.Server.listen`. The API for getting this information is currently not
-considered public, but using the Embedder API users can provide and document
-their own resource objects. Such as resource object could for example contain
-the SQL query being executed.
+`resource` is an object that represents the actual async resource that has
+been initialized. This can contain useful information that can vary based on
+the value of `type`. For instance, for the `GETADDRINFOREQWRAP` resource type,
+`resource` provides the hostname used when looking up the IP address for the
+hostname in `net.Server.listen()`. The API for accessing this information is
+currently not considered public, but using the Embedder API, users can provide
+and document their own resource objects. Such a resource object could for
+example contain the SQL query being executed.
 
 In the case of Promises, the `resource` object will have `promise` property
 that refers to the Promise that is being initialized, and a `parentId` property
-that equals the `asyncId` of a parent Promise, if there is one, and
-`undefined` otherwise. For example, in the case of `b = a.then(handler)`,
-`a` is considered a parent Promise of `b`.
+set to the `asyncId` of a parent Promise, if there is one, and `undefined`
+otherwise. For example, in the case of `b = a.then(handler)`, `a` is considered
+a parent Promise of `b`.
 
 *Note*: In some cases the resource object is reused for performance reasons,
 it is thus not safe to use it as a key in a `WeakMap` or add properties to it.
 
-###### asynchronous context example
+###### Asynchronous context example
 
-Below is another example with additional information about the calls to
+The following is an example with additional information about the calls to
 `init` between the `before` and `after` calls, specifically what the
 callback to `listen()` will look like. The output formatting is slightly more
 elaborate to make calling context easier to see.
@@ -348,10 +379,10 @@ Only using `execution` to graph resource allocation results in the following:
 TTYWRAP(6) -> Timeout(4) -> TIMERWRAP(5) -> TickObject(3) -> root(1)
 ```
 
-The `TCPWRAP` isn't part of this graph; even though it was the reason for
+The `TCPWRAP` is not part of this graph; even though it was the reason for
 `console.log()` being called. This is because binding to a port without a
-hostname is actually synchronous, but to maintain a completely asynchronous API
-the user's callback is placed in a `process.nextTick()`.
+hostname is a *synchronous* operation, but to maintain a completely asynchronous
+API the user's callback is placed in a `process.nextTick()`.
 
 The graph only shows *when* a resource was created, not *why*, so to track
 the *why* use `triggerAsyncId`.
@@ -369,9 +400,10 @@ resource about to execute the callback.
 
 The `before` callback will be called 0 to N times. The `before` callback
 will typically be called 0 times if the asynchronous operation was cancelled
-or for example if no connections are received by a TCP server. Asynchronous
-like the TCP server will typically call the `before` callback multiple times,
-while other operations like `fs.open()` will only call it once.
+or, for example, if no connections are received by a TCP server. Persistent
+asynchronous resources like a TCP server will typically call the `before`
+callback multiple times, while other operations like `fs.open()` will only call
+it only once.
 
 
 ##### `after(asyncId)`
@@ -381,7 +413,7 @@ while other operations like `fs.open()` will only call it once.
 Called immediately after the callback specified in `before` is completed.
 
 *Note:* If an uncaught exception occurs during execution of the callback then
-`after` will run after the `'uncaughtException'` event is emitted or a
+`after` will run *after* the `'uncaughtException'` event is emitted or a
 `domain`'s handler runs.
 
 
@@ -389,22 +421,25 @@ Called immediately after the callback specified in `before` is completed.
 
 * `asyncId` {number}
 
-Called after the resource corresponding to `asyncId` is destroyed. It is also called
-asynchronously from the embedder API `emitDestroy()`.
+Called after the resource corresponding to `asyncId` is destroyed. It is also
+called asynchronously from the embedder API `emitDestroy()`.
 
-*Note:* Some resources depend on GC for cleanup, so if a reference is made to
-the `resource` object passed to `init` it's possible that `destroy` is
-never called, causing a memory leak in the application. Of course if
-the resource doesn't depend on GC then this isn't an issue.
+*Note:* Some resources depend on garbage collection for cleanup, so if a
+reference is made to the `resource` object passed to `init` it is possible that
+`destroy` will never be called, causing a memory leak in the application. If
+the resource does not depend on garbage collection, then this will not be an
+issue.
 
 #### `async_hooks.executionAsyncId()`
 
-* Returns {number} the `asyncId` of the current execution context. Useful to track
-  when something calls.
+* Returns {number} the `asyncId` of the current execution context. Useful to
+  track when something calls.
 
 For example:
 
 ```js
+const async_hooks = require('async_hooks');
+
 console.log(async_hooks.executionAsyncId());  // 1 - bootstrap
 fs.open(path, 'r', (err, fd) => {
   console.log(async_hooks.executionAsyncId());  // 6 - open()
@@ -453,10 +488,9 @@ const server = net.createServer((conn) => {
 
 ## JavaScript Embedder API
 
-Library developers that handle their own I/O, a connection pool, or
-callback queues will need to hook into the AsyncWrap API so that all the
-appropriate callbacks are called. To accommodate this a JavaScript API is
-provided.
+Library developers that handle their own asychronous resources performing tasks
+like I/O, connection pooling, or managing callback queues may use the `AsyncWrap`
+JavaScript API so that all the appropriate callbacks are called.
 
 ### `class AsyncResource()`
 
@@ -466,9 +500,9 @@ own resources.
 
 The `init` hook will trigger when an `AsyncResource` is instantiated.
 
-It is important that `before`/`after` calls are unwound
+*Note*: It is important that `before`/`after` calls are unwound
 in the same order they are called. Otherwise an unrecoverable exception
-will occur and node will abort.
+will occur and the process will abort.
 
 The following is an overview of the `AsyncResource` API.
 
@@ -499,9 +533,9 @@ asyncResource.triggerAsyncId();
 #### `AsyncResource(type[, triggerAsyncId])`
 
 * arguments
-  * `type` {string} the type of ascyc event
-  * `triggerAsyncId` {number} the ID of the execution context that created this async
-    event
+  * `type` {string} the type of async event
+  * `triggerAsyncId` {number} the ID of the execution context that created this
+    async event
 
 Example usage:
 
@@ -531,9 +565,9 @@ class DBQuery extends AsyncResource {
 
 * Returns {undefined}
 
-Call all `before` callbacks and let them know a new asynchronous execution
-context is being entered. If nested calls to `emitBefore()` are made, the stack
-of `asyncId`s will be tracked and properly unwound.
+Call all `before` callbacks to notify that a new asynchronous execution context
+is being entered. If nested calls to `emitBefore()` are made, the stack of
+`asyncId`s will be tracked and properly unwound.
 
 #### `asyncResource.emitAfter()`
 
@@ -542,9 +576,9 @@ of `asyncId`s will be tracked and properly unwound.
 Call all `after` callbacks. If nested calls to `emitBefore()` were made, then
 make sure the stack is unwound properly. Otherwise an error will be thrown.
 
-If the user's callback throws an exception then `emitAfter()` will
-automatically be called for all `asyncId`s on the stack if the error is handled by
-a domain or `'uncaughtException'` handler.
+If the user's callback throws an exception, `emitAfter()` will automatically be
+called for all `asyncId`s on the stack if the error is handled by a domain or
+`'uncaughtException'` handler.
 
 #### `asyncResource.emitDestroy()`
 
@@ -564,4 +598,8 @@ never be called.
 * Returns {number} the same `triggerAsyncId` that is passed to the `AsyncResource`
 constructor.
 
+[`after` callback]: #async_hooks_after_asyncid
+[`before` callback]: #async_hooks_before_asyncid
+[`destroy` callback]: #async_hooks_before_asyncid
 [`Hook Callbacks`]: #async_hooks_hook_callbacks
+[`init` callback]: #async_hooks_init_asyncid_type_triggerasyncid_resource

lib/async_hooks.js

@@ -234,6 +234,9 @@ function triggerAsyncId() {
 
 class AsyncResource {
   constructor(type, triggerAsyncId = initTriggerId()) {
+    if (typeof type !== 'string')
+      throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'type', 'string');
+
     // Unlike emitInitScript, AsyncResource doesn't supports null as the
     // triggerAsyncId.
     if (!Number.isSafeInteger(triggerAsyncId) || triggerAsyncId < -1) {

test/async-hooks/test-embedder.api.async-resource-no-type.js

@@ -0,0 +1,34 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const async_hooks = require('async_hooks');
+const { AsyncResource } = async_hooks;
+const { spawn } = require('child_process');
+
+const initHooks = require('./init-hooks');
+
+if (process.argv[2] === 'child') {
+  initHooks().enable();
+
+  class Foo extends AsyncResource {
+    constructor(type) {
+      super(type, async_hooks.executionAsyncId());
+    }
+  }
+
+  [null, undefined, 1, Date, {}, []].forEach((i) => {
+    common.expectsError(() => new Foo(i), {
+      code: 'ERR_INVALID_ARG_TYPE',
+      type: TypeError
+    });
+  });
+
+} else {
+  const args = process.argv.slice(1).concat('child');
+  spawn(process.execPath, args)
+    .on('close', common.mustCall((code) => {
+      // No error because the type was defaulted
+      assert.strictEqual(code, 0);
+    }));
+}

test/async-hooks/test-embedder.api.async-resource.js

@@ -12,12 +12,11 @@ const { checkInvocations } = require('./hook-checks');
 const hooks = initHooks();
 hooks.enable();
 
-assert.throws(() => {
-  new AsyncResource();
-}, common.expectsError({
-  code: 'ERR_ASYNC_TYPE',
-  type: TypeError,
-}));
+common.expectsError(
+  () => new AsyncResource(), {
+    code: 'ERR_INVALID_ARG_TYPE',
+    type: TypeError,
+  });
 assert.throws(() => {
   new AsyncResource('invalid_trigger_id', null);
 }, common.expectsError({

test/parallel/test-async-hooks-asyncresource-constructor.js

@@ -15,7 +15,7 @@ async_hooks.createHook({
 assert.throws(() => {
   return new AsyncResource();
 }, common.expectsError({
-  code: 'ERR_ASYNC_TYPE',
+  code: 'ERR_INVALID_ARG_TYPE',
   type: TypeError,
 }));