diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index d854d737dcec20..4813169bfd8179 100644 --- a/doc/api/async_hooks.md +++ b/doc/api/async_hooks.md @@ -599,10 +599,6 @@ own resources. The `init` hook will trigger when an `AsyncResource` is instantiated. -The `before` and `after` calls must be unwound in the same order that they -are called. Otherwise, an unrecoverable exception will occur and the process -will abort. - The following is an overview of the `AsyncResource` API. ```js @@ -615,11 +611,13 @@ const asyncResource = new AsyncResource( type, { triggerAsyncId: executionAsyncId(), requireManualDestroy: false } ); -// Call AsyncHooks before callbacks. -asyncResource.emitBefore(); - -// Call AsyncHooks after callbacks. -asyncResource.emitAfter(); +// Run a function in the execution context of the resource. This will +// * establish the context of the resource +// * trigger the AsyncHooks before callbacks +// * call the provided function `fn` with the supplied arguments +// * trigger the AsyncHooks after callbacks +// * restore the original execution context +asyncResource.runInAsyncScope(fn, thisArg, ...args); // Call AsyncHooks destroy callbacks. asyncResource.emitDestroy(); @@ -629,6 +627,14 @@ asyncResource.asyncId(); // Return the trigger ID for the AsyncResource instance. asyncResource.triggerAsyncId(); + +// Call AsyncHooks before callbacks. +// Deprecated: Use asyncResource.runInAsyncScope instead. +asyncResource.emitBefore(); + +// Call AsyncHooks after callbacks. +// Deprecated: Use asyncResource.runInAsyncScope instead. +asyncResource.emitAfter(); ``` #### `AsyncResource(type[, options])` @@ -654,9 +660,7 @@ class DBQuery extends AsyncResource { getInfo(query, callback) { this.db.get(query, (err, data) => { - this.emitBefore(); - callback(err, data); - this.emitAfter(); + this.runInAsyncScope(callback, null, err, data); }); } @@ -667,7 +671,26 @@ class DBQuery extends AsyncResource { } ``` +#### `asyncResource.runInAsyncScope(fn[, thisArg, ...args])` + + +* `fn` {Function} The function to call in the execution context of this async + resource. +* `thisArg` {any} The receiver to be used for the function call. +* `...args` {any} Optional arguments to pass to the function. + +Call the provided function with the provided arguments in the execution context +of the async resource. This will establish the context, trigger the AsyncHooks +before callbacks, call the function, trigger the AsyncHooks after callbacks, and +then restore the original execution context. + #### `asyncResource.emitBefore()` + +> Stability: 0 - Deprecated: Use [`asyncResource.runInAsyncScope()`][] instead. * Returns: {undefined} @@ -675,7 +698,17 @@ 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. +`before` and `after` calls must be unwound in the same order that they +are called. Otherwise, an unrecoverable exception will occur and the process +will abort. For this reason, the `emitBefore` and `emitAfter` APIs are +considered deprecated. Please use `runInAsyncScope`, as it provides a much safer +alternative. + #### `asyncResource.emitAfter()` + +> Stability: 0 - Deprecated: Use [`asyncResource.runInAsyncScope()`][] instead. * Returns: {undefined} @@ -686,6 +719,12 @@ 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. +`before` and `after` calls must be unwound in the same order that they +are called. Otherwise, an unrecoverable exception will occur and the process +will abort. For this reason, the `emitBefore` and `emitAfter` APIs are +considered deprecated. Please use `runInAsyncScope`, as it provides a much safer +alternative. + #### `asyncResource.emitDestroy()` * Returns: {undefined} @@ -705,6 +744,7 @@ never be called. constructor. [`after` callback]: #async_hooks_after_asyncid +[`asyncResource.runInAsyncScope()`]: #async_hooks_asyncresource_runinasyncscope_fn_thisarg_args [`before` callback]: #async_hooks_before_asyncid [`destroy` callback]: #async_hooks_destroy_asyncid [`init` callback]: #async_hooks_init_asyncid_type_triggerasyncid_resource diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 1f5809624afe95..c18c9d58e0b521 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -880,6 +880,18 @@ Users of `MakeCallback` that add the `domain` property to carry context, should start using the `async_context` variant of `MakeCallback` or `CallbackScope`, or the the high-level `AsyncResource` class. + +### DEP0098: AsyncHooks Embedder AsyncResource.emit{Before,After} APIs + +Type: Runtime + +The embedded API provided by AsyncHooks exposes emit{Before,After} methods +which are very easy to use incorrectly which can lead to unrecoverable errors. + +Use [`asyncResource.runInAsyncScope()`][] API instead which provides a much +safer, and more convenient, alternative. See +https://github.com/nodejs/node/pull/18513 for more details. + [`--pending-deprecation`]: cli.html#cli_pending_deprecation [`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size [`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array @@ -892,6 +904,7 @@ should start using the `async_context` variant of `MakeCallback` or [`Server.getConnections()`]: net.html#net_server_getconnections_callback [`Server.listen({fd: })`]: net.html#net_server_listen_handle_backlog_callback [`SlowBuffer`]: buffer.html#buffer_class_slowbuffer +[`asyncResource.runInAsyncScope()`]: async_hooks.html#async_hooks_asyncresource_runinasyncscope_fn_thisarg_args [`child_process`]: child_process.html [`console.error()`]: console.html#console_console_error_data_args [`console.log()`]: console.html#console_console_log_data_args diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 55a16a569e908b..e7450f29acc05f 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -139,6 +139,17 @@ function triggerAsyncId() { const destroyedSymbol = Symbol('destroyed'); +let emitBeforeAfterWarning = true; +function showEmitBeforeAfterWarning() { + if (emitBeforeAfterWarning) { + process.emitWarning( + 'asyncResource.emitBefore and emitAfter are deprecated. Please use ' + + 'asyncResource.runInAsyncScope instead', + 'DeprecationWarning', 'DEP00XX'); + emitBeforeAfterWarning = false; + } +} + class AsyncResource { constructor(type, opts = {}) { if (typeof type !== 'string') @@ -174,15 +185,28 @@ class AsyncResource { } emitBefore() { + showEmitBeforeAfterWarning(); emitBefore(this[async_id_symbol], this[trigger_async_id_symbol]); return this; } emitAfter() { + showEmitBeforeAfterWarning(); emitAfter(this[async_id_symbol]); return this; } + runInAsyncScope(fn, thisArg, ...args) { + emitBefore(this[async_id_symbol], this[trigger_async_id_symbol]); + let ret; + try { + ret = Reflect.apply(fn, thisArg, args); + } finally { + emitAfter(this[async_id_symbol]); + } + return ret; + } + emitDestroy() { this[destroyedSymbol].destroyed = true; emitDestroy(this[async_id_symbol]); diff --git a/test/async-hooks/test-embedder.api.async-resource.runInAsyncScope.js b/test/async-hooks/test-embedder.api.async-resource.runInAsyncScope.js new file mode 100644 index 00000000000000..627880b4d98b07 --- /dev/null +++ b/test/async-hooks/test-embedder.api.async-resource.runInAsyncScope.js @@ -0,0 +1,11 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const async_hooks = require('async_hooks'); + +// Ensure that asyncResource.makeCallback returns the callback return value. +const a = new async_hooks.AsyncResource('foobar'); +const ret = a.runInAsyncScope(() => { + return 1729; +}); +assert.strictEqual(ret, 1729); diff --git a/test/parallel/test-async-hooks-recursive-stack-runInAsyncScope.js b/test/parallel/test-async-hooks-recursive-stack-runInAsyncScope.js new file mode 100644 index 00000000000000..bc4ac86e7f1ca1 --- /dev/null +++ b/test/parallel/test-async-hooks-recursive-stack-runInAsyncScope.js @@ -0,0 +1,20 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const async_hooks = require('async_hooks'); + +// This test verifies that the async ID stack can grow indefinitely. + +function recurse(n) { + const a = new async_hooks.AsyncResource('foobar'); + a.runInAsyncScope(() => { + assert.strictEqual(a.asyncId(), async_hooks.executionAsyncId()); + assert.strictEqual(a.triggerAsyncId(), async_hooks.triggerAsyncId()); + if (n >= 0) + recurse(n - 1); + assert.strictEqual(a.asyncId(), async_hooks.executionAsyncId()); + assert.strictEqual(a.triggerAsyncId(), async_hooks.triggerAsyncId()); + }); +} + +recurse(1000); diff --git a/test/parallel/test-emit-after-uncaught-exception-runInAsyncScope.js b/test/parallel/test-emit-after-uncaught-exception-runInAsyncScope.js new file mode 100644 index 00000000000000..5003972e9984aa --- /dev/null +++ b/test/parallel/test-emit-after-uncaught-exception-runInAsyncScope.js @@ -0,0 +1,40 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const async_hooks = require('async_hooks'); + +const id_obj = {}; +let collect = true; + +const hook = async_hooks.createHook({ + before(id) { if (collect) id_obj[id] = true; }, + after(id) { delete id_obj[id]; }, +}).enable(); + +process.once('uncaughtException', common.mustCall((er) => { + assert.strictEqual(er.message, 'bye'); + collect = false; +})); + +setImmediate(common.mustCall(() => { + process.nextTick(common.mustCall(() => { + assert.strictEqual(Object.keys(id_obj).length, 0); + hook.disable(); + })); + + // Create a stack of async ids that will need to be emitted in the case of + // an uncaught exception. + const ar1 = new async_hooks.AsyncResource('Mine'); + ar1.runInAsyncScope(() => { + const ar2 = new async_hooks.AsyncResource('Mine'); + ar2.runInAsyncScope(() => { + throw new Error('bye'); + }); + }); + + // TODO(trevnorris): This test shows that the after() hooks are always called + // correctly, but it doesn't solve where the emitDestroy() is missed because + // of the uncaught exception. Simple solution is to always call emitDestroy() + // before the emitAfter(), but how to codify this? +}));