From e2bc395c4206ca96b921900c030da30c05d38447 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Sun, 23 Feb 2025 02:07:21 +0100
Subject: [PATCH] inspector: skip promise hook in the inspector async hook

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: https://github.com/nodejs/node/pull/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>
---
 lib/async_hooks.js                            |  7 +++++--
 lib/internal/async_hooks.js                   |  2 ++
 lib/internal/inspector_async_hook.js          | 19 +++----------------
 .../test-inspector-async-call-stack.js        |  2 +-
 4 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/lib/async_hooks.js b/lib/async_hooks.js
index 5e381d5b3c8a28..7dc7bfab328498 100644
--- 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
index c20be0eb15b80c..366ede83da2e5a 100644
--- 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
index e65fd05619a4e7..db78e05f0a1da9 100644
--- a/lib/internal/inspector_async_hook.js
+++ b/lib/internal/inspector_async_hook.js
@@ -1,9 +1,5 @@
 'use strict';
 
-const {
-  SafeSet,
-} = primordials;
-
 let hook;
 let config;
 
@@ -11,6 +7,7 @@ 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
index d42cf42c7d7aab..80e9ba3c228747 100644
--- 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)}`
   );