inspector: skip promise hook in the inspector async hook

joyeecheung · targos · commit d6234b46522f · 2025-02-25T17:51:04.000+01:00
Instead of filtering out promises in the async hooks added for async task tracking, add an internal path to skip adding the promise hook completely for the inspector async hook. The actual user-land promise tracking is already handled by V8 inspector. This prevents the internal promise hook from showing up and creating unnecessary noise when stepping into async execution in the inspector. PR-URL: #57148 Refs: https://issues.chromium.org/issues/390581540 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
diff --git a/lib/async_hooks.js b/lib/async_hooks.js
@@ -34,7 +34,7 @@ const AsyncContextFrame = require('internal/async_context_frame');
 // Get functions
 // For userland AsyncResources, make sure to emit a destroy event when the
 // resource gets gced.
-const { registerDestroyHook } = internal_async_hooks;
+const { registerDestroyHook, kNoPromiseHook } = internal_async_hooks;
 const {
   asyncWrap,
   executionAsyncId,
@@ -90,6 +90,7 @@ class AsyncHook {
     this[after_symbol] = after;
     this[destroy_symbol] = destroy;
     this[promise_resolve_symbol] = promiseResolve;
+    this[kNoPromiseHook] = false;
   }
 
   enable() {
@@ -120,7 +121,9 @@ class AsyncHook {
       enableHooks();
     }
 
-    updatePromiseHookMode();
+    if (!this[kNoPromiseHook]) {
+      updatePromiseHookMode();
+    }
 
     return this;
   }
diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js
@@ -576,6 +576,7 @@ function triggerAsyncId() {
   return async_id_fields[kTriggerAsyncId];
 }
 
+const kNoPromiseHook = Symbol('kNoPromiseHook');
 
 module.exports = {
   executionAsyncId,
@@ -624,4 +625,5 @@ module.exports = {
   asyncWrap: {
     Providers: async_wrap.Providers,
   },
+  kNoPromiseHook,
 };
diff --git a/lib/internal/inspector_async_hook.js b/lib/internal/inspector_async_hook.js
@@ -1,16 +1,13 @@
 'use strict';
 
-const {
-  SafeSet,
-} = primordials;
-
 let hook;
 let config;
 
 function lazyHookCreation() {
   const inspector = internalBinding('inspector');
   const { createHook } = require('async_hooks');
   config = internalBinding('config');
+  const { kNoPromiseHook } = require('internal/async_hooks');
 
   hook = createHook({
     init(asyncId, type, triggerAsyncId, resource) {
@@ -19,32 +16,22 @@ function lazyHookCreation() {
     // in https://github.com/nodejs/node/pull/13870#discussion_r124515293,
     // this should be fine as long as we call asyncTaskCanceled() too.
       const recurring = true;
-      if (type === 'PROMISE')
-        this.promiseIds.add(asyncId);
-      else
-        inspector.asyncTaskScheduled(type, asyncId, recurring);
+      inspector.asyncTaskScheduled(type, asyncId, recurring);
     },
 
     before(asyncId) {
-      if (this.promiseIds.has(asyncId))
-        return;
       inspector.asyncTaskStarted(asyncId);
     },
 
     after(asyncId) {
-      if (this.promiseIds.has(asyncId))
-        return;
       inspector.asyncTaskFinished(asyncId);
     },
 
     destroy(asyncId) {
-      if (this.promiseIds.has(asyncId))
-        return this.promiseIds.delete(asyncId);
       inspector.asyncTaskCanceled(asyncId);
     },
   });
-
-  hook.promiseIds = new SafeSet();
+  hook[kNoPromiseHook] = true;
 }
 
 function enable() {
diff --git a/test/parallel/test-inspector-async-call-stack.js b/test/parallel/test-inspector-async-call-stack.js
@@ -27,7 +27,7 @@ function verifyAsyncHookEnabled(message) {
   assert.strictEqual(async_hook_fields[kTotals], 4,
                      `${async_hook_fields[kTotals]} !== 4: ${message}`);
   const promiseHooks = getPromiseHooks();
-  assert.notDeepStrictEqual(
+  assert.deepStrictEqual(  // Inspector async hooks should not enable promise hooks
     promiseHooks, emptyPromiseHooks,
     `${message}: promise hooks ${inspect(promiseHooks)}`
   );
