fix: guard null execSync return in benchmark exec helper#1772
Conversation
When exec() is called with { stdio: "ignore" }, execSync returns null
instead of a string. Calling .trim() on null crashes with TypeError,
breaking the entire benchmark workflow on the first test.
Also improve JSON validation in performance-monitor.yml to catch empty
files, since `jq empty` passes on empty input.
Discovered during end-to-end validation of the benchmark system.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
This PR hardens the performance benchmark tooling used by the Performance Monitor workflow by preventing a runtime crash in the benchmark script when execSync is invoked with stdio: "ignore", and by making the workflow’s benchmark JSON validation reject empty output files.
Changes:
- Guard
execSync(...)results in the benchmarkexec()helper to avoid calling.trim()onnullwhenstdiodiscards output. - Update the Performance Monitor workflow’s JSON validation to fail early when
benchmark-results.jsonis empty.
Show a summary per file
| File | Description |
|---|---|
| scripts/ci/benchmark-performance.ts | Prevents benchmark execution from crashing when execSync returns null under ignored stdio. |
| .github/workflows/performance-monitor.yml | Strengthens benchmark result validation by rejecting empty result files before running jq. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
.github/workflows/performance-monitor.yml:75
- The
jq emptyvalidation (even with the-ssize check) will still succeed ifbenchmark-results.jsoncontains only whitespace/newlines (non-zero size but no JSON values). To robustly fail on empty/whitespace input, consider switching tojq -e . benchmark-results.json >/dev/null(or a stricter filter that asserts the expected object shape) and relying on its exit code, optionally keeping a separate-fcheck for a clearer "missing file" error message.
if [ ! -s benchmark-results.json ]; then
echo "ERROR: benchmark-results.json is empty"
exit 1
fi
if ! jq empty benchmark-results.json 2>/dev/null; then
echo "ERROR: benchmark-results.json is not valid JSON"
cat benchmark-results.json
exit 1
fi
- Files reviewed: 2/2 changed files
- Comments generated: 0
|
Smoke Test Results — PASS
|
🔬 Smoke Test Results — PR #1772
PR: fix: guard null execSync return in benchmark exec helper Overall:
|
Smoke Test: GitHub Actions Services Connectivity
All checks passed. (
|
|
🔮 The oracle has read the runes of this smoke test.
|
Summary
nullreturn fromexecSyncwhen called with{ stdio: "ignore" }in the benchmark script'sexec()helper. Without this,.trim()crashes withTypeError: Cannot read properties of null (reading 'trim'), breaking the entire performance-monitor workflow on the first benchmark.performance-monitor.ymlto reject empty files (the previousjq emptycheck silently passed on empty input).Details
When
execSyncis called with{ stdio: "ignore" }, it returnsnullinstead of a string. Theexec()helper at line 58 ofbenchmark-performance.tscalls.trim()on the return value unconditionally, causing a crash. This affectsbenchmarkNetworkCreation()(line 311) and several other callers that pass{ stdio: "ignore" }.The fix uses nullish coalescing (
?? "") to default to an empty string before calling.trim().Additionally, the
Validate benchmark JSONstep in the workflow usedjq emptywhich succeeds on empty files. Added a[ ! -s ... ]check to catch this case.Discovered during end-to-end validation of the benchmark system.
Test plan
npm run buildsucceedsnpm testpasses (1341 tests)🤖 Generated with Claude Code