From 68a5477dd7a8a89795c38cfa8f9e13a7ab15f649 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 8 Mar 2019 16:42:21 +0100 Subject: [PATCH] async_hooks: remove deprecated emitBefore and emitAfter AsyncResource.emitBefore and AsyncResource.emitAfter have been deprecated in https://github.com/nodejs/node/pull/18632. This PR removes it all. This commit also updates some embedder tests to use internal APIs. The conditions are still possible for Node.js core developers but not for end users. --- doc/api/async_hooks.md | 36 ------------------ doc/api/deprecations.md | 5 ++- lib/async_hooks.js | 23 ----------- test/async-hooks/test-callback-error.js | 2 +- .../test-embedder.api.async-resource.js | 24 ++++++------ ...yed.js => test-emit-after-on-destroyed.js} | 38 +++++++++++++------ ...ed.js => test-emit-before-on-destroyed.js} | 38 +++++++++++++------ ...proper-order.js => test-improper-order.js} | 35 ++++++++++++----- ...oper-unwind.js => test-improper-unwind.js} | 34 +++++++++++------ .../test-async-hooks-recursive-stack.js | 16 ++++---- .../test-emit-after-uncaught-exception.js | 12 +++--- 11 files changed, 130 insertions(+), 133 deletions(-) rename test/async-hooks/{test-embedder.api.async-resource.after-on-destroyed.js => test-emit-after-on-destroyed.js} (61%) rename test/async-hooks/{test-embedder.api.async-resource.before-on-destroyed.js => test-emit-before-on-destroyed.js} (61%) rename test/async-hooks/{test-embedder.api.async-resource.improper-order.js => test-improper-order.js} (63%) rename test/async-hooks/{test-embedder.api.async-resource.improper-unwind.js => test-improper-unwind.js} (65%) diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index fe4f87c9eff1b9..8878c03aa73b28 100644 --- a/doc/api/async_hooks.md +++ b/doc/api/async_hooks.md @@ -659,41 +659,6 @@ 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. - -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. - -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, `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: {AsyncResource} A reference to `asyncResource`. @@ -713,7 +678,6 @@ never be called. `AsyncResource` 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 dbd9e8648c612b..0cc60aee30ba02 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -1905,6 +1905,9 @@ should start using the `async_context` variant of `MakeCallback` or ### DEP0098: AsyncHooks Embedder AsyncResource.emitBefore and AsyncResource.emitAfter APIs -Type: Runtime +Type: End-of-Life The embedded API provided by AsyncHooks exposes `.emitBefore()` and `.emitAfter()` methods which are very easy to use incorrectly which can lead diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 7ba5f40f18ebc4..107c92c97d670c 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -127,17 +127,6 @@ function createHook(fns) { 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', 'DEP0098'); - emitBeforeAfterWarning = false; - } -} - class AsyncResource { constructor(type, opts = {}) { validateString(type, 'type'); @@ -169,18 +158,6 @@ 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; diff --git a/test/async-hooks/test-callback-error.js b/test/async-hooks/test-callback-error.js index 07293e0315931c..06d8cb7224ccad 100644 --- a/test/async-hooks/test-callback-error.js +++ b/test/async-hooks/test-callback-error.js @@ -19,7 +19,7 @@ switch (arg) { onbefore: common.mustCall(() => { throw new Error(arg); }) }).enable(); const resource = new async_hooks.AsyncResource(`${arg}_type`); - resource.emitBefore(); + resource.runInAsyncScope(() => {}); return; case 'test_callback_abort': diff --git a/test/async-hooks/test-embedder.api.async-resource.js b/test/async-hooks/test-embedder.api.async-resource.js index 74d6c478c835bb..19c1b7187a9e2c 100644 --- a/test/async-hooks/test-embedder.api.async-resource.js +++ b/test/async-hooks/test-embedder.api.async-resource.js @@ -47,16 +47,16 @@ assert.strictEqual(typeof alcaEvent.asyncId(), 'number'); assert.notStrictEqual(alcaEvent.asyncId(), alcaTriggerId); assert.strictEqual(alcaEvent.triggerAsyncId(), alcaTriggerId); -alcaEvent.emitBefore(); -checkInvocations(alcazares, { init: 1, before: 1 }, - 'alcazares emitted before'); -alcaEvent.emitAfter(); +alcaEvent.runInAsyncScope(() => { + checkInvocations(alcazares, { init: 1, before: 1 }, + 'alcazares emitted before'); +}); checkInvocations(alcazares, { init: 1, before: 1, after: 1 }, 'alcazares emitted after'); -alcaEvent.emitBefore(); -checkInvocations(alcazares, { init: 1, before: 2, after: 1 }, - 'alcazares emitted before again'); -alcaEvent.emitAfter(); +alcaEvent.runInAsyncScope(() => { + checkInvocations(alcazares, { init: 1, before: 2, after: 1 }, + 'alcazares emitted before again'); +}); checkInvocations(alcazares, { init: 1, before: 2, after: 2 }, 'alcazares emitted after again'); alcaEvent.emitDestroy(); @@ -75,11 +75,11 @@ function tick1() { assert.strictEqual(typeof poblado.uid, 'number'); assert.strictEqual(poblado.triggerAsyncId, pobTriggerId); checkInvocations(poblado, { init: 1 }, 'poblado constructed'); - pobEvent.emitBefore(); - checkInvocations(poblado, { init: 1, before: 1 }, - 'poblado emitted before'); + pobEvent.runInAsyncScope(() => { + checkInvocations(poblado, { init: 1, before: 1 }, + 'poblado emitted before'); + }); - pobEvent.emitAfter(); checkInvocations(poblado, { init: 1, before: 1, after: 1 }, 'poblado emitted after'); diff --git a/test/async-hooks/test-embedder.api.async-resource.after-on-destroyed.js b/test/async-hooks/test-emit-after-on-destroyed.js similarity index 61% rename from test/async-hooks/test-embedder.api.async-resource.after-on-destroyed.js rename to test/async-hooks/test-emit-after-on-destroyed.js index a389eddf421cc8..b015dacddba506 100644 --- a/test/async-hooks/test-embedder.api.async-resource.after-on-destroyed.js +++ b/test/async-hooks/test-emit-after-on-destroyed.js @@ -1,13 +1,18 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); const assert = require('assert'); -const async_hooks = require('async_hooks'); -const { AsyncResource } = async_hooks; +const internal_async_hooks = require('internal/async_hooks'); const { spawn } = require('child_process'); const corruptedMsg = /async hook stack has become corrupted/; const heartbeatMsg = /heartbeat: still alive/; +const { + newAsyncId, getDefaultTriggerAsyncId, + emitInit, emitBefore, emitAfter, emitDestroy +} = internal_async_hooks; + const initHooks = require('./init-hooks'); if (process.argv[2] === 'child') { @@ -17,20 +22,29 @@ if (process.argv[2] === 'child') { // Once 'destroy' has been emitted, we can no longer emit 'after' // Emitting 'before', 'after' and then 'destroy' - const event1 = new AsyncResource('event1', async_hooks.executionAsyncId()); - event1.emitBefore(); - event1.emitAfter(); - event1.emitDestroy(); + { + const asyncId = newAsyncId(); + const triggerId = getDefaultTriggerAsyncId(); + emitInit(asyncId, 'event1', triggerId, {}); + emitBefore(asyncId, triggerId); + emitAfter(asyncId); + emitDestroy(asyncId); + } // Emitting 'after' after 'destroy' - const event2 = new AsyncResource('event2', async_hooks.executionAsyncId()); - event2.emitDestroy(); - - console.log('heartbeat: still alive'); - event2.emitAfter(); + { + const asyncId = newAsyncId(); + const triggerId = getDefaultTriggerAsyncId(); + emitInit(asyncId, 'event2', triggerId, {}); + emitDestroy(asyncId); + console.log('heartbeat: still alive'); + emitAfter(asyncId); + } } else { - const args = process.argv.slice(1).concat('child'); + const args = ['--expose-internals'] + .concat(process.argv.slice(1)) + .concat('child'); let errData = Buffer.from(''); let outData = Buffer.from(''); diff --git a/test/async-hooks/test-embedder.api.async-resource.before-on-destroyed.js b/test/async-hooks/test-emit-before-on-destroyed.js similarity index 61% rename from test/async-hooks/test-embedder.api.async-resource.before-on-destroyed.js rename to test/async-hooks/test-emit-before-on-destroyed.js index d6465a8445282d..36a50a050f9324 100644 --- a/test/async-hooks/test-embedder.api.async-resource.before-on-destroyed.js +++ b/test/async-hooks/test-emit-before-on-destroyed.js @@ -1,13 +1,18 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); const assert = require('assert'); -const async_hooks = require('async_hooks'); -const { AsyncResource } = async_hooks; +const internal_async_hooks = require('internal/async_hooks'); const { spawn } = require('child_process'); const corruptedMsg = /async hook stack has become corrupted/; const heartbeatMsg = /heartbeat: still alive/; +const { + newAsyncId, getDefaultTriggerAsyncId, + emitInit, emitBefore, emitAfter, emitDestroy +} = internal_async_hooks; + const initHooks = require('./init-hooks'); if (process.argv[2] === 'child') { @@ -17,20 +22,29 @@ if (process.argv[2] === 'child') { // Once 'destroy' has been emitted, we can no longer emit 'before' // Emitting 'before', 'after' and then 'destroy' - const event1 = new AsyncResource('event1', async_hooks.executionAsyncId()); - event1.emitBefore(); - event1.emitAfter(); - event1.emitDestroy(); + { + const asyncId = newAsyncId(); + const triggerId = getDefaultTriggerAsyncId(); + emitInit(asyncId, 'event1', triggerId, {}); + emitBefore(asyncId, triggerId); + emitAfter(asyncId); + emitDestroy(asyncId); + } // Emitting 'before' after 'destroy' - const event2 = new AsyncResource('event2', async_hooks.executionAsyncId()); - event2.emitDestroy(); - - console.log('heartbeat: still alive'); - event2.emitBefore(); + { + const asyncId = newAsyncId(); + const triggerId = getDefaultTriggerAsyncId(); + emitInit(asyncId, 'event2', triggerId, {}); + emitDestroy(asyncId); + console.log('heartbeat: still alive'); + emitBefore(asyncId, triggerId); + } } else { - const args = process.argv.slice(1).concat('child'); + const args = ['--expose-internals'] + .concat(process.argv.slice(1)) + .concat('child'); let errData = Buffer.from(''); let outData = Buffer.from(''); diff --git a/test/async-hooks/test-embedder.api.async-resource.improper-order.js b/test/async-hooks/test-improper-order.js similarity index 63% rename from test/async-hooks/test-embedder.api.async-resource.improper-order.js rename to test/async-hooks/test-improper-order.js index 4a38d87ce21e1f..a9d46917df59e0 100644 --- a/test/async-hooks/test-embedder.api.async-resource.improper-order.js +++ b/test/async-hooks/test-improper-order.js @@ -1,13 +1,18 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); const assert = require('assert'); -const async_hooks = require('async_hooks'); -const { AsyncResource } = async_hooks; +const internal_async_hooks = require('internal/async_hooks'); const { spawn } = require('child_process'); const corruptedMsg = /async hook stack has become corrupted/; const heartbeatMsg = /heartbeat: still alive/; +const { + newAsyncId, getDefaultTriggerAsyncId, + emitInit, emitBefore, emitAfter +} = internal_async_hooks; + const initHooks = require('./init-hooks'); if (process.argv[2] === 'child') { @@ -17,18 +22,28 @@ if (process.argv[2] === 'child') { // Async hooks enforce proper order of 'before' and 'after' invocations // Proper ordering - const event1 = new AsyncResource('event1', async_hooks.executionAsyncId()); - event1.emitBefore(); - event1.emitAfter(); + { + const asyncId = newAsyncId(); + const triggerId = getDefaultTriggerAsyncId(); + emitInit(asyncId, 'event1', triggerId, {}); + emitBefore(asyncId, triggerId); + emitAfter(asyncId); + } // Improper ordering // Emitting 'after' without 'before' which is illegal - const event2 = new AsyncResource('event2', async_hooks.executionAsyncId()); - - console.log('heartbeat: still alive'); - event2.emitAfter(); + { + const asyncId = newAsyncId(); + const triggerId = getDefaultTriggerAsyncId(); + emitInit(asyncId, 'event2', triggerId, {}); + + console.log('heartbeat: still alive'); + emitAfter(asyncId); + } } else { - const args = process.argv.slice(1).concat('child'); + const args = ['--expose-internals'] + .concat(process.argv.slice(1)) + .concat('child'); let errData = Buffer.from(''); let outData = Buffer.from(''); diff --git a/test/async-hooks/test-embedder.api.async-resource.improper-unwind.js b/test/async-hooks/test-improper-unwind.js similarity index 65% rename from test/async-hooks/test-embedder.api.async-resource.improper-unwind.js rename to test/async-hooks/test-improper-unwind.js index cb9e338905c386..a62512db974ac6 100644 --- a/test/async-hooks/test-embedder.api.async-resource.improper-unwind.js +++ b/test/async-hooks/test-improper-unwind.js @@ -1,13 +1,18 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); const assert = require('assert'); -const async_hooks = require('async_hooks'); -const { AsyncResource } = async_hooks; +const internal_async_hooks = require('internal/async_hooks'); const { spawn } = require('child_process'); const corruptedMsg = /async hook stack has become corrupted/; const heartbeatMsg = /heartbeat: still alive/; +const { + newAsyncId, getDefaultTriggerAsyncId, + emitInit, emitBefore, emitAfter +} = internal_async_hooks; + const initHooks = require('./init-hooks'); if (process.argv[2] === 'child') { @@ -21,23 +26,28 @@ if (process.argv[2] === 'child') { // The first test of the two below follows that rule, // the second one doesn't. - const event1 = new AsyncResource('event1', async_hooks.executionAsyncId()); - const event2 = new AsyncResource('event2', async_hooks.executionAsyncId()); + const eventOneId = newAsyncId(); + const eventTwoId = newAsyncId(); + const triggerId = getDefaultTriggerAsyncId(); + emitInit(eventOneId, 'event1', triggerId, {}); + emitInit(eventTwoId, 'event2', triggerId, {}); // Proper unwind - event1.emitBefore(); - event2.emitBefore(); - event2.emitAfter(); - event1.emitAfter(); + emitBefore(eventOneId, triggerId); + emitBefore(eventTwoId, triggerId); + emitAfter(eventTwoId); + emitAfter(eventOneId); // Improper unwind - event1.emitBefore(); - event2.emitBefore(); + emitBefore(eventOneId, triggerId); + emitBefore(eventTwoId, triggerId); console.log('heartbeat: still alive'); - event1.emitAfter(); + emitAfter(eventOneId); } else { - const args = process.argv.slice(1).concat('child'); + const args = ['--expose-internals'] + .concat(process.argv.slice(1)) + .concat('child'); let errData = Buffer.from(''); let outData = Buffer.from(''); diff --git a/test/parallel/test-async-hooks-recursive-stack.js b/test/parallel/test-async-hooks-recursive-stack.js index 7ab73dc1bf4538..bc4ac86e7f1ca1 100644 --- a/test/parallel/test-async-hooks-recursive-stack.js +++ b/test/parallel/test-async-hooks-recursive-stack.js @@ -7,14 +7,14 @@ const async_hooks = require('async_hooks'); function recurse(n) { const a = new async_hooks.AsyncResource('foobar'); - a.emitBefore(); - 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()); - a.emitAfter(); + 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.js b/test/parallel/test-emit-after-uncaught-exception.js index 368099d48355be..5003972e9984aa 100644 --- a/test/parallel/test-emit-after-uncaught-exception.js +++ b/test/parallel/test-emit-after-uncaught-exception.js @@ -26,12 +26,12 @@ setImmediate(common.mustCall(() => { // 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.emitBefore(); - - const ar2 = new async_hooks.AsyncResource('Mine'); - ar2.emitBefore(); - - throw new Error('bye'); + 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