Skip to content

debugger: surface inspector failures in probe mode#63437

Open
joyeecheung wants to merge 1 commit into
nodejs:mainfrom
joyeecheung:probe-errors
Open

debugger: surface inspector failures in probe mode#63437
joyeecheung wants to merge 1 commit into
nodejs:mainfrom
joyeecheung:probe-errors

Conversation

@joyeecheung
Copy link
Copy Markdown
Member

@joyeecheung joyeecheung commented May 19, 2026

This addresses a few TODOs left from the initial implementation around the error handling when the inspector session fails mid-probe - for example because a probe expression terminates the target, severs the inspector connection, or hangs the evaluation. Before this patch, these failures would cause the probe report to silently miss hits or hit the session timeout. A consumer of these reports could not tell whether the recorded hits were reliable or a partial trace cut short by an inspector-side failure.

This patch surfaces those cases as a new probe_failure terminal error event on the report, with best-effort attribution to the implicated probe via error.probe and additional context on error.details. Together with probe_timeout, it marks the report as best-effort, and the probing process exits 1 in both cases. probe_target_exit and clean completed/miss reports continue to exit 0 because the recorded hits in those cases remain ground truth, so tooling can use the exit code as a quick trustworthiness signal.

The per-hit error is reshaped to { message, details? } so that the per-hit and terminal errors share a top-level shape. Both fields are informative-only, only their top-level type is stable.

Refs: #62713

@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 May 19, 2026
@joyeecheung joyeecheung force-pushed the probe-errors branch 4 times, most recently from e9c4534 to 9f3a1d9 Compare May 19, 2026 17:34
@joyeecheung joyeecheung marked this pull request as draft May 19, 2026 17:46
@joyeecheung joyeecheung force-pushed the probe-errors branch 2 times, most recently from fe70e9b to 32eca19 Compare May 19, 2026 18:32
@joyeecheung joyeecheung marked this pull request as ready for review May 19, 2026 18:40
@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2026

Codecov Report

❌ Patch coverage is 69.80519% with 93 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.03%. Comparing base (b5659d1) to head (6f5c4a8).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/debugger/inspect_probe.js 69.80% 91 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63437      +/-   ##
==========================================
- Coverage   90.06%   90.03%   -0.03%     
==========================================
  Files         714      714              
  Lines      225918   226098     +180     
  Branches    42729    42796      +67     
==========================================
+ Hits       203466   203573     +107     
- Misses      14231    14320      +89     
+ Partials     8221     8205      -16     
Files with missing lines Coverage Δ
lib/internal/debugger/inspect_probe.js 79.89% <69.80%> (+1.17%) ⬆️

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

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@joyeecheung joyeecheung force-pushed the probe-errors branch 2 times, most recently from 4d7cd4f to 40e4bbe Compare May 20, 2026 23:17
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

This addresses a few TODOs left from the initial implementation
around the error handling when the inspector session fails
mid-probe - for example because a probe expression terminates
the target, severs the inspector connection, or hangs the
evaluation. Before this patch, these failures would cause the
probe report to silently miss hits or hit the session timeout.
A consumer of these reports could not tell whether the
recorded hits were reliable or a partial trace cut short
by an inspector-side failure.

This patch surfaces those cases as a new `probe_failure` terminal
`error` event on the report, with best-effort attribution to the
implicated probe via `error.probe` and additional context on
`error.details`. Together with `probe_timeout`, it marks the
report as best-effort, and the probing process exits 1 in both
cases. `probe_target_exit` and clean `completed`/`miss` reports
continue to exit 0 because the recorded hits in those cases remain
ground truth, so tooling can use the exit code as a quick
trustworthiness signal. `error.message` now also contains
actionable recovery hints whereever possible.

The per-hit `error` is reshaped to `{ message, details? }`
so that the per-hit and terminal errors share a top-level shape.
Both fields are informative-only, only their top-level type is stable.

Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com>
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@joyeecheung
Copy link
Copy Markdown
Member Author

Looks like CI is happy @hybrist @legendecas would you mind taking a look? Thanks!

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. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants