Skip to content

debugger: preserve overlapping CDP request state#64467

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:flaky-test-debugger-probe-failure-process-exit
Open

debugger: preserve overlapping CDP request state#64467
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:flaky-test-debugger-probe-failure-process-exit

Conversation

@trivikr

@trivikr trivikr commented Jul 13, 2026

Copy link
Copy Markdown
Member

A breakpoint pause can arrive before the preceding Debugger.resume
request has finished. When that happens, the probe evaluation becomes the
current inFlight request, but the older resume request clears it when it
finishes.

If the target exits during the probe evaluation, we then lose the
information needed to attribute the failure to that probe. This caused the
test to intermittently report the probe as still pending, most notably on
Windows CI.

This change makes each request clear inFlight only when it still owns
that state. It also adds a deterministic regression test that reproduces
the ordering without relying on inspector timing.

Refs: https://github.com/nodejs/reliability/issues?q=sort%3Aupdated-desc%20test-debugger-probe-failure-process-exit

Example Error
  ---
  duration_ms: 2673.03700
  severity: fail
  exitcode: 1
  stack: |-
    [process 13216]: --- stderr ---
    INSPECT_PROBE 13216: child stderr: "Debugger listening on ws://127.0.0.1:51299/8810a2af-54fc-49c1-9dd9-888d0b156452\r\nFor help, see: https://nodejs.org/learn/getting-started/debugging\r\n"
    INSPECT_PROBE 13216: CDP -> Runtime.enable
    INSPECT_PROBE 13216: child stderr: "Debugger attached.\r\n"
    INSPECT_PROBE 13216: CDP <- Runtime.enable (success)
    INSPECT_PROBE 13216: CDP -> Debugger.enable
    INSPECT_PROBE 13216: CDP <- Debugger.enable (success)
    INSPECT_PROBE 13216: CDP -> Debugger.setBreakpointByUrl
    INSPECT_PROBE 13216: CDP <- Debugger.setBreakpointByUrl (success)
    INSPECT_PROBE 13216: breakpoint set: id=2:7:0:^(.*[\/\\])?probe-exits-during-probe\.js$ urlRegex=^(.*[\/\\])?probe-exits-during-probe\.js$ locations=[]
    INSPECT_PROBE 13216: CDP -> Runtime.runIfWaitingForDebugger
    INSPECT_PROBE 13216: CDP <- Runtime.runIfWaitingForDebugger (success)
    INSPECT_PROBE 13216: scriptParsed: scriptId=89 url=evalmachine.<anonymous>, length=4
    INSPECT_PROBE 13216: scriptParsed: scriptId=90 url=file:///c:/workspace/node-test-binary-windows-js-suites/node/test/fixtures/debugger/probe-exits-during-probe.js, length=138
    INSPECT_PROBE 13216: paused: finished=0, reason=Break on start hitBreakpoints=[]
    INSPECT_PROBE 13216: CDP -> Debugger.resume
    INSPECT_PROBE 13216: paused: finished=0, reason=other hitBreakpoints=["2:7:0:^(.*[\\/\\\\])?probe-exits-during-probe\\.js$"]
    INSPECT_PROBE 13216: CDP -> Debugger.evaluateOnCallFrame, probe=0
    INSPECT_PROBE 13216: CDP <- Debugger.resume (success)
    INSPECT_PROBE 13216: child stderr: "Waiting for the debugger to disconnect...\r\n"
    INSPECT_PROBE 13216: disconnect sentinel detected, resetting client
    INSPECT_PROBE 13216: CDP <- Debugger.evaluateOnCallFrame error: ERR_DEBUGGER_ERROR
    INSPECT_PROBE 13216: recordInspectorFailure "Target process exited during probe evaluation": inFlight=null, lastProbeIndex=null, cdpError=undefined
    INSPECT_PROBE 13216: finish: exitCode=1, terminal=error
    
    [process 13216]: --- stdout ---
    {"v":2,"probes":[{"expr":"exitDuringProbe()","target":{"suffix":"probe-exits-during-probe.js","line":8}}],"results":[{"event":"error","pending":[0],"error":{"code":"probe_failure","message":"Target process exited during probe evaluation before probes: probe-exits-during-probe.js:8. If the failure repeats, review the probe expression.","stderr":""}}]}
    
    [process 13216]: status = 1, signal = null
    c:\workspace\node-test-binary-windows-js-suites\node\test\common\child_process.js:112
        throw error;
        ^
    
    Error: - stdout did not match expectation, checker throws:
    AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
    + actual - expected
    ... Skipped lines
    
      {
        probes: [
          {
            expr: 'exitDuringProbe()',
            target: {
    ...
            error: {
    -         message: 'Probe evaluation did not complete'
    -       },
    -       event: 'hit',
    -       hit: 1,
    -       location: {
    -         column: 1,
    -         line: 8,
    -         url: 'file:///c:/workspace/node-test-binary-windows-js-suites/node/test/fixtures/debugger/probe-exits-during-probe.js'
    -       },
    -       probe: 0
    -     },
    -     {
    -       error: {
              code: 'probe_failure',
    +         message: 'Target process exited during probe evaluation before probes: probe-exits-during-probe.js:8. If the failure repeats, review the probe expression.',
    -         details: {
    -           lastCdpMethod: 'Debugger.evaluateOnCallFrame'
    -         },
    -         message: 'Target process exited during probe evaluation. If the failure repeats, review the probe expression.',
    -         probe: 0,
              stderr: ''
            },
            event: 'error',
    +       pending: [
    +         0
    +       ]
    -       pending: []
          }
        ],
        v: 2
      }
    
        at assertProbeJson (c:\workspace\node-test-binary-windows-js-suites\node\test\common\debugger-probe.js:65:10)
        at stdout (c:\workspace\node-test-binary-windows-js-suites\node\test\parallel\test-debugger-probe-failure-process-exit.js:25:5)
        at checkOutput (c:\workspace\node-test-binary-windows-js-suites\node\test\common\child_process.js:52:7)
        at expectSyncExit (c:\workspace\node-test-binary-windows-js-suites\node\test\common\child_process.js:129:32)
        at spawnSyncAndExit (c:\workspace\node-test-binary-windows-js-suites\node\test\common\child_process.js:143:10)
        at Object.<anonymous> (c:\workspace\node-test-binary-windows-js-suites\node\test\parallel\test-debugger-probe-failure-process-exit.js:17:1)
        at Module._compile (node:internal/modules/cjs/loader:1937:14)
        at Object..js (node:internal/modules/cjs/loader:2077:10)
        at Module.load (node:internal/modules/cjs/loader:1659:32)
        at Module._load (node:internal/modules/cjs/loader:1451:12) {
      generatedMessage: true,
      code: 'ERR_ASSERTION',
      actual: [Object],
      expected: [Object],
      operator: 'deepStrictEqual',
      diff: 'simple'
    }
        at Object.<anonymous> (c:\workspace\node-test-binary-windows-js-suites\node\test\parallel\test-debugger-probe-failure-process-exit.js:17:1)
        at Module._compile (node:internal/modules/cjs/loader:1937:14)
        at Object..js (node:internal/modules/cjs/loader:2077:10)
        at Module.load (node:internal/modules/cjs/loader:1659:32)
        at Module._load (node:internal/modules/cjs/loader:1451:12)
        at wrapModuleLoad (node:internal/modules/cjs/loader:261:19)
        at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5)
        at node:internal/main/run_main_module:33:47 {
      options: {
        cwd: 'c:\\workspace\\node-test-binary-windows-js-suites\\node\\test\\fixtures\\debugger',
        env: { NODE_DEBUG: 'inspect_probe' }
      },
      command: 'c:\\workspace\\node-test-binary-windows-js-suites\\node\\Release\\node.exe inspect --json --probe probe-exits-during-probe.js:8 --expr exitDuringProbe() probe-exits-during-probe.js'
    }

Assisted-by: openai:gpt-5.6-sol

@nodejs-github-bot nodejs-github-bot added debugger Issues and PRs related to the debugger subsystem. needs-ci PRs that need a full CI run. labels Jul 13, 2026
@trivikr

trivikr commented Jul 13, 2026

Copy link
Copy Markdown
Member Author

The stress test isn't runnable on Windows environments mentioned in reports, as clang is not available

Discussion on Slack: https://openjs-foundation.slack.com/archives/C03BJP63CH0/p1783477923723679

@trivikr trivikr added request-ci Add this label to start a Jenkins CI on a PR. flaky-test Issues and PRs related to the tests with unstable failures on the CI. labels Jul 13, 2026
@trivikr trivikr requested a review from joyeecheung July 13, 2026 06:00
@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.24%. Comparing base (b4c7be3) to head (7d883b8).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64467      +/-   ##
==========================================
+ Coverage   90.23%   90.24%   +0.01%     
==========================================
  Files         741      741              
  Lines      241604   241607       +3     
  Branches    45520    45522       +2     
==========================================
+ Hits       218010   218042      +32     
+ Misses      15120    15114       -6     
+ Partials     8474     8451      -23     
Files with missing lines Coverage Δ
lib/internal/debugger/inspect_probe.js 81.09% <100.00%> (+0.05%) ⬆️

... and 25 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.

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jul 13, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 13, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Comment thread test/parallel/test-debugger-probe-overlapping-cdp.js Outdated
A Debugger.paused event can arrive before the response to the resume
request that triggered it. The resulting probe evaluation replaces the
resume request in `inFlight`, but the resume cleanup then clears the
newer request's state.

Only clear `inFlight` when it still refers to the request being
completed. This preserves probe attribution when the target exits
during evaluation. Clarify the existing end-to-end test coverage
for this case.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.6-sol
@trivikr trivikr force-pushed the flaky-test-debugger-probe-failure-process-exit branch from 7d883b8 to 2eb6ee1 Compare July 13, 2026 15:07
@trivikr trivikr requested a review from joyeecheung July 13, 2026 15:09
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

debugger Issues and PRs related to the debugger subsystem. flaky-test Issues and PRs related to the tests with unstable failures on the CI. needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants