From ce8a375cdd2712cdbfa994b5b0b7bec9392d4055 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 20 Feb 2025 14:52:10 +0100 Subject: [PATCH 1/3] 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. --- lib/async_hooks.js | 7 +++++-- lib/internal/inspector_async_hook.js | 15 +++------------ test/parallel/test-inspector-async-call-stack.js | 2 +- 3 files changed, 9 insertions(+), 15 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/inspector_async_hook.js b/lib/internal/inspector_async_hook.js index e65fd05619a4e7..2e12ee64fb0161 100644 --- a/lib/internal/inspector_async_hook.js +++ b/lib/internal/inspector_async_hook.js @@ -11,6 +11,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 +20,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)}` ); From 6819706ec0aae25072c11f9b362928d43e8d552d Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 20 Feb 2025 16:27:09 +0100 Subject: [PATCH 2/3] fixup! inspector: skip promise hook in the inspector async hook --- lib/internal/inspector_async_hook.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/internal/inspector_async_hook.js b/lib/internal/inspector_async_hook.js index 2e12ee64fb0161..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; From 0349af91f13b855c51ebc8220d3396660e097cab Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 20 Feb 2025 20:15:18 +0100 Subject: [PATCH 3/3] fixup! fixup! inspector: skip promise hook in the inspector async hook --- lib/internal/async_hooks.js | 2 ++ 1 file changed, 2 insertions(+) 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, };