Repository: nodejs/node
Pull request: 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.

Refs: https://issues.chromium.org/issues/390581540

Quoting myself from the DevTools issue:

> From some code archeology my guess is:
> 1. Node.js uses its own async hooks to call asyncTaskScheduled for all the async tasks so that they show up in the inspector https://github.com/nodejs/node/pull/13870
> 2. The Node.js async hooks implementation unconditionally adds Promise hooks because the API is designed in a way that allows all promises to pass through the hook (with resource type being "Promise" that allows users to filter through)
> 3. The implementation in 1 does not actually need the promise hooks unconditionally set in 2. In fact it is counter-productive as Node.js needs to filter them out to avoid extra counting in the inspector https://github.com/nodejs/node/pull/17118
>
> I think it can probably be cleaned up by just carving out some internal path in the Node.js async hooks implementation to skip that useless promise hook.

When running `node --inspect-brk await.mjs` on this snippet

```js
const test = await Promise.resolve('test')
console.log(test);
```

Previously, this is what you'd see in the Chrome DevTools

<img width="900" alt="Screenshot 2025-02-20 at 16 05 39" src="https://github.com/user-attachments/assets/ef0a7aa9-9e0b-4a5f-8f15-7cd5760b77b1" />

After this patch

<img width="883" alt="Screenshot 2025-02-20 at 16 05 55" src="https://github.com/user-attachments/assets/5b25ff8d-dc60-4173-9708-b78fd715dc83" />



<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: lib/async_hooks.js
patch: @@ -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;
   }

filename: lib/internal/async_hooks.js
patch: @@ -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,
 };

filename: lib/internal/inspector_async_hook.js
patch: @@ -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() {

filename: test/parallel/test-inspector-async-call-stack.js
patch: @@ -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)}`
   );