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>
This commit is contained in:
Joyee Cheung 2025-02-23 02:07:21 +01:00 committed by GitHub
parent 7f768f6768
commit e2bc395c42
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 11 additions and 19 deletions

View File

@ -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;
}

View File

@ -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,
};

View File

@ -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() {

View File

@ -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)}`
);