From ccbf4f95b4aff8b8f31fb99132d32d119e353417 Mon Sep 17 00:00:00 2001 From: Andrey Pechkurov Date: Sat, 14 Mar 2020 09:06:06 +0300 Subject: [PATCH] async_hooks: merge run and exit methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/31950 Reviewed-By: Vladimir de Turckheim Reviewed-By: Gerhard Stöbich --- .../async_hooks/async-resource-vs-destroy.js | 2 +- doc/api/async_hooks.md | 146 ++---------------- lib/async_hooks.js | 20 +-- .../test-async-local-storage-args.js | 9 +- .../test-async-local-storage-async-await.js | 2 +- ...est-async-local-storage-async-functions.js | 2 +- ...test-async-local-storage-enable-disable.js | 4 +- .../test-async-local-storage-errors-async.js | 26 ---- ....js => test-async-local-storage-errors.js} | 2 +- .../test-async-local-storage-gcable.js | 2 +- .../test-async-local-storage-misc-stores.js | 15 +- .../test-async-local-storage-nested.js | 13 -- 12 files changed, 29 insertions(+), 214 deletions(-) delete mode 100644 test/async-hooks/test-async-local-storage-errors-async.js rename test/async-hooks/{test-async-local-storage-errors-sync-ret.js => test-async-local-storage-errors.js} (93%) diff --git a/benchmark/async_hooks/async-resource-vs-destroy.js b/benchmark/async_hooks/async-resource-vs-destroy.js index 2d0bb74d57a6f9..ef2e3decd5de81 100644 --- a/benchmark/async_hooks/async-resource-vs-destroy.js +++ b/benchmark/async_hooks/async-resource-vs-destroy.js @@ -103,7 +103,7 @@ function buildDestroy(getServe) { function buildAsyncLocalStorage(getServe) { const asyncLocalStorage = new AsyncLocalStorage(); const server = createServer((req, res) => { - asyncLocalStorage.runSyncAndReturn({}, () => { + asyncLocalStorage.run({}, () => { getServe(getCLS, setCLS)(req, res); }); }); diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index f0c2373448711a..17d6435d47506a 100644 --- a/doc/api/async_hooks.md +++ b/doc/api/async_hooks.md @@ -922,7 +922,7 @@ added: REPLACEME --> Creates a new instance of `AsyncLocalStorage`. Store is only provided within a -`run` or a `runSyncAndReturn` method call. +`run` method call. ### `asyncLocalStorage.disable()` - -* `callback` {Function} -* `...args` {any} - -Calling `asyncLocalStorage.exit(callback)` will create a new asynchronous -context. -Within the callback function and the asynchronous operations from the callback, -`asyncLocalStorage.getStore()` will return `undefined`. - -The callback will be ran asynchronously. Optionally, arguments can be passed -to the function. They will be passed to the callback function. - -If an error is thrown by the callback function, it will not be caught by -a `try/catch` block as the callback is ran in a new asynchronous resource. -Also, the stacktrace will be impacted by the asynchronous call. - -Example: - -```js -asyncLocalStorage.run('store value', () => { - asyncLocalStorage.getStore(); // Returns 'store value' - asyncLocalStorage.exit(() => { - asyncLocalStorage.getStore(); // Returns undefined - }); - asyncLocalStorage.getStore(); // Returns 'store value' -}); -``` - -### `asyncLocalStorage.runSyncAndReturn(store, callback[, ...args])` - - -* `store` {any} -* `callback` {Function} -* `...args` {any} - This methods runs a function synchronously within a context and return its return value. The store is not accessible outside of the callback function or the asynchronous operations created within the callback. @@ -1082,16 +1013,16 @@ the asynchronous operations created within the callback. Optionally, arguments can be passed to the function. They will be passed to the callback function. -If the callback function throws an error, it will be thrown by -`runSyncAndReturn` too. The stacktrace will not be impacted by this call and -the context will be exited. +If the callback function throws an error, it will be thrown by `run` too. +The stacktrace will not be impacted by this call and the context will +be exited. Example: ```js const store = { id: 2 }; try { - asyncLocalStorage.runSyncAndReturn(store, () => { + asyncLocalStorage.run(store, () => { asyncLocalStorage.getStore(); // Returns the store object throw new Error(); }); @@ -1101,7 +1032,7 @@ try { } ``` -### `asyncLocalStorage.exitSyncAndReturn(callback[, ...args])` +### `asyncLocalStorage.exit(callback[, ...args])` @@ -1116,17 +1047,17 @@ the asynchronous operations created within the callback. Optionally, arguments can be passed to the function. They will be passed to the callback function. -If the callback function throws an error, it will be thrown by -`exitSyncAndReturn` too. The stacktrace will not be impacted by this call and +If the callback function throws an error, it will be thrown by `exit` too. +The stacktrace will not be impacted by this call and the context will be re-entered. Example: ```js -// Within a call to run or runSyncAndReturn +// Within a call to run try { asyncLocalStorage.getStore(); // Returns the store object or value - asyncLocalStorage.exitSyncAndReturn(() => { + asyncLocalStorage.exit(() => { asyncLocalStorage.getStore(); // Returns undefined throw new Error(); }); @@ -1136,59 +1067,14 @@ try { } ``` -### Choosing between `run` and `runSyncAndReturn` - -#### When to choose `run` - -`run` is asynchronous. It is called with a callback function that -runs within a new asynchronous call. This is the most explicit behavior as -everything that is executed within the callback of `run` (including further -asynchronous operations) will have access to the store. - -If an instance of `AsyncLocalStorage` is used for error management (for -instance, with `process.setUncaughtExceptionCaptureCallback`), only -exceptions thrown in the scope of the callback function will be associated -with the context. - -This method is the safest as it provides strong scoping and consistent -behavior. - -It cannot be promisified using `util.promisify`. If needed, the `Promise` -constructor can be used: - -```js -const store = new Map(); // initialize the store -new Promise((resolve, reject) => { - asyncLocalStorage.run(store, () => { - someFunction((err, result) => { - if (err) { - return reject(err); - } - return resolve(result); - }); - }); -}); -``` - -#### When to choose `runSyncAndReturn` - -`runSyncAndReturn` is synchronous. The callback function will be executed -synchronously and its return value will be returned by `runSyncAndReturn`. -The store will only be accessible from within the callback -function and the asynchronous operations created within this scope. -If the callback throws an error, `runSyncAndReturn` will throw it and it will -not be associated with the context. - -This method provides good scoping while being synchronous. - -#### Usage with `async/await` +### Usage with `async/await` If, within an async function, only one `await` call is to run within a context, the following pattern should be used: ```js async function fn() { - await asyncLocalStorage.runSyncAndReturn(new Map(), () => { + await asyncLocalStorage.run(new Map(), () => { asyncLocalStorage.getStore().set('key', value); return foo(); // The return value of foo will be awaited }); @@ -1196,8 +1082,8 @@ async function fn() { ``` In this example, the store is only available in the callback function and the -functions called by `foo`. Outside of `runSyncAndReturn`, calling `getStore` -will return `undefined`. +functions called by `foo`. Outside of `run`, calling `getStore` will return +`undefined`. [`after` callback]: #async_hooks_after_asyncid [`before` callback]: #async_hooks_before_asyncid diff --git a/lib/async_hooks.js b/lib/async_hooks.js index dadde6a0f2e7ad..0943534790550c 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -256,7 +256,7 @@ class AsyncLocalStorage { resource[this.kResourceStore] = store; } - runSyncAndReturn(store, callback, ...args) { + run(store, callback, ...args) { const resource = new AsyncResource('AsyncLocalStorage'); return resource.runInAsyncScope(() => { this.enterWith(store); @@ -264,7 +264,7 @@ class AsyncLocalStorage { }); } - exitSyncAndReturn(callback, ...args) { + exit(callback, ...args) { if (!this.enabled) { return callback(...args); } @@ -282,22 +282,6 @@ class AsyncLocalStorage { return resource[this.kResourceStore]; } } - - run(store, callback, ...args) { - process.nextTick(() => { - this.enterWith(store); - return callback(...args); - }); - } - - exit(callback, ...args) { - if (!this.enabled) { - return process.nextTick(callback, ...args); - } - this.enabled = false; - process.nextTick(callback, ...args); - this.enabled = true; - } } // Placing all exports down here because the exported classes won't export diff --git a/test/async-hooks/test-async-local-storage-args.js b/test/async-hooks/test-async-local-storage-args.js index 04316dff59d71a..71853ecc4af0eb 100644 --- a/test/async-hooks/test-async-local-storage-args.js +++ b/test/async-hooks/test-async-local-storage-args.js @@ -6,15 +6,8 @@ const { AsyncLocalStorage } = require('async_hooks'); const asyncLocalStorage = new AsyncLocalStorage(); asyncLocalStorage.run({}, (runArg) => { - assert.strictEqual(runArg, 1); - asyncLocalStorage.exit((exitArg) => { - assert.strictEqual(exitArg, 2); - }, 2); -}, 1); - -asyncLocalStorage.runSyncAndReturn({}, (runArg) => { assert.strictEqual(runArg, 'foo'); - asyncLocalStorage.exitSyncAndReturn((exitArg) => { + asyncLocalStorage.exit((exitArg) => { assert.strictEqual(exitArg, 'bar'); }, 'bar'); }, 'foo'); diff --git a/test/async-hooks/test-async-local-storage-async-await.js b/test/async-hooks/test-async-local-storage-async-await.js index a03f803186bdab..64333eee938f3d 100644 --- a/test/async-hooks/test-async-local-storage-async-await.js +++ b/test/async-hooks/test-async-local-storage-async-await.js @@ -12,7 +12,7 @@ async function test() { } async function main() { - await asyncLocalStorage.runSyncAndReturn(new Map(), test); + await asyncLocalStorage.run(new Map(), test); assert.strictEqual(asyncLocalStorage.getStore(), undefined); } diff --git a/test/async-hooks/test-async-local-storage-async-functions.js b/test/async-hooks/test-async-local-storage-async-functions.js index a0852bc1098a1a..ea186e9f68da4e 100644 --- a/test/async-hooks/test-async-local-storage-async-functions.js +++ b/test/async-hooks/test-async-local-storage-async-functions.js @@ -16,7 +16,7 @@ async function testAwait() { await foo(); assert.notStrictEqual(asyncLocalStorage.getStore(), undefined); assert.strictEqual(asyncLocalStorage.getStore().get('key'), 'value'); - await asyncLocalStorage.exitSyncAndReturn(testOut); + await asyncLocalStorage.exit(testOut); } asyncLocalStorage.run(new Map(), () => { diff --git a/test/async-hooks/test-async-local-storage-enable-disable.js b/test/async-hooks/test-async-local-storage-enable-disable.js index 93132079827eeb..15ab2f5d9ebbb2 100644 --- a/test/async-hooks/test-async-local-storage-enable-disable.js +++ b/test/async-hooks/test-async-local-storage-enable-disable.js @@ -5,7 +5,7 @@ const { AsyncLocalStorage } = require('async_hooks'); const asyncLocalStorage = new AsyncLocalStorage(); -asyncLocalStorage.runSyncAndReturn(new Map(), () => { +asyncLocalStorage.run(new Map(), () => { asyncLocalStorage.getStore().set('foo', 'bar'); process.nextTick(() => { assert.strictEqual(asyncLocalStorage.getStore().get('foo'), 'bar'); @@ -24,7 +24,7 @@ asyncLocalStorage.runSyncAndReturn(new Map(), () => { process.nextTick(() => { assert.strictEqual(asyncLocalStorage.getStore(), undefined); - asyncLocalStorage.runSyncAndReturn(new Map(), () => { + asyncLocalStorage.run(new Map(), () => { assert.notStrictEqual(asyncLocalStorage.getStore(), undefined); }); }); diff --git a/test/async-hooks/test-async-local-storage-errors-async.js b/test/async-hooks/test-async-local-storage-errors-async.js deleted file mode 100644 index b6f0b4fa742f61..00000000000000 --- a/test/async-hooks/test-async-local-storage-errors-async.js +++ /dev/null @@ -1,26 +0,0 @@ -'use strict'; -require('../common'); -const assert = require('assert'); -const { AsyncLocalStorage } = require('async_hooks'); - -// case 1 fully async APIS (safe) -const asyncLocalStorage = new AsyncLocalStorage(); - -let i = 0; -process.setUncaughtExceptionCaptureCallback((err) => { - ++i; - assert.strictEqual(err.message, 'err' + i); - assert.strictEqual(asyncLocalStorage.getStore().get('hello'), 'node'); -}); - -asyncLocalStorage.run(new Map(), () => { - const store = asyncLocalStorage.getStore(); - store.set('hello', 'node'); - setTimeout(() => { - process.nextTick(() => { - assert.strictEqual(i, 2); - }); - throw new Error('err2'); - }, 0); - throw new Error('err1'); -}); diff --git a/test/async-hooks/test-async-local-storage-errors-sync-ret.js b/test/async-hooks/test-async-local-storage-errors.js similarity index 93% rename from test/async-hooks/test-async-local-storage-errors-sync-ret.js rename to test/async-hooks/test-async-local-storage-errors.js index 3b5c57a73472f6..0dd5754e02cbd9 100644 --- a/test/async-hooks/test-async-local-storage-errors-sync-ret.js +++ b/test/async-hooks/test-async-local-storage-errors.js @@ -14,7 +14,7 @@ process.setUncaughtExceptionCaptureCallback((err) => { }); try { - asyncLocalStorage.runSyncAndReturn(new Map(), () => { + asyncLocalStorage.run(new Map(), () => { const store = asyncLocalStorage.getStore(); store.set('hello', 'node'); setTimeout(() => { diff --git a/test/async-hooks/test-async-local-storage-gcable.js b/test/async-hooks/test-async-local-storage-gcable.js index 37b04b38d14588..f0d23a0d22793b 100644 --- a/test/async-hooks/test-async-local-storage-gcable.js +++ b/test/async-hooks/test-async-local-storage-gcable.js @@ -10,7 +10,7 @@ const onGC = require('../common/ongc'); let asyncLocalStorage = new AsyncLocalStorage(); -asyncLocalStorage.runSyncAndReturn({}, () => { +asyncLocalStorage.run({}, () => { asyncLocalStorage.disable(); onGC(asyncLocalStorage, { ongc: common.mustCall() }); diff --git a/test/async-hooks/test-async-local-storage-misc-stores.js b/test/async-hooks/test-async-local-storage-misc-stores.js index 56873008dd644f..fbbbb52a5d7a6b 100644 --- a/test/async-hooks/test-async-local-storage-misc-stores.js +++ b/test/async-hooks/test-async-local-storage-misc-stores.js @@ -5,20 +5,11 @@ const { AsyncLocalStorage } = require('async_hooks'); const asyncLocalStorage = new AsyncLocalStorage(); -asyncLocalStorage.run(42, () => { - assert.strictEqual(asyncLocalStorage.getStore(), 42); +asyncLocalStorage.run('hello node', () => { + assert.strictEqual(asyncLocalStorage.getStore(), 'hello node'); }); -const runStore = { foo: 'bar' }; +const runStore = { hello: 'node' }; asyncLocalStorage.run(runStore, () => { assert.strictEqual(asyncLocalStorage.getStore(), runStore); }); - -asyncLocalStorage.runSyncAndReturn('hello node', () => { - assert.strictEqual(asyncLocalStorage.getStore(), 'hello node'); -}); - -const runSyncStore = { hello: 'node' }; -asyncLocalStorage.runSyncAndReturn(runSyncStore, () => { - assert.strictEqual(asyncLocalStorage.getStore(), runSyncStore); -}); diff --git a/test/async-hooks/test-async-local-storage-nested.js b/test/async-hooks/test-async-local-storage-nested.js index 143d5d45de9e25..870c294b74d22a 100644 --- a/test/async-hooks/test-async-local-storage-nested.js +++ b/test/async-hooks/test-async-local-storage-nested.js @@ -19,20 +19,7 @@ function testInner() { assert.strictEqual(asyncLocalStorage.getStore(), undefined); }); assert.strictEqual(asyncLocalStorage.getStore(), outer); - - asyncLocalStorage.runSyncAndReturn(inner, () => { - assert.strictEqual(asyncLocalStorage.getStore(), inner); - }); - assert.strictEqual(asyncLocalStorage.getStore(), outer); - - asyncLocalStorage.exitSyncAndReturn(() => { - assert.strictEqual(asyncLocalStorage.getStore(), undefined); - }); - assert.strictEqual(asyncLocalStorage.getStore(), outer); } asyncLocalStorage.run(outer, testInner); assert.strictEqual(asyncLocalStorage.getStore(), undefined); - -asyncLocalStorage.runSyncAndReturn(outer, testInner); -assert.strictEqual(asyncLocalStorage.getStore(), undefined);