debugger: preserve overlapping CDP request state - #64467
Conversation
|
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #64467 +/- ##
========================================
Coverage 90.23% 90.23%
========================================
Files 741 739 -2
Lines 241604 241775 +171
Branches 45520 45571 +51
========================================
+ Hits 218010 218168 +158
+ Misses 15120 15101 -19
- Partials 8474 8506 +32
🚀 New features to boost your workflow:
|
7d883b8 to
2eb6ee1
Compare
|
The regression test which used internal-state was removed in @joyeecheung Can you please review the PR? |
|
Stress test on main: https://ci.nodejs.org/job/node-stress-single-test/797/ |
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
2eb6ee1 to
9bc57a3
Compare
There was a problem hiding this comment.
Stress test did not prove or disprove this makes any difference in the flakiness (also test-debugger-probe-failure-process-exit flake has been gone by itself on the main branch). I am not sure if the path is actually reachable in real applications but it seems harmless so LGTM.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Landed in 9024119 |
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 PR-URL: #64467 Refs: https://github.com/nodejs/reliability/issues?q=sort%3Aupdated-desc%20test-debugger-probe-failure-process-exit Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
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 PR-URL: #64467 Refs: https://github.com/nodejs/reliability/issues?q=sort%3Aupdated-desc%20test-debugger-probe-failure-process-exit Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
A breakpoint pause can arrive before the preceding
Debugger.resumerequest has finished. When that happens, the probe evaluation becomes the
current
inFlightrequest, but the older resume request clears it when itfinishes.
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
inFlightonly when it still ownsthat 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
Assisted-by: openai:gpt-5.6-sol