async_hooks: remove experimental onPropagate option

The `onPropagate` option for `AsyncLocalStorage` is problematic for a
couple of reasons:

1. It is not expected to be forwards compatible in any way with the
   upcoming TC-39 `AsyncContext` proposal.
2. It introduces a non-trivial O(n) cost invoking a JavaScript callback
   for *every* AsyncResource that is created, including every Promise.

While it is still experimental, I recommend removing it while we can
revisit the fundamental use cases in light of the coming `AsyncContext`
proposal.

Refs: https://github.com/nodejs/node/issues/46374
PR-URL: https://github.com/nodejs/node/pull/46386
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
This commit is contained in:
James M Snell
2023-01-27 14:03:51 -08:00
parent 088e470dcd
commit dc90810f9f
3 changed files with 6 additions and 79 deletions

View File

@@ -116,13 +116,16 @@ Each instance of `AsyncLocalStorage` maintains an independent storage context.
Multiple instances can safely exist simultaneously without risk of interfering
with each other's data.
### `new AsyncLocalStorage([options])`
### `new AsyncLocalStorage()`
<!-- YAML
added:
- v13.10.0
- v12.17.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/46386
description: Removed experimental onPropagate option.
- version:
- v19.2.0
- v18.13.0
@@ -130,23 +133,9 @@ changes:
description: Add option onPropagate.
-->
> Stability: 1 - `options.onPropagate` is experimental.
* `options` {Object}
* `onPropagate` {Function} Optional callback invoked before a store is
propagated to a new async resource. Returning `true` allows propagation,
returning `false` avoids it. Default is to propagate always.
Creates a new instance of `AsyncLocalStorage`. Store is only provided within a
`run()` call or after an `enterWith()` call.
The `onPropagate` is called during creation of an async resource. Throwing at
this time will print the stack trace and exit. See
[`async_hooks` Error handling][] for details.
Creating an async resource within the `onPropagate` callback will result in
a recursive call to `onPropagate`.
### `asyncLocalStorage.disable()`
<!-- YAML
@@ -836,5 +825,4 @@ const server = createServer((req, res) => {
[`EventEmitter`]: events.md#class-eventemitter
[`Stream`]: stream.md#stream
[`Worker`]: worker_threads.md#class-worker
[`async_hooks` Error handling]: async_hooks.md#error-handling
[`util.promisify()`]: util.md#utilpromisifyoriginal

View File

@@ -23,7 +23,6 @@ const {
const { kEmptyObject } = require('internal/util');
const {
validateFunction,
validateObject,
validateString,
} = require('internal/validators');
const internal_async_hooks = require('internal/async_hooks');
@@ -275,17 +274,9 @@ const storageHook = createHook({
});
class AsyncLocalStorage {
constructor(options = kEmptyObject) {
validateObject(options, 'options');
const { onPropagate = null } = options;
if (onPropagate !== null) {
validateFunction(onPropagate, 'options.onPropagate');
}
constructor() {
this.kResourceStore = Symbol('kResourceStore');
this.enabled = false;
this._onPropagate = onPropagate;
}
disable() {
@@ -312,9 +303,7 @@ class AsyncLocalStorage {
_propagate(resource, triggerResource, type) {
const store = triggerResource[this.kResourceStore];
if (this.enabled) {
if (this._onPropagate === null || this._onPropagate(type, store)) {
resource[this.kResourceStore] = store;
}
resource[this.kResourceStore] = store;
}
}

View File

@@ -1,50 +0,0 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const { AsyncLocalStorage, AsyncResource } = require('async_hooks');
let cnt = 0;
function onPropagate(type, store) {
assert.strictEqual(als.getStore(), store);
cnt++;
if (cnt === 1) {
assert.strictEqual(type, 'r1');
return true;
}
if (cnt === 2) {
assert.strictEqual(type, 'r2');
return false;
}
}
const als = new AsyncLocalStorage({
onPropagate: common.mustCall(onPropagate, 2),
});
const myStore = {};
als.run(myStore, common.mustCall(() => {
const r1 = new AsyncResource('r1');
const r2 = new AsyncResource('r2');
r1.runInAsyncScope(common.mustCall(() => {
assert.strictEqual(als.getStore(), myStore);
}));
r2.runInAsyncScope(common.mustCall(() => {
assert.strictEqual(als.getStore(), undefined);
r1.runInAsyncScope(common.mustCall(() => {
assert.strictEqual(als.getStore(), myStore);
}));
}));
}));
assert.throws(() => new AsyncLocalStorage(15), {
message: 'The "options" argument must be of type object. Received type number (15)',
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
});
assert.throws(() => new AsyncLocalStorage({ onPropagate: 'bar' }), {
message: 'The "options.onPropagate" property must be of type function. Received type string (\'bar\')',
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
});