Skip to content

test: deflake test-inspect-async-hook-setup-at-inspect - #65584

Open
christianaurichzm wants to merge 1 commit into
nodejs:mainfrom
christianaurichzm:deflake-inspect-async-hook-setup
Open

test: deflake test-inspect-async-hook-setup-at-inspect#65584
christianaurichzm wants to merge 1 commit into
nodejs:mainfrom
christianaurichzm:deflake-inspect-async-hook-setup

Conversation

@christianaurichzm

Copy link
Copy Markdown
Contributor

parallel/test-inspect-async-hook-setup-at-inspect failed in 4 PRs in the daily reliability report, on win2019, win2022 and ubuntu2404-x64, always with:

AssertionError [ERR_ASSERTION]: callFrames,reason,hitBreakpoints contains "asyncStackTrace" property

The test treats the response to Debugger.setAsyncCallStackDepth as proof that async stack recording is on. It is not: dispatched from a V8 interrupt, the agent defers enabling the async hook to an immediate that the subsequent pause
prevents from running. Sending the command after the first pause dispatches it from the nested message loop instead, where the hook is enabled inline.

Reproduction

The race window is normally a few microseconds wide. Widening it in the test script makes the failure reliably reproducible:

- const waiting = setInterval(() => { debugger; }, 50);
+ const waiting = setInterval(() => { const t = Date.now(); while (Date.now() - t < 8); debugger; }, 10);
widened as-is, 24 concurrent processes
before 59 / 60 18 / 9600 (0.19%)
after 0 / 60 0 / 9600

All measurements are Linux x64 on 16 cores, so the 24 processes are deliberately oversubscribed. All 18 baseline failures carried the signature above.

Unwidened, node-stress-single-test is weak evidence here: it runs iterations sequentially on an idle worker, where the rate is lower still, and even at the 0.19% measured under contention 100 iterations have an 83% chance of coming back
clean. Widened, it needs no concurrency at all - 38 / 40 failures running one process at a time on an idle machine, 34 / 40 pinned to a single core.

Scope

Deferring the hook is deliberate: calling into JS from a V8 interrupt is not safe (#65028). This corrects an assumption in the test, not the runtime. Whether the protocol should expose when the hook becomes active is a separate matter.

Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md
Refs: #65028

The test sends Debugger.setAsyncCallStackDepth in the same batch as
Runtime.enable and Debugger.enable, and treats the command response as
proof that async stack recording is on. It is not.

While the target runs JS the command is dispatched from a V8 interrupt,
where Agent::SyncAsyncHookState() cannot call into JS and defers
enabling the async hook to an immediate, answering the command anyway.
The `debugger` in the polling interval then pauses the target before
that immediate runs, and the nested message loop does not process
immediates, so setupTimeoutWithBreak() schedules the timer with the
hook still disabled and Debugger.paused arrives without asyncStackTrace.

Send the command after the first pause instead. Messages dispatched
while the target is paused come from the nested message loop rather
than an interrupt, so the hook is enabled inline and the response is
the barrier the test assumed it to be.

Deferring is deliberate: calling into JS from a V8 interrupt is not
safe. This corrects an assumption of the test, not the runtime.

Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Aug 27, 2026
@MikeMcC399 MikeMcC399 added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Aug 27, 2026
@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.05%. Comparing base (9baabd4) to head (f7a8857).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65584      +/-   ##
==========================================
- Coverage   90.07%   90.05%   -0.02%     
==========================================
  Files         751      751              
  Lines      254875   254875              
  Branches    48108    48118      +10     
==========================================
- Hits       229579   229532      -47     
- Misses      16466    16518      +52     
+ Partials     8830     8825       -5     

see 33 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@panva panva added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Aug 27, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 27, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. flaky-test Issues and PRs related to the tests with unstable failures on the CI. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants