Skip to content

debugger: disambiguate probe location binding#63286

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

debugger: disambiguate probe location binding#63286
joyeecheung wants to merge 1 commit into
nodejs:mainfrom
joyeecheung:probe-loc

Conversation

@joyeecheung
Copy link
Copy Markdown
Member

In probe mode, --probe utils.js:10 can match multiple scripts (e.g. src/utils.js and lib/utils.js) because the matcher uses a path-separator-anchored URL suffix (similar to how e.g. gdb/lldb behaves). Previously the report echoed only the user's request in hit events, so a user seeing two hits at utils.js:10 could not tell which script each hit came from. In addition, previously when the column was omitted we bound to 1 which was technically different from how CDP binds omitted columns (to the first executable column on the line).

This patch clarifies the semantics by tracking the scripts via Debugger.scriptParsed, as recommended in the CDP docs, and reports the actual execution location as results[i].location. The same shape can be reused in the future for source maps or additional events in attach mode.

This bumps the schema version because target is now { suffix, line, column? } instead of a positional array for clarity. We picked suffix as the field name in case we introduce other matching modes in the future. column is omitted when the user did not supply one, and the actual resolved column is reported in the hit event instead.

This patch also adds more tests for column-specific bindings, multi-location resolution via require(cjs), and late script binding via dynamic import(cjs) and require(esm).

@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 13, 2026
In probe mode, `--probe utils.js:10` can match multiple scripts
(e.g. `src/utils.js` and `lib/utils.js`) because the matcher uses
a path-separator-anchored URL suffix (similar to how e.g. gdb/lldb
behaves). Previously the report echoed only the user's request in
hit events, so a user seeing two hits at `utils.js:10` could not
tell which script each hit came from. In addition, previously
when the column was omitted we bound to 1 which was technically
different from how CDP binds omitted columns (to the first
executable column on the line).

This patch clarifies the semantics by tracking the scripts via
`Debugger.scriptParsed`, as recommended in the CDP docs, and
reports the actual execution location as `results[i].location`.
The same shape can be reused in the future for source maps or
additional events in attach mode.

This bumps the schema version because `target` is now
`{ suffix, line, column? }` instead of a positional array for
clarity. We picked `suffix` as the field name in case we
introduce other matching modes in the future. `column` is
omitted when the user did not supply one, and the actual
resolved column is reported in the hit event instead.

This patch also adds more tests for column-specific bindings,
multi-location resolution via require(cjs), and late script
binding via dynamic import(cjs) and require(esm).

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

@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

❌ Patch coverage is 97.47899% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.05%. Comparing base (78bbee3) to head (c45f945).
⚠️ Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/debugger/inspect_probe.js 97.29% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #63286    +/-   ##
========================================
  Coverage   90.04%   90.05%            
========================================
  Files         713      714     +1     
  Lines      225003   225305   +302     
  Branches    42536    42584    +48     
========================================
+ Hits       202606   202893   +287     
- Misses      14177    14206    +29     
+ Partials     8220     8206    -14     
Files with missing lines Coverage Δ
lib/internal/debugger/inspect_helpers.js 97.26% <100.00%> (+0.07%) ⬆️
lib/internal/debugger/inspect_probe.js 78.72% <97.29%> (+2.38%) ⬆️

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

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.

3 participants