async_hooks: fix AsyncLocalStorage in unhandledRejection cases

bmeck · danielleadams · commit b688f201df16 · 2022-01-31T23:53:31.000-05:00
PR-URL: #41202 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js
@@ -443,7 +443,16 @@ function clearDefaultTriggerAsyncId() {
   async_id_fields[kDefaultTriggerAsyncId] = -1;
 }
 
-
+/**
+ * Sets a default top level trigger ID to be used
+ *
+ * @template {Array<unknown>} T
+ * @template {unknown} R
+ * @param {number} triggerAsyncId
+ * @param { (...T: args) => R } block
+ * @param  {T} args
+ * @returns {R}
+ */
 function defaultTriggerAsyncIdScope(triggerAsyncId, block, ...args) {
   if (triggerAsyncId === undefined)
     return block.apply(null, args);
diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js
@@ -27,8 +27,11 @@ const {
 const {
   pushAsyncContext,
   popAsyncContext,
+  symbols: {
+    async_id_symbol: kAsyncIdSymbol,
+    trigger_async_id_symbol: kTriggerAsyncIdSymbol
+  }
 } = require('internal/async_hooks');
-const async_hooks = require('async_hooks');
 const { isErrorStackTraceLimitWritable } = require('internal/errors');
 
 // *Must* match Environment::TickInfo::Fields in src/env.h.
@@ -123,20 +126,11 @@ function resolveError(type, promise, reason) {
 }
 
 function unhandledRejection(promise, reason) {
-  const asyncId = async_hooks.executionAsyncId();
-  const triggerAsyncId = async_hooks.triggerAsyncId();
-  const resource = promise;
-
   const emit = (reason, promise, promiseInfo) => {
-    try {
-      pushAsyncContext(asyncId, triggerAsyncId, resource);
-      if (promiseInfo.domain) {
-        return promiseInfo.domain.emit('error', reason);
-      }
-      return process.emit('unhandledRejection', reason, promise);
-    } finally {
-      popAsyncContext(asyncId);
+    if (promiseInfo.domain) {
+      return promiseInfo.domain.emit('error', reason);
     }
+    return process.emit('unhandledRejection', reason, promise);
   };
 
   maybeUnhandledPromises.set(promise, {
@@ -220,40 +214,73 @@ function processPromiseRejections() {
     promiseInfo.warned = true;
     const { reason, uid, emit } = promiseInfo;
 
-    switch (unhandledRejectionsMode) {
-      case kStrictUnhandledRejections: {
-        const err = reason instanceof Error ?
-          reason : generateUnhandledRejectionError(reason);
-        triggerUncaughtException(err, true /* fromPromise */);
-        const handled = emit(reason, promise, promiseInfo);
-        if (!handled) emitUnhandledRejectionWarning(uid, reason);
-        break;
-      }
-      case kIgnoreUnhandledRejections: {
-        emit(reason, promise, promiseInfo);
-        break;
-      }
-      case kAlwaysWarnUnhandledRejections: {
-        emit(reason, promise, promiseInfo);
-        emitUnhandledRejectionWarning(uid, reason);
-        break;
-      }
-      case kThrowUnhandledRejections: {
-        const handled = emit(reason, promise, promiseInfo);
-        if (!handled) {
+    let needPop = true;
+    const {
+      [kAsyncIdSymbol]: promiseAsyncId,
+      [kTriggerAsyncIdSymbol]: promiseTriggerAsyncId,
+    } = promise;
+    // We need to check if async_hooks are enabled
+    // don't use enabledHooksExist as a Promise could
+    // come from a vm.* context and not have an async id
+    if (typeof promiseAsyncId !== 'undefined') {
+      pushAsyncContext(
+        promiseAsyncId,
+        promiseTriggerAsyncId,
+        promise
+      );
+    }
+    try {
+      switch (unhandledRejectionsMode) {
+        case kStrictUnhandledRejections: {
           const err = reason instanceof Error ?
             reason : generateUnhandledRejectionError(reason);
+          // This destroys the async stack, don't clear it after
           triggerUncaughtException(err, true /* fromPromise */);
+          if (typeof promiseAsyncId !== 'undefined') {
+            pushAsyncContext(
+              promise[kAsyncIdSymbol],
+              promise[kTriggerAsyncIdSymbol],
+              promise
+            );
+          }
+          const handled = emit(reason, promise, promiseInfo);
+          if (!handled) emitUnhandledRejectionWarning(uid, reason);
+          break;
         }
-        break;
-      }
-      case kWarnWithErrorCodeUnhandledRejections: {
-        const handled = emit(reason, promise, promiseInfo);
-        if (!handled) {
+        case kIgnoreUnhandledRejections: {
+          emit(reason, promise, promiseInfo);
+          break;
+        }
+        case kAlwaysWarnUnhandledRejections: {
+          emit(reason, promise, promiseInfo);
           emitUnhandledRejectionWarning(uid, reason);
-          process.exitCode = 1;
+          break;
+        }
+        case kThrowUnhandledRejections: {
+          const handled = emit(reason, promise, promiseInfo);
+          if (!handled) {
+            const err = reason instanceof Error ?
+              reason : generateUnhandledRejectionError(reason);
+              // This destroys the async stack, don't clear it after
+            triggerUncaughtException(err, true /* fromPromise */);
+            needPop = false;
+          }
+          break;
+        }
+        case kWarnWithErrorCodeUnhandledRejections: {
+          const handled = emit(reason, promise, promiseInfo);
+          if (!handled) {
+            emitUnhandledRejectionWarning(uid, reason);
+            process.exitCode = 1;
+          }
+          break;
+        }
+      }
+    } finally {
+      if (needPop) {
+        if (typeof promiseAsyncId !== 'undefined') {
+          popAsyncContext(promiseAsyncId);
         }
-        break;
       }
     }
     maybeScheduledTicksOrMicrotasks = true;
diff --git a/test/async-hooks/test-async-local-storage-errors.js b/test/async-hooks/test-async-local-storage-errors.js
@@ -1,31 +1,118 @@
 'use strict';
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 const { AsyncLocalStorage } = require('async_hooks');
+const vm = require('vm');
+
+// err1 is emitted sync as a control - no events
+// err2 is emitted after a timeout - uncaughtExceptionMonitor
+//                                 + uncaughtException
+// err3 is emitted after some awaits -  unhandledRejection
+// err4 is emitted during handling err3 - uncaughtExceptionMonitor
+// err5 is emitted after err4 from a VM lacking hooks - unhandledRejection
+//                                                    + uncaughtException
 
-// case 2 using *AndReturn calls (dual behaviors)
 const asyncLocalStorage = new AsyncLocalStorage();
+const callbackToken = { callbackToken: true };
+const awaitToken = { awaitToken: true };
 
 let i = 0;
-process.setUncaughtExceptionCaptureCallback((err) => {
-  ++i;
-  assert.strictEqual(err.message, 'err2');
-  assert.strictEqual(asyncLocalStorage.getStore().get('hello'), 'node');
-});
-
-try {
-  asyncLocalStorage.run(new Map(), () => {
-    const store = asyncLocalStorage.getStore();
-    store.set('hello', 'node');
-    setTimeout(() => {
-      process.nextTick(() => {
-        assert.strictEqual(i, 1);
-      });
-      throw new Error('err2');
-    }, 0);
-    throw new Error('err1');
+
+// Redefining the uncaughtExceptionHandler is a bit odd, so we just do this
+// so we can track total invocations
+let underlyingExceptionHandler;
+const exceptionHandler = common.mustCall(function(...args) {
+  return underlyingExceptionHandler.call(this, ...args);
+}, 2);
+process.setUncaughtExceptionCaptureCallback(exceptionHandler);
+
+const exceptionMonitor = common.mustCall((err, origin) => {
+  if (err.message === 'err2') {
+    assert.strictEqual(origin, 'uncaughtException');
+    assert.strictEqual(asyncLocalStorage.getStore(), callbackToken);
+  } else if (err.message === 'err4') {
+    assert.strictEqual(origin, 'unhandledRejection');
+    assert.strictEqual(asyncLocalStorage.getStore(), awaitToken);
+  } else {
+    assert.fail('unknown error ' + err);
+  }
+}, 2);
+process.on('uncaughtExceptionMonitor', exceptionMonitor);
+
+function fireErr1() {
+  underlyingExceptionHandler = common.mustCall(function(err) {
+    ++i;
+    assert.strictEqual(err.message, 'err2');
+    assert.strictEqual(asyncLocalStorage.getStore(), callbackToken);
+  }, 1);
+  try {
+    asyncLocalStorage.run(callbackToken, () => {
+      setTimeout(fireErr2, 0);
+      throw new Error('err1');
+    });
+  } catch (e) {
+    assert.strictEqual(e.message, 'err1');
+    assert.strictEqual(asyncLocalStorage.getStore(), undefined);
+  }
+}
+
+function fireErr2() {
+  process.nextTick(() => {
+    assert.strictEqual(i, 1);
+    fireErr3();
   });
-} catch (e) {
-  assert.strictEqual(e.message, 'err1');
-  assert.strictEqual(asyncLocalStorage.getStore(), undefined);
+  throw new Error('err2');
+}
+
+function fireErr3() {
+  assert.strictEqual(asyncLocalStorage.getStore(), callbackToken);
+  const rejectionHandler3 = common.mustCall((err) => {
+    assert.strictEqual(err.message, 'err3');
+    assert.strictEqual(asyncLocalStorage.getStore(), awaitToken);
+    process.off('unhandledRejection', rejectionHandler3);
+
+    fireErr4();
+  }, 1);
+  process.on('unhandledRejection', rejectionHandler3);
+  async function awaitTest() {
+    await null;
+    throw new Error('err3');
+  }
+  asyncLocalStorage.run(awaitToken, awaitTest);
+}
+
+const uncaughtExceptionHandler4 = common.mustCall(
+  function(err) {
+    assert.strictEqual(err.message, 'err4');
+    assert.strictEqual(asyncLocalStorage.getStore(), awaitToken);
+    fireErr5();
+  }, 1);
+function fireErr4() {
+  assert.strictEqual(asyncLocalStorage.getStore(), awaitToken);
+  underlyingExceptionHandler = uncaughtExceptionHandler4;
+  // re-entrant check
+  Promise.reject(new Error('err4'));
 }
+
+function fireErr5() {
+  assert.strictEqual(asyncLocalStorage.getStore(), awaitToken);
+  underlyingExceptionHandler = () => {};
+  const rejectionHandler5 = common.mustCall((err) => {
+    assert.strictEqual(err.message, 'err5');
+    assert.strictEqual(asyncLocalStorage.getStore(), awaitToken);
+    process.off('unhandledRejection', rejectionHandler5);
+  }, 1);
+  process.on('unhandledRejection', rejectionHandler5);
+  const makeOrphan = vm.compileFunction(`(${String(() => {
+    async function main() {
+      await null;
+      Promise.resolve().then(() => {
+        throw new Error('err5');
+      });
+    }
+    main();
+  })})()`);
+  makeOrphan();
+}
+
+fireErr1();
