Ignore set() calls for removed AsyncLocal

nodejs · puzpuzpuz · Dec 18, 2019 · Feb 13, 2020 · Feb 13, 2020 · Feb 13, 2020
commit 4efc80ff41151b845f78445c535c975600746e63
diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md
@@ -682,7 +682,7 @@ setImmediate(() => {
 ```
 
 If the `AsyncLocal` was removed before this call is made,
-[`ERR_ASYNC_LOCAL_CANNOT_SET_VALUE`][] is thrown.
+the call will have no effect.
 
 ### `asyncLocal.remove()`
 
@@ -691,8 +691,7 @@ values stored by it. Calling `asyncLocal.remove()` multiple times will
 have no effect.
 
 Any subsequent `asyncLocal.get()` calls will return `undefined`.
-Any subsequent `asyncLocal.set(value)` calls will throw
-[`ERR_ASYNC_LOCAL_CANNOT_SET_VALUE`][].
+Any subsequent `asyncLocal.set(value)` calls will have no effect.
 
 ## Promise execution tracking
 
@@ -983,4 +982,3 @@ for (let i = 0; i < 10; i++) {
 [PromiseHooks]: https://docs.google.com/document/d/1rda3yKGHimKIhg5YeoAmCOtyURgsbTH_qaYR79FELlk/edit
 [`Worker`]: worker_threads.html#worker_threads_class_worker
 [promise execution tracking]: #async_hooks_promise_execution_tracking
-[`ERR_ASYNC_LOCAL_CANNOT_SET_VALUE`]: errors.html#ERR_ASYNC_LOCAL_CANNOT_SET_VALUE
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -643,11 +643,6 @@ by the `assert` module.
 An attempt was made to register something that is not a function as an
 `AsyncHooks` callback.
 
-<a id="ERR_ASYNC_LOCAL_CANNOT_SET_VALUE"></a>
-### `ERR_ASYNC_LOCAL_CANNOT_SET_VALUE`
-
-An attempt was made to set value for a `AsyncLocal` after it was removed.
-
 <a id="ERR_ASYNC_TYPE"></a>
 ### `ERR_ASYNC_TYPE`
 

diff --git a/lib/async_hooks.js b/lib/async_hooks.js
@@ -11,7 +11,6 @@ const {
   ERR_ASYNC_CALLBACK,
   ERR_ASYNC_TYPE,
   ERR_INVALID_ASYNC_ID,
-  ERR_ASYNC_LOCAL_CANNOT_SET_VALUE,
 } = require('internal/errors').codes;
 const { validateString } = require('internal/validators');
 const internal_async_hooks = require('internal/async_hooks');
@@ -166,16 +165,17 @@ class AsyncLocal {
 
   get() {
     if (this[kResToValSymbol]) {
-      return this[kResToValSymbol].get(executionAsyncResource());
+      const execRes = executionAsyncResource();
+      return this[kResToValSymbol].get(execRes);
     }
     return undefined;
   }
 
   set(value) {
-    if (!this[kResToValSymbol]) {
-      throw new ERR_ASYNC_LOCAL_CANNOT_SET_VALUE();
+    if (this[kResToValSymbol]) {
+      const execRes = executionAsyncResource();
+      this[kResToValSymbol].set(execRes, value);
     }
-    this[kResToValSymbol].set(executionAsyncResource(), value);
   }
 
   remove() {

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -730,8 +730,6 @@ E('ERR_AMBIGUOUS_ARGUMENT', 'The "%s" argument is ambiguous. %s', TypeError);
 E('ERR_ARG_NOT_ITERABLE', '%s must be iterable', TypeError);
 E('ERR_ASSERTION', '%s', Error);
 E('ERR_ASYNC_CALLBACK', '%s must be a function', TypeError);
-E('ERR_ASYNC_LOCAL_CANNOT_SET_VALUE', 'Cannot set value for removed AsyncLocal',
-  Error);
 E('ERR_ASYNC_TYPE', 'Invalid name for async "type": %s', TypeError);
 E('ERR_BROTLI_INVALID_PARAM', '%s is not a valid Brotli parameter', RangeError);
 E('ERR_BUFFER_OUT_OF_BOUNDS',

diff --git a/test/async-hooks/test-async-local.js b/test/async-hooks/test-async-local.js
@@ -1,6 +1,6 @@
 'use strict';
 
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 const async_hooks = require('async_hooks');
 const { AsyncLocal } = async_hooks;
@@ -22,12 +22,9 @@ assert.strictEqual(asyncLocal.get(), obj);
 asyncLocal.remove();
 assert.strictEqual(asyncLocal.get(), undefined);
 
-// Throws on modification after removal
-const error = common.expectsError({
-  code: 'ERR_ASYNC_LOCAL_CANNOT_SET_VALUE',
-  name: 'Error',
-});
-assert.throws(() => asyncLocal.set('bar'), error);
+// Subsequent .set() is ignored
+asyncLocal.set('bar');
+assert.strictEqual(asyncLocal.get(), undefined);
 
 // Subsequent .remove() does not throw
 asyncLocal.remove();