inspector: skip promise hook in the inspector async hook

[joyeecheung]

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 Report async tasks to V8 Inspector #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 inspector: no async tracking for promises #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

const test = await Promise.resolve('test')
console.log(test);

Previously, this is what you'd see in the Chrome DevTools

After this patch

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/65341/

[jasnell]

LGTM modulo the lint issue

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/65343/

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.37%. Comparing base (06d5701) to head (0349af9).
Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #57148      +/-   ##
==========================================
+ Coverage   89.02%   90.37%   +1.35%     
==========================================
  Files         665      628      -37     
  Lines      193408   184042    -9366     
  Branches    37270    35951    -1319     
==========================================
- Hits       172181   166335    -5846     
+ Misses      13888    10876    -3012     
+ Partials     7339     6831     -508     

Files with missing lines	Coverage Δ
lib/async_hooks.js	99.66% <100.00%> (+<0.01%)	⬆️
lib/internal/async_hooks.js	99.36% <100.00%> (+<0.01%)	⬆️
lib/internal/inspector_async_hook.js	100.00% <100.00%> (ø)

... and 81 files with indirect coverage changes

[joyeecheung]

Looks like I forgot to stage the symbol definition before I pushed (the undefined symbol still worked after destruction because..it will just get coerced to "undefined" and is still a valid key). Thanks @43081j for spotting it.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/65346/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/65349/

[legendecas]

This significant improves breakpoint experience!

@joyeecheung please correct me if i'm wrong -- even though it is linked in the OP, this does not fix https://issues.chromium.org/issues/390581540 that devtools can not autocomplete in the console, right? For the autocomplete issue, there was an issue #56637 (comment) as well that V8 in Node.js assumes all devtools scripts failed side-effect check.

[joyeecheung]

Yes, this is just an adjacent issue to the autocomplete issue in DevTools (that might interfere with debugging the actual issue, I suppose).

[GeoffreyBooth]

I've been wanting this for years. Thank you!