async_hooks: only emit after for AsyncResource if stack not empty

addaleax · targos · commit 56be32d22d80 · 2019-11-11T10:24:53.000+01:00
We clear the async id stack inside the uncaught exception handler and emit `after` events in the process, so we should not emit `after` a second time from the `runInAsyncScope()` code. This should match the behaviour we have in C++. Fixes: #30080 PR-URL: #30087 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
diff --git a/lib/async_hooks.js b/lib/async_hooks.js
@@ -17,6 +17,7 @@ const {
   executionAsyncId,
   triggerAsyncId,
   // Private API
+  hasAsyncIdStack,
   getHookArrays,
   enableHooks,
   disableHooks,
@@ -172,7 +173,8 @@ class AsyncResource {
         return fn(...args);
       return Reflect.apply(fn, thisArg, args);
     } finally {
-      emitAfter(asyncId);
+      if (hasAsyncIdStack())
+        emitAfter(asyncId);
     }
   }
 
diff --git a/test/parallel/test-queue-microtask-uncaught-asynchooks.js b/test/parallel/test-queue-microtask-uncaught-asynchooks.js
@@ -0,0 +1,36 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const async_hooks = require('async_hooks');
+
+// Regression test for https://github.com/nodejs/node/issues/30080:
+// An uncaught exception inside a queueMicrotask callback should not lead
+// to multiple after() calls for it.
+
+let &micro;taskId;
+const events = [];
+
+async_hooks.createHook({
+  init(id, type, triggerId, resoure) {
+    if (type === 'Microtask') {
+      &micro;taskId = id;
+      events.push('init');
+    }
+  },
+  before(id) {
+    if (id === &micro;taskId) events.push('before');
+  },
+  after(id) {
+    if (id === &micro;taskId) events.push('after');
+  },
+  destroy(id) {
+    if (id === &micro;taskId) events.push('destroy');
+  }
+}).enable();
+
+queueMicrotask(() => { throw new Error(); });
+
+process.on('uncaughtException', common.mustCall());
+process.on('exit', () => {
+  assert.deepStrictEqual(events, ['init', 'after', 'before', 'destroy']);
+});
