diff --git a/.changeset/965-json-errors-unexpected-throw.md b/.changeset/965-json-errors-unexpected-throw.md new file mode 100644 index 000000000..35db58e9a --- /dev/null +++ b/.changeset/965-json-errors-unexpected-throw.md @@ -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) diff --git a/src/cli-exit.cts b/src/cli-exit.cts index 7e4a0afda..ff7f93aa3 100644 --- a/src/cli-exit.cts +++ b/src/cli-exit.cts @@ -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): void { Promise.resolve() @@ -32,8 +38,18 @@ function runMain(main: () => number | void | Promise): 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; }); } diff --git a/tests/cli-exit.test.cjs b/tests/cli-exit.test.cjs index 75a52c6a5..03a1de939 100644 --- a/tests/cli-exit.test.cjs +++ b/tests/cli-exit.test.cjs @@ -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)}` + ); + }); + }); +});