Skip to content

Commit caaa240

Browse files
committed
inspector: add --cond to node inspect probe mode
On a hot path, the probe can record every hit and require filtering afterwards. This patch adds a per-probe `--cond <expr>` option that allows limiting the hit to only when the expression is truthy at the probe location. V8 evaluates it as the breakpoint's native condition, so the target is not paused when it does not hold, and a condition that throws is treated as false. Since in CDP, a location can only carry one breakpoint per URL pattern, probes sharing a location must share one condition (or none). Conflicting conditions are rejected. Example: ```js // app.js let total = 0; for (let i = 0; i < 10; i++) { total += i; // line 4 } ``` ``` $ out/Release/node inspect --probe app.js:4 --expr 'total' \ --cond 'i % 3 === 0' app.js ``` ``` Hit 1 at file:///path/to/app.js:3:3 total = 0 Hit 2 at file:///path/to/app.js:3:3 total = 3 Hit 3 at file:///path/to/app.js:3:3 total = 15 Hit 4 at file:///path/to/app.js:3:3 total = 36 Completed ``` Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com>
1 parent d7a658a commit caaa240

9 files changed

Lines changed: 316 additions & 17 deletions

doc/api/debugger.md

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,10 @@ added:
237237
- v26.1.0
238238
- v24.16.0
239239
changes:
240+
- version: REPLACEME
241+
pr-url: https://github.com/nodejs/node/pull/REPLACEME
242+
description: Add per-probe `--cond <expr>` option to only record a hit when the
243+
condition is truthy at the probe location.
240244
- version: v26.4.0
241245
pr-url: https://github.com/nodejs/node/pull/63704
242246
description: Add per-probe `--max-hit <n>` option to limit evaluated hits and finish
@@ -269,8 +273,8 @@ printf-style debugging without having to modify the application code and
269273
clean up afterwards. It also supports structured JSON output for tool use.
270274

271275
```console
272-
$ node inspect --probe <file>:<line>[:<col>] --expr <expr> [--max-hit <n>]
273-
[--probe <file>:<line>[:<col>] --expr <expr> [--max-hit <n>] ...]
276+
$ node inspect --probe <file>:<line>[:<col>] --expr <expr> [--cond <expr>] [--max-hit <n>]
277+
[--probe <file>:<line>[:<col>] --expr <expr> [--cond <expr>] [--max-hit <n>] ...]
274278
[--json] [--preview] [--timeout=<ms>] [--port=<port>]
275279
[--] [<node-option> ...] <script> [<script-args> ...]
276280
```
@@ -283,6 +287,9 @@ $ node inspect --probe <file>:<line>[:<col>] --expr <expr> [--max-hit <n>]
283287
* `--expr <expr>`: JavaScript expression to evaluate whenever execution reaches
284288
the location specified by the preceding `--probe`.
285289
Must immediately follow the `--probe` it belongs to.
290+
* `--cond <expr>`: An optional condition for the probe location. The probe only
291+
records a hit when `<expr>` is truthy at the location. A condition that throws
292+
is treated as false.
286293
* `--max-hit <n>`: An optional per-probe limit on the number of times the probe
287294
can be hit. When not specified, there's no hit limit. When any probe reaches
288295
its hit limit, the probing process will detach and report the results. The process
@@ -298,14 +305,17 @@ $ node inspect --probe <file>:<line>[:<col>] --expr <expr> [--max-hit <n>]
298305
will listen. Defaults to `0`, which requests a random port.
299306
* `--` is optional unless the child needs its own Node.js flags.
300307

301-
Additional rules about the `--probe` and `--expr` arguments:
308+
Additional rules about the composition of the options:
302309

303310
* `--probe <file>:<line>[:<col>]` and `--expr <expr>` are strict pairs. Each
304311
`--probe` must be followed immediately by exactly one `--expr`.
305-
* `--max-hit <n>` is an optional per-probe option that applies to the most recent
306-
`--probe`/`--expr` pair. It may not appear before the first `--probe` or
307-
between a `--probe` and its matching `--expr`, and may be given at most once
308-
per probe.
312+
* `--cond <expr>` and `--max-hit <n>` are optional modifiers written _after_ the
313+
`--probe`/`--expr` pair they apply to, each at most once per pair. They may not
314+
appear before the first `--probe` or between a `--probe` and its matching
315+
`--expr`.
316+
* `--max-hit` scopes to the `--probe`/`--expr` pair it follows, so pairs
317+
sharing a location may set different limits. `--cond` scopes to the whole
318+
location, probes sharing a location must share one condition (or none).
309319
* `--timeout`, `--json`, `--preview`, and `--port` are global probe options
310320
for the whole probe session. They may appear before or between probe pairs,
311321
but not between a `--probe` and its matching `--expr`.
@@ -321,6 +331,8 @@ $ node inspect --probe app.js:10 --expr "user"
321331
--json --preview -- --no-warnings app.js --arg-for-app=foo
322332
```
323333

334+
<!-- TODO(joyeecheung): add more examples for different options -->
335+
324336
### Probe output format
325337

326338
When the probe session ends, the probing process prints a final report of all the probe hits and results.
@@ -380,6 +392,7 @@ $ node inspect --json --probe cli.js:5 --expr 'rss' cli.js
380392
"suffix": "cli.js",
381393
"line": 5
382394
}
395+
// `condition` is present only when the probe was given a --cond expression.
383396
// `maxHit` is present only when the probe was given a --max-hit limit.
384397
}
385398
],
@@ -481,6 +494,10 @@ When multiple `--probe`/`--expr` pairs share the same `--probe`, the
481494
expressions will be evaluated on the same pause in the order they appear
482495
on the command line.
483496

497+
For each location, there can only be at most one `--cond` (or none).
498+
Multiple `--probe`/`--expr` pairs with conflicting conditions
499+
at the same location will be rejected at launch time.
500+
484501
```js
485502
// app.js
486503
const x = { x: 42 }; // line 2

lib/internal/debugger/inspect.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ function parseInteractiveArgs(args) {
268268

269269
const kInspectArgOptions = {
270270
'__proto__': null,
271+
'cond': { type: 'string' },
271272
'expr': { type: 'string' },
272273
'help': { type: 'boolean', short: 'h' },
273274
'json': { type: 'boolean' },

lib/internal/debugger/inspect_helpers.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ function writeInspectUsageAndExit(invokedAs, message, exitCode) {
6969
}
7070
out.write(`Usage: ${invokedAs} [--port=<port>] [<node-option> ...]
7171
[<script> [<script-args>] | <host>:<port> | -p <pid>]
72-
${invokedAs} --probe <file>:<line>[:<col>] --expr <expr> [--max-hit <n>]
73-
[--probe <file>:<line>[:<col>] --expr <expr> [--max-hit <n>] ...]
72+
${invokedAs} --probe <file>:<line>[:<col>] --expr <expr> [--cond <expr>] [--max-hit <n>]
73+
[--probe <file>:<line>[:<col>] --expr <expr> [--cond <expr>] [--max-hit <n>] ...]
7474
[--json] [--preview] [--timeout=<ms>] [--port=<port>]
7575
[--] [<node-option> ...] <script> [<script-args> ...]
7676
@@ -109,6 +109,9 @@ Options:
109109
preceding --probe each time execution reaches it.
110110
Avoid probing let/const-bound variables at their
111111
declaration site or a ReferenceError may be thrown.
112+
--cond <expr> Optional condition for the probe location. The probe only
113+
records a hit when <expr> is truthy at the location. A
114+
condition that throws is treated as false.
112115
--max-hit <n> Per-probe limit on evaluated hits. When not specified,
113116
there's no hit limit. When any probe reaches its hit LIMIT,
114117
the probing process will detach and report the results.
@@ -121,6 +124,9 @@ Options:
121124
Semantics:
122125
* Multiple --probe/--expr pairs are allowed. Same-location --probes share
123126
a pause and scope, their --exprs are evaluated in command-line order.
127+
* --max-hit scopes to one --probe/--expr pair, so same-location pairs may set
128+
different limits. --cond scopes to the location, probes sharing a location
129+
must all share one condition (or none).
124130
* --probe utils.js:<line>[:<col>] matches every loaded utils.js. Pass a
125131
fuller path e.g. src/utils.js to narrow the match.
126132
* Use -- before any Node.js flags intended for the child process.

lib/internal/debugger/inspect_probe.js

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const {
2121
StringPrototypeIncludes,
2222
StringPrototypeSlice,
2323
StringPrototypeStartsWith,
24+
StringPrototypeTrim,
2425
Symbol,
2526
} = primordials;
2627

@@ -87,6 +88,8 @@ const kInspectPortRegex = /^--inspect-port=(\d+)$/;
8788
* @typedef {object} Probe
8889
* @property {string} expr Expression to evaluate on hit.
8990
* @property {ProbeTarget} target User's original --probe request shape.
91+
* @property {string} [condition] Condition from --cond. V8 only breaks when it is truthy.
92+
* Scoped to the location, so probes sharing a location all carry the same value.
9093
* @property {number} maxHit Per-probe hit limit from --max-hit. Infinity when unlimited.
9194
* @property {number} hits Count of hits observed.
9295
*/
@@ -130,6 +133,12 @@ function formatTargetText(target) {
130133
return column === undefined ? `${suffix}:${line}` : `${suffix}:${line}:${column}`;
131134
}
132135

136+
// Identity of a probe location. Probes sharing a key share one V8 breakpoint,
137+
// so this must stay in sync between condition validation and breakpoint setup.
138+
function locationKey(target) {
139+
return `${target.suffix}\n${target.line}\n${target.column ?? ''}`;
140+
}
141+
133142
function formatPendingProbeLocations(probes, pending) {
134143
const seen = new SafeSet();
135144
for (const probeIndex of pending) {
@@ -391,6 +400,22 @@ function parseProbeTokens(tokens, args) {
391400
probe.maxHit = parseUnsignedInteger(token.value, 'max-hit');
392401
break;
393402
}
403+
case 'cond': {
404+
if (probes.length === 0) {
405+
throw new ERR_DEBUGGER_STARTUP_ERROR('Unexpected --cond before --probe');
406+
}
407+
// A blank condition does not act as a real predicate in V8 (an empty
408+
// string always breaks), so reject it rather than silently mislead.
409+
if (token.value === undefined || StringPrototypeTrim(token.value) === '') {
410+
throw new ERR_DEBUGGER_STARTUP_ERROR(`Missing value for ${token.rawName}`);
411+
}
412+
const probe = probes[probes.length - 1];
413+
if (probe.condition !== undefined) {
414+
throw new ERR_DEBUGGER_STARTUP_ERROR('A --probe can have at most one --cond');
415+
}
416+
probe.condition = token.value;
417+
break;
418+
}
394419
default:
395420
if (probes.length > 0) {
396421
throw new ERR_DEBUGGER_STARTUP_ERROR(
@@ -410,6 +435,21 @@ function parseProbeTokens(tokens, args) {
410435
'Probe mode requires at least one --probe <loc> --expr <expr> group');
411436
}
412437

438+
// V8 allows only one breakpoint per location, so probes sharing a location
439+
// cannot carry different conditions.
440+
const conditionByLocation = new SafeMap();
441+
for (const { target, condition } of probes) {
442+
const key = locationKey(target);
443+
if (conditionByLocation.has(key)) {
444+
if (conditionByLocation.get(key) !== condition) {
445+
throw new ERR_DEBUGGER_STARTUP_ERROR(
446+
`Probes at ${formatTargetText(target)} must use the same --cond (or none)`);
447+
}
448+
} else {
449+
conditionByLocation.set(key, condition);
450+
}
451+
}
452+
413453
const childArgv = ArrayPrototypeSlice(args, childStartIndex);
414454
if (childArgv.length === 0) {
415455
throw new ERR_DEBUGGER_STARTUP_ERROR('Probe mode requires a child script');
@@ -479,8 +519,8 @@ class ProbeInspectorSession {
479519
this.resolveCompletion = resolve;
480520
/** @type {Probe[]} */
481521
this.probes = ArrayPrototypeMap(options.probes,
482-
({ expr, target, maxHit }) =>
483-
({ expr, target, maxHit: maxHit ?? Infinity, hits: 0 }));
522+
({ expr, target, maxHit, condition }) =>
523+
({ expr, target, condition, maxHit: maxHit ?? Infinity, hits: 0 }));
484524
this.onChildOutput = FunctionPrototypeBind(this.onChildOutput, this);
485525
this.onChildExit = FunctionPrototypeBind(this.onChildExit, this);
486526
this.onClientClose = FunctionPrototypeBind(this.onClientClose, this);
@@ -887,17 +927,19 @@ class ProbeInspectorSession {
887927
const uniqueTargets = new SafeMap();
888928

889929
for (let probeIndex = 0; probeIndex < this.probes.length; probeIndex++) {
890-
const { target } = this.probes[probeIndex];
891-
const key = `${target.suffix}\n${target.line}\n${target.column ?? ''}`;
930+
const { target, condition } = this.probes[probeIndex];
931+
// Probes at the same location share one V8 breakpoint. parseProbeTokens has
932+
// already ensured they carry the same condition.
933+
const key = locationKey(target);
892934
let entry = uniqueTargets.get(key);
893935
if (entry === undefined) {
894-
entry = { target, probeIndices: [] };
936+
entry = { target, condition, probeIndices: [] };
895937
uniqueTargets.set(key, entry);
896938
}
897939
ArrayPrototypePush(entry.probeIndices, probeIndex);
898940
}
899941

900-
for (const { target, probeIndices } of uniqueTargets.values()) {
942+
for (const { target, condition, probeIndices } of uniqueTargets.values()) {
901943
// On Windows, normalize backslashes to forward slashes so the regex matches
902944
// V8 script URLs which always use forward slashes.
903945
const normalizedFile = process.platform === 'win32' ?
@@ -918,6 +960,9 @@ class ProbeInspectorSession {
918960
// the inspector bind to the first executable column.
919961
params.columnNumber = target.column - 1;
920962
}
963+
if (condition !== undefined) {
964+
params.condition = condition;
965+
}
921966

922967
const result = await this.callCdp('Debugger.setBreakpointByUrl', params);
923968
debug('breakpoint set: id=%s urlRegex=%s locations=%j',
@@ -943,9 +988,10 @@ class ProbeInspectorSession {
943988
code: exitCode,
944989
report: {
945990
v: kProbeVersion,
946-
probes: ArrayPrototypeMap(this.probes, ({ expr, target, maxHit }) => {
947-
// Omit an unlimited maxHit, as Infinity would serialize to null in JSON.
991+
probes: ArrayPrototypeMap(this.probes, ({ expr, target, maxHit, condition }) => {
948992
const probe = { expr, target };
993+
if (condition !== undefined) { probe.condition = condition; }
994+
// Omit an unlimited maxHit, as Infinity would serialize to null in JSON.
949995
if (maxHit !== Infinity) { probe.maxHit = maxHit; }
950996
return probe;
951997
}),
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// This tests that probe mode rejects malformed --cond usage.
2+
'use strict';
3+
4+
const common = require('../common');
5+
common.skipIfInspectorDisabled();
6+
7+
const fixtures = require('../common/fixtures');
8+
const { assertProbeCliError } = require('../common/debugger-probe');
9+
10+
const cwd = fixtures.path('debugger');
11+
12+
assertProbeCliError(
13+
['--cond', 'x', '--probe', 'probe.js:12', '--expr', 'finalValue', 'probe.js'],
14+
/Unexpected --cond before --probe/, { cwd });
15+
16+
assertProbeCliError(
17+
['--probe', 'probe.js:12', '--cond', 'x', '--expr', 'finalValue', 'probe.js'],
18+
/Each --probe must be followed immediately by --expr/, { cwd });
19+
20+
assertProbeCliError(
21+
['--probe', 'probe.js:12', '--expr', 'finalValue', '--cond', 'x', '--cond', 'y', 'probe.js'],
22+
/A --probe can have at most one --cond/, { cwd });
23+
24+
assertProbeCliError(
25+
['--probe', 'probe.js:12', '--expr', 'finalValue', '--cond', ' ', 'probe.js'],
26+
/Missing value for --cond/, { cwd });
27+
28+
assertProbeCliError(
29+
['--probe', 'probe.js:12', '--expr', 'finalValue', '--cond'],
30+
/Missing value for --cond/, { cwd });
31+
32+
assertProbeCliError(
33+
['--probe', 'probe.js:12', '--expr', 'a', '--cond', 'x',
34+
'--probe', 'probe.js:12', '--expr', 'b', '--cond', 'y', 'probe.js'],
35+
/Probes at probe\.js:12 must use the same --cond \(or none\)/, { cwd });
36+
37+
assertProbeCliError(
38+
['--probe', 'probe.js:12', '--expr', 'a', '--cond', 'x',
39+
'--probe', 'probe.js:12', '--expr', 'b', 'probe.js'],
40+
/Probes at probe\.js:12 must use the same --cond \(or none\)/, { cwd });
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// This tests that --cond and --max-hit work together.
2+
'use strict';
3+
4+
const common = require('../common');
5+
common.skipIfInspectorDisabled();
6+
7+
const fixtures = require('../common/fixtures');
8+
const { spawnSyncAndAssert } = require('../common/child_process');
9+
const { assertProbeJson } = require('../common/debugger-probe');
10+
11+
const cwd = fixtures.path('debugger');
12+
const probeUrl = fixtures.fileURL('debugger', 'probe-max-hit.js').href;
13+
14+
// --max-hit written before --cond. The condition still filters index !== 1, so
15+
// the only recorded hit carries value 1 rather than the loop's first iteration.
16+
spawnSyncAndAssert(process.execPath, [
17+
'inspect',
18+
'--json',
19+
'--probe', 'probe-max-hit.js:5',
20+
'--expr', 'index',
21+
'--max-hit', '5',
22+
'--cond', 'index === 1',
23+
'probe-max-hit.js',
24+
], { cwd }, {
25+
stdout(output) {
26+
assertProbeJson(output, {
27+
v: 2,
28+
probes: [{
29+
expr: 'index',
30+
condition: 'index === 1',
31+
maxHit: 5,
32+
target: { suffix: 'probe-max-hit.js', line: 5 },
33+
}],
34+
results: [
35+
{
36+
probe: 0,
37+
event: 'hit',
38+
hit: 1,
39+
location: { url: probeUrl, line: 5, column: 3 },
40+
result: { type: 'number', value: 1, description: '1' },
41+
},
42+
{ event: 'completed' },
43+
],
44+
});
45+
},
46+
trim: true,
47+
});

0 commit comments

Comments
 (0)