inspector: add --cond to node inspect probe mode

doc/api/debugger.md

@@ -237,6 +237,10 @@ added:
   - v26.1.0
   - v24.16.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/64328
+    description: Add per-probe `--cond <expr>` option to only record a hit when the
+        condition is truthy at the probe location.
   - version: v26.4.0
     pr-url: https://github.com/nodejs/node/pull/63704
     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
 clean up afterwards. It also supports structured JSON output for tool use.
 
 ```console
-$ node inspect --probe <file>:<line>[:<col>] --expr <expr> [--max-hit <n>]
-              [--probe <file>:<line>[:<col>] --expr <expr> [--max-hit <n>] ...]
+$ node inspect --probe <file>:<line>[:<col>] --expr <expr> [--cond <expr>] [--max-hit <n>]
+              [--probe <file>:<line>[:<col>] --expr <expr> [--cond <expr>] [--max-hit <n>] ...]
               [--json] [--preview] [--timeout=<ms>] [--port=<port>]
               [--] [<node-option> ...] <script> [<script-args> ...]
 ```
@@ -283,6 +287,9 @@ $ node inspect --probe <file>:<line>[:<col>] --expr <expr> [--max-hit <n>]
 * `--expr <expr>`: JavaScript expression to evaluate whenever execution reaches
   the location specified by the preceding `--probe`.
   Must immediately follow the `--probe` it belongs to.
+* `--cond <expr>`: An optional condition for the probe location. The probe only
+  records a hit when `<expr>` is truthy at the location. A condition that throws
+  is treated as false.
 * `--max-hit <n>`: An optional per-probe limit on the number of times the probe
   can be hit. When not specified, there's no hit limit. When any probe reaches
   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>]
   will listen. Defaults to `0`, which requests a random port.
 * `--` is optional unless the child needs its own Node.js flags.
 
-Additional rules about the `--probe` and `--expr` arguments:
+Additional rules about the composition of the options:
 
 * `--probe <file>:<line>[:<col>]` and `--expr <expr>` are strict pairs. Each
   `--probe` must be followed immediately by exactly one `--expr`.
-* `--max-hit <n>` is an optional per-probe option that applies to the most recent
-  `--probe`/`--expr` pair. It may not appear before the first `--probe` or
-  between a `--probe` and its matching `--expr`, and may be given at most once
-  per probe.
+* `--cond <expr>` and `--max-hit <n>` are optional modifiers written _after_ the
+  `--probe`/`--expr` pair they apply to, each at most once per pair. They may not
+  appear before the first `--probe` or between a `--probe` and its matching
+  `--expr`.
+* `--max-hit` scopes to the `--probe`/`--expr` pair it follows, so pairs
+  sharing a location may set different limits. `--cond` scopes to the whole
+  location, probes sharing a location must share one condition (or none).
 * `--timeout`, `--json`, `--preview`, and `--port` are global probe options
   for the whole probe session. They may appear before or between probe pairs,
   but not between a `--probe` and its matching `--expr`.
@@ -321,6 +331,8 @@ $ node inspect --probe app.js:10 --expr "user"
                --json --preview -- --no-warnings app.js --arg-for-app=foo
 ```
 
+<!-- TODO(joyeecheung): add more examples for different options -->
+
 ### Probe output format
 
 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
         "suffix": "cli.js",
         "line": 5
       }
+      // `condition` is present only when the probe was given a --cond expression.
       // `maxHit` is present only when the probe was given a --max-hit limit.
     }
   ],
@@ -481,6 +494,10 @@ When multiple `--probe`/`--expr` pairs share the same `--probe`, the
 expressions will be evaluated on the same pause in the order they appear
 on the command line.
 
+For each location, there can only be at most one `--cond` (or none).
+Multiple `--probe`/`--expr` pairs with conflicting conditions
+at the same location will be rejected at launch time.
+
 ```js
 // app.js
 const x = { x: 42 };       // line 2

lib/internal/debugger/inspect.js

@@ -268,6 +268,7 @@ function parseInteractiveArgs(args) {
 
 const kInspectArgOptions = {
   '__proto__': null,
+  'cond': { type: 'string' },
   'expr': { type: 'string' },
   'help': { type: 'boolean', short: 'h' },
   'json': { type: 'boolean' },

lib/internal/debugger/inspect_helpers.js

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

lib/internal/debugger/inspect_probe.js

@@ -21,6 +21,7 @@ const {
   StringPrototypeIncludes,
   StringPrototypeSlice,
   StringPrototypeStartsWith,
+  StringPrototypeTrim,
   Symbol,
 } = primordials;
 
@@ -87,6 +88,8 @@ const kInspectPortRegex = /^--inspect-port=(\d+)$/;
  * @typedef {object} Probe
  * @property {string} expr Expression to evaluate on hit.
  * @property {ProbeTarget} target User's original --probe request shape.
+ * @property {string} [condition] Condition from --cond. V8 only breaks when it is truthy.
+ *   Scoped to the location, so probes sharing a location all carry the same value.
  * @property {number} maxHit Per-probe hit limit from --max-hit. Infinity when unlimited.
  * @property {number} hits Count of hits observed.
  */
@@ -130,6 +133,12 @@ function formatTargetText(target) {
   return column === undefined ? `${suffix}:${line}` : `${suffix}:${line}:${column}`;
 }
 
+// Identity of a probe location. Probes sharing a key share one V8 breakpoint,
+// so this must stay in sync between condition validation and breakpoint setup.
+function locationKey(target) {
+  return `${target.suffix}\n${target.line}\n${target.column ?? ''}`;
+}
+
 function formatPendingProbeLocations(probes, pending) {
   const seen = new SafeSet();
   for (const probeIndex of pending) {
@@ -391,6 +400,22 @@ function parseProbeTokens(tokens, args) {
         probe.maxHit = parseUnsignedInteger(token.value, 'max-hit');
         break;
       }
+      case 'cond': {
+        if (probes.length === 0) {
+          throw new ERR_DEBUGGER_STARTUP_ERROR('Unexpected --cond before --probe');
+        }
+        // A blank condition does not act as a real predicate in V8 (an empty
+        // string always breaks), so reject it rather than silently mislead.
+        if (token.value === undefined || StringPrototypeTrim(token.value) === '') {
+          throw new ERR_DEBUGGER_STARTUP_ERROR(`Missing value for ${token.rawName}`);
+        }
+        const probe = probes[probes.length - 1];
+        if (probe.condition !== undefined) {
+          throw new ERR_DEBUGGER_STARTUP_ERROR('A --probe can have at most one --cond');
+        }
+        probe.condition = token.value;
+        break;
+      }
       default:
         if (probes.length > 0) {
           throw new ERR_DEBUGGER_STARTUP_ERROR(
@@ -410,6 +435,21 @@ function parseProbeTokens(tokens, args) {
       'Probe mode requires at least one --probe <loc> --expr <expr> group');
   }
 
+  // V8 allows only one breakpoint per location, so probes sharing a location
+  // cannot carry different conditions.
+  const conditionByLocation = new SafeMap();
+  for (const { target, condition } of probes) {
+    const key = locationKey(target);
+    if (conditionByLocation.has(key)) {
+      if (conditionByLocation.get(key) !== condition) {
+        throw new ERR_DEBUGGER_STARTUP_ERROR(
+          `Probes at ${formatTargetText(target)} must use the same --cond (or none)`);
+      }
+    } else {
+      conditionByLocation.set(key, condition);
+    }
+  }
+
   const childArgv = ArrayPrototypeSlice(args, childStartIndex);
   if (childArgv.length === 0) {
     throw new ERR_DEBUGGER_STARTUP_ERROR('Probe mode requires a child script');
@@ -479,8 +519,8 @@ class ProbeInspectorSession {
     this.resolveCompletion = resolve;
     /** @type {Probe[]} */
     this.probes = ArrayPrototypeMap(options.probes,
-                                    ({ expr, target, maxHit }) =>
-                                      ({ expr, target, maxHit: maxHit ?? Infinity, hits: 0 }));
+                                    ({ expr, target, maxHit, condition }) =>
+                                      ({ expr, target, condition, maxHit: maxHit ?? Infinity, hits: 0 }));
     this.onChildOutput = FunctionPrototypeBind(this.onChildOutput, this);
     this.onChildExit = FunctionPrototypeBind(this.onChildExit, this);
     this.onClientClose = FunctionPrototypeBind(this.onClientClose, this);
@@ -887,17 +927,19 @@ class ProbeInspectorSession {
     const uniqueTargets = new SafeMap();
 
     for (let probeIndex = 0; probeIndex < this.probes.length; probeIndex++) {
-      const { target } = this.probes[probeIndex];
-      const key = `${target.suffix}\n${target.line}\n${target.column ?? ''}`;
+      const { target, condition } = this.probes[probeIndex];
+      // Probes at the same location share one V8 breakpoint. parseProbeTokens has
+      // already ensured they carry the same condition.
+      const key = locationKey(target);
       let entry = uniqueTargets.get(key);
       if (entry === undefined) {
-        entry = { target, probeIndices: [] };
+        entry = { target, condition, probeIndices: [] };
         uniqueTargets.set(key, entry);
       }
       ArrayPrototypePush(entry.probeIndices, probeIndex);
     }
 
-    for (const { target, probeIndices } of uniqueTargets.values()) {
+    for (const { target, condition, probeIndices } of uniqueTargets.values()) {
       // On Windows, normalize backslashes to forward slashes so the regex matches
       // V8 script URLs which always use forward slashes.
       const normalizedFile = process.platform === 'win32' ?
@@ -918,6 +960,9 @@ class ProbeInspectorSession {
         // the inspector bind to the first executable column.
         params.columnNumber = target.column - 1;
       }
+      if (condition !== undefined) {
+        params.condition = condition;
+      }
 
       const result = await this.callCdp('Debugger.setBreakpointByUrl', params);
       debug('breakpoint set: id=%s urlRegex=%s locations=%j',
@@ -943,9 +988,10 @@ class ProbeInspectorSession {
       code: exitCode,
       report: {
         v: kProbeVersion,
-        probes: ArrayPrototypeMap(this.probes, ({ expr, target, maxHit }) => {
-          // Omit an unlimited maxHit, as Infinity would serialize to null in JSON.
+        probes: ArrayPrototypeMap(this.probes, ({ expr, target, maxHit, condition }) => {
           const probe = { expr, target };
+          if (condition !== undefined) { probe.condition = condition; }
+          // Omit an unlimited maxHit, as Infinity would serialize to null in JSON.
           if (maxHit !== Infinity) { probe.maxHit = maxHit; }
           return probe;
         }),

test/parallel/test-debugger-probe-cond-invalid.js

@@ -0,0 +1,40 @@
+// This tests that probe mode rejects malformed --cond usage.
+'use strict';
+
+const common = require('../common');
+common.skipIfInspectorDisabled();
+
+const fixtures = require('../common/fixtures');
+const { assertProbeCliError } = require('../common/debugger-probe');
+
+const cwd = fixtures.path('debugger');
+
+assertProbeCliError(
+  ['--cond', 'x', '--probe', 'probe.js:12', '--expr', 'finalValue', 'probe.js'],
+  /Unexpected --cond before --probe/, { cwd });
+
+assertProbeCliError(
+  ['--probe', 'probe.js:12', '--cond', 'x', '--expr', 'finalValue', 'probe.js'],
+  /Each --probe must be followed immediately by --expr/, { cwd });
+
+assertProbeCliError(
+  ['--probe', 'probe.js:12', '--expr', 'finalValue', '--cond', 'x', '--cond', 'y', 'probe.js'],
+  /A --probe can have at most one --cond/, { cwd });
+
+assertProbeCliError(
+  ['--probe', 'probe.js:12', '--expr', 'finalValue', '--cond', '   ', 'probe.js'],
+  /Missing value for --cond/, { cwd });
+
+assertProbeCliError(
+  ['--probe', 'probe.js:12', '--expr', 'finalValue', '--cond'],
+  /Missing value for --cond/, { cwd });
+
+assertProbeCliError(
+  ['--probe', 'probe.js:12', '--expr', 'a', '--cond', 'x',
+   '--probe', 'probe.js:12', '--expr', 'b', '--cond', 'y', 'probe.js'],
+  /Probes at probe\.js:12 must use the same --cond \(or none\)/, { cwd });
+
+assertProbeCliError(
+  ['--probe', 'probe.js:12', '--expr', 'a', '--cond', 'x',
+   '--probe', 'probe.js:12', '--expr', 'b', 'probe.js'],
+  /Probes at probe\.js:12 must use the same --cond \(or none\)/, { cwd });

test/parallel/test-debugger-probe-cond-max-hit.js

@@ -0,0 +1,47 @@
+// This tests that --cond and --max-hit work together.
+'use strict';
+
+const common = require('../common');
+common.skipIfInspectorDisabled();
+
+const fixtures = require('../common/fixtures');
+const { spawnSyncAndAssert } = require('../common/child_process');
+const { assertProbeJson } = require('../common/debugger-probe');
+
+const cwd = fixtures.path('debugger');
+const probeUrl = fixtures.fileURL('debugger', 'probe-max-hit.js').href;
+
+// --max-hit written before --cond. The condition still filters index !== 1, so
+// the only recorded hit carries value 1 rather than the loop's first iteration.
+spawnSyncAndAssert(process.execPath, [
+  'inspect',
+  '--json',
+  '--probe', 'probe-max-hit.js:5',
+  '--expr', 'index',
+  '--max-hit', '5',
+  '--cond', 'index === 1',
+  'probe-max-hit.js',
+], { cwd }, {
+  stdout(output) {
+    assertProbeJson(output, {
+      v: 2,
+      probes: [{
+        expr: 'index',
+        condition: 'index === 1',
+        maxHit: 5,
+        target: { suffix: 'probe-max-hit.js', line: 5 },
+      }],
+      results: [
+        {
+          probe: 0,
+          event: 'hit',
+          hit: 1,
+          location: { url: probeUrl, line: 5, column: 3 },
+          result: { type: 'number', value: 1, description: '1' },
+        },
+        { event: 'completed' },
+      ],
+    });
+  },
+  trim: true,
+});