* fix(#965): emit structured json error for unexpected handler throws under --json-errors When GSD_JSON_ERRORS=1 / --json-errors is active, an unexpected (non-ExitError) throw in a handler now emits { ok: false, reason: "sdk_fail_fast", message } to stderr instead of a raw stack trace. The plain-text behaviour (no json-error mode) is unchanged. Closes #965 * chore(#965): backfill changeset pr number (987) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This commit is contained in:
5
.changeset/965-json-errors-unexpected-throw.md
Normal file
5
.changeset/965-json-errors-unexpected-throw.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 987
|
||||
---
|
||||
**`--json-errors` now emits a structured error even when a handler throws unexpectedly** — an unexpected (non-`ExitError`) throw fell through to a raw stack trace on stderr, breaking SDK structured-error parsing. (#965)
|
||||
@@ -1,3 +1,8 @@
|
||||
import fs from 'node:fs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import ioModule = require('./io.cjs');
|
||||
const { getJsonErrorMode, ERROR_REASON } = ioModule;
|
||||
|
||||
/**
|
||||
* Error carrying a process exit code. CLI logic throws this instead of calling
|
||||
* process.exit() (banned by n/no-process-exit); runMain() translates it into
|
||||
@@ -20,7 +25,8 @@ class ExitError extends Error {
|
||||
* process.on('exit') cleanup still fires). main may be sync or async:
|
||||
* number return -> process.exitCode = it
|
||||
* thrown ExitError -> process.exitCode = err.code (+ stderr err.message if hasUserMessage && code!=0)
|
||||
* other throw -> stderr stack + process.exitCode = 1
|
||||
* other throw -> when json-error mode is active, emits structured { ok:false, reason, message }
|
||||
* to stderr; otherwise writes raw stack trace. exit code = 1 in either case.
|
||||
*/
|
||||
function runMain(main: () => number | void | Promise<number | void>): void {
|
||||
Promise.resolve()
|
||||
@@ -32,8 +38,18 @@ function runMain(main: () => number | void | Promise<number | void>): void {
|
||||
process.exitCode = err.code;
|
||||
return;
|
||||
}
|
||||
const e = err as Error;
|
||||
process.stderr.write(`${e && e.stack ? e.stack : String(err)}\n`);
|
||||
if (getJsonErrorMode()) {
|
||||
const e = err as Error;
|
||||
const payload = JSON.stringify({
|
||||
ok: false,
|
||||
reason: ERROR_REASON.SDK_FAIL_FAST,
|
||||
message: (e && e.message) ? e.message : String(err),
|
||||
}) + '\n';
|
||||
fs.writeSync(2, payload);
|
||||
} else {
|
||||
const e = err as Error;
|
||||
process.stderr.write(`${e && e.stack ? e.stack : String(err)}\n`);
|
||||
}
|
||||
process.exitCode = 1;
|
||||
});
|
||||
}
|
||||
|
||||
@@ -2,9 +2,16 @@
|
||||
|
||||
const { describe, test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const { spawnSync } = require('node:child_process');
|
||||
const path = require('node:path');
|
||||
|
||||
const { ExitError, runMain } = require('../scripts/lib/cli-exit.cjs');
|
||||
|
||||
// Paths to the compiled product seam (src/cli-exit.cts → gsd-core/bin/lib/cli-exit.cjs)
|
||||
// used for json-error mode regression tests which require io.cjs integration.
|
||||
const BUILT_CLI_EXIT_PATH = path.resolve(__dirname, '../gsd-core/bin/lib/cli-exit.cjs');
|
||||
const IO_PATH = path.resolve(__dirname, '../gsd-core/bin/lib/io.cjs');
|
||||
|
||||
/** Settle the runMain promise chain before asserting. */
|
||||
async function settle() {
|
||||
await new Promise((r) => setImmediate(r));
|
||||
@@ -159,3 +166,84 @@ describe('runMain', () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Regressions ─────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* bug #965 — runMain unexpected throw with --json-errors active emitted a raw
|
||||
* stack trace instead of a structured { ok:false, reason, message } envelope.
|
||||
* SDK consumers parsing structured errors would receive an unparseable string.
|
||||
*
|
||||
* Fix: src/cli-exit.cts non-ExitError catch branch now checks getJsonErrorMode()
|
||||
* and emits the same structured envelope as error() when active.
|
||||
*
|
||||
* Tests run against the compiled product seam (gsd-core/bin/lib/cli-exit.cjs)
|
||||
* via subprocess so that io.cjs module-level state is isolated per spawn.
|
||||
*/
|
||||
describe('regressions', () => {
|
||||
/** Spawn a one-shot script that sets json-error mode and calls runMain with a throwing handler. */
|
||||
function spawnJsonErrorRun({ jsonMode, errorType = 'TypeError', message = 'unexpected boom' } = {}) {
|
||||
const script = `
|
||||
const io = require(${JSON.stringify(IO_PATH)});
|
||||
const { runMain } = require(${JSON.stringify(BUILT_CLI_EXIT_PATH)});
|
||||
io.setJsonErrorMode(${jsonMode ? 'true' : 'false'});
|
||||
runMain(() => { throw new ${errorType}(${JSON.stringify(message)}); });
|
||||
setImmediate(() => {});
|
||||
`;
|
||||
return spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' });
|
||||
}
|
||||
|
||||
describe('bug-965: unexpected throw in json-error mode emits structured envelope', () => {
|
||||
test('stderr is a single parseable JSON object (not a raw stack trace)', () => {
|
||||
const result = spawnJsonErrorRun({ jsonMode: true });
|
||||
assert.strictEqual(result.status, 1,
|
||||
`expected exit code 1, got ${result.status}; stderr: ${result.stderr}`);
|
||||
const stderrTrimmed = result.stderr.trim();
|
||||
assert.ok(stderrTrimmed.length > 0, 'expected non-empty stderr');
|
||||
let parsed;
|
||||
try {
|
||||
parsed = JSON.parse(stderrTrimmed);
|
||||
} catch (e) {
|
||||
assert.fail(
|
||||
`stderr is NOT valid JSON (raw stack trace leaked through):\n${stderrTrimmed}\nparse error: ${e.message}`
|
||||
);
|
||||
}
|
||||
assert.strictEqual(parsed.ok, false, `expected ok:false, got: ${JSON.stringify(parsed)}`);
|
||||
assert.strictEqual(parsed.reason, 'sdk_fail_fast',
|
||||
`expected reason "sdk_fail_fast", got: ${parsed.reason}`);
|
||||
assert.ok(
|
||||
parsed.message && parsed.message.includes('unexpected boom'),
|
||||
`expected message to include "unexpected boom", got: ${JSON.stringify(parsed.message)}`
|
||||
);
|
||||
});
|
||||
|
||||
test('stderr JSON works for RangeError as well as TypeError', () => {
|
||||
const result = spawnJsonErrorRun({ jsonMode: true, errorType: 'RangeError', message: 'out of bounds' });
|
||||
assert.strictEqual(result.status, 1);
|
||||
const parsed = JSON.parse(result.stderr.trim());
|
||||
assert.strictEqual(parsed.ok, false);
|
||||
assert.strictEqual(parsed.reason, 'sdk_fail_fast');
|
||||
assert.ok(parsed.message.includes('out of bounds'));
|
||||
});
|
||||
|
||||
test('stdout is empty when unexpected throw emits structured error', () => {
|
||||
const result = spawnJsonErrorRun({ jsonMode: true });
|
||||
assert.strictEqual(result.stdout, '',
|
||||
`expected empty stdout, got: ${result.stdout}`);
|
||||
});
|
||||
|
||||
test('plain mode (json-error off) preserves raw stack trace on stderr', () => {
|
||||
const result = spawnJsonErrorRun({ jsonMode: false });
|
||||
assert.strictEqual(result.status, 1);
|
||||
const stderrTrimmed = result.stderr.trim();
|
||||
let parsed = null;
|
||||
try { parsed = JSON.parse(stderrTrimmed); } catch { /* expected — not JSON */ }
|
||||
assert.strictEqual(parsed, null,
|
||||
`expected raw stack (non-JSON) on stderr in plain mode, but got valid JSON: ${stderrTrimmed.slice(0, 200)}`);
|
||||
assert.ok(
|
||||
stderrTrimmed.includes('unexpected boom'),
|
||||
`expected "unexpected boom" in stderr, got: ${stderrTrimmed.slice(0, 200)}`
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user