From 0b93469b604e27de71287a48ae37abb4c50a4c2c Mon Sep 17 00:00:00 2001 From: bluwy Date: Fri, 26 Jun 2026 12:43:52 +0800 Subject: [PATCH 1/3] feat: improve NonZeroExitError message --- src/main.ts | 15 ++++++++++++--- src/non-zero-exit-error.ts | 36 ++++++++++++++++++++++++++++-------- src/test/main_test.ts | 38 +++++++++++++++++++++++++++++--------- 3 files changed, 69 insertions(+), 20 deletions(-) diff --git a/src/main.ts b/src/main.ts index cffa0a7..cebde17 100644 --- a/src/main.ts +++ b/src/main.ts @@ -227,7 +227,7 @@ export class ExecProcess implements Result { this.exitCode !== 0 && this.exitCode !== undefined ) { - throw new NonZeroExitError(this); + throw new NonZeroExitError(this, undefined, this._command, this._args); } } @@ -268,7 +268,7 @@ export class ExecProcess implements Result { this.exitCode !== 0 && this.exitCode !== undefined ) { - throw new NonZeroExitError(this, result); + throw new NonZeroExitError(this, result, this._command, this._args); } return result; @@ -431,7 +431,16 @@ export function xSync( }; if (opts.throwOnError && exitCode !== 0 && exitCode !== undefined) { - throw new NonZeroExitError(result, result); + throw new NonZeroExitError( + result, + { + stdout: result.stdout, + stderr: result.stderr, + exitCode: result.exitCode + }, + command, + args + ); } return result; diff --git a/src/non-zero-exit-error.ts b/src/non-zero-exit-error.ts index 6c0e900..712e1b7 100644 --- a/src/non-zero-exit-error.ts +++ b/src/non-zero-exit-error.ts @@ -1,17 +1,37 @@ import type {Output, CommonOutputApi} from './main.js'; export class NonZeroExitError extends Error { - public get exitCode(): number | undefined { - if (this.result.exitCode !== null) { - return this.result.exitCode; - } - return undefined; - } + public readonly exitCode: number; public constructor( public readonly result: CommonOutputApi, - public readonly output?: Output + public readonly output?: Output, + command?: string, + args?: readonly string[] ) { - super(`Process exited with non-zero status (${result.exitCode})`); + let target = 'The process'; + if (command) { + const fullCommand = args?.length + ? `${command} ${args.map((a) => (a.includes(' ') ? JSON.stringify(a) : a)).join(' ')}` + : command; + target = `The command \`${fullCommand}\``; + } + + // This error is normally only created when the exit code is non-zero, so it + // must exist here. However, due to types compatibility, we accept it being + // nullable and default to 1 in case. + const exitCode = result.exitCode ?? 1; + + super(`${target} exited with a non-zero status (${exitCode})`); + this.exitCode = exitCode; + + // `result` is sometimes passed the entire child process object, which + // results in very large logs as it used to be typed `Result`. However, + // we don't manually subset it for now to keep compatibility. + Object.defineProperty(this, 'result', { + enumerable: false, + writable: false, + configurable: false + }); } } diff --git a/src/test/main_test.ts b/src/test/main_test.ts index 2dfb581..e51e804 100644 --- a/src/test/main_test.ts +++ b/src/test/main_test.ts @@ -61,9 +61,16 @@ describe.each(variants)('exec ($name)', ({x, isAsync}) => { describe('exec (async)', () => { test('non-zero exitCode throws when throwOnError=true', async () => { const proc = x('node', ['-e', 'process.exit(1);'], {throwOnError: true}); - await expect(async () => { + try { await proc; - }).rejects.toThrow(NonZeroExitError); + expect.fail('Expected to throw'); + } catch (err) { + expect.assert(err instanceof NonZeroExitError); + expect(err.exitCode).toBe(1); + expect(err.message).toBe( + 'The command `node -e process.exit(1);` exited with a non-zero status (1)' + ); + } expect(proc.exitCode).toBe(1); }); @@ -72,11 +79,18 @@ describe('exec (async)', () => { throwOnError: true }); const lines: string[] = []; - await expect(async () => { + try { for await (const line of proc) { lines.push(line); } - }).rejects.toThrow(NonZeroExitError); + expect.fail('Expected to throw'); + } catch (err) { + expect.assert(err instanceof NonZeroExitError); + expect(err.exitCode).toBe(1); + expect(err.message).toBe( + 'The command `node -e process.exit(1);` exited with a non-zero status (1)' + ); + } expect(lines).toEqual(['foo']); expect(proc.exitCode).toBe(1); }); @@ -136,9 +150,15 @@ describe('exec (async)', () => { describe('exec (sync)', () => { test('non-zero exitCode throws when throwOnError=true', () => { - expect(() => { + try { xSync('node', ['-e', 'process.exit(1);'], {throwOnError: true}); - }).toThrow(NonZeroExitError); + expect.fail('Expected to throw'); + } catch (err) { + expect(err instanceof NonZeroExitError).ok; + expect((err as NonZeroExitError).message).toBe( + '`node -e process.exit(1);` exited with a non-zero status (1)' + ); + } }); }); @@ -171,12 +191,12 @@ if (isWindows) { await proc; expect.fail('Expected to throw'); } catch (err) { - expect(err instanceof NonZeroExitError).ok; - expect((err as NonZeroExitError).output?.stderr).toBe( + expect.assert(err instanceof NonZeroExitError); + expect(err.output?.stderr).toBe( "'definitelyNonExistent' is not recognized as an internal" + ' or external command,\r\noperable program or batch file.\r\n' ); - expect((err as NonZeroExitError).output?.stdout).toBe(''); + expect(err.output?.stdout).toBe(''); } }); From c189819e5eae97b012b792925ed8dedc1b405799 Mon Sep 17 00:00:00 2001 From: bluwy Date: Fri, 26 Jun 2026 13:00:06 +0800 Subject: [PATCH 2/3] chore: fix test --- src/non-zero-exit-error.ts | 2 +- src/test/main_test.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/non-zero-exit-error.ts b/src/non-zero-exit-error.ts index 712e1b7..a4c3660 100644 --- a/src/non-zero-exit-error.ts +++ b/src/non-zero-exit-error.ts @@ -12,7 +12,7 @@ export class NonZeroExitError extends Error { let target = 'The process'; if (command) { const fullCommand = args?.length - ? `${command} ${args.map((a) => (a.includes(' ') ? JSON.stringify(a) : a)).join(' ')}` + ? `${command} ${args.map((a) => (/[ "'`()]/.test(a) ? JSON.stringify(a) : a)).join(' ')}` : command; target = `The command \`${fullCommand}\``; } diff --git a/src/test/main_test.ts b/src/test/main_test.ts index e51e804..19b1bd3 100644 --- a/src/test/main_test.ts +++ b/src/test/main_test.ts @@ -68,7 +68,7 @@ describe('exec (async)', () => { expect.assert(err instanceof NonZeroExitError); expect(err.exitCode).toBe(1); expect(err.message).toBe( - 'The command `node -e process.exit(1);` exited with a non-zero status (1)' + 'The command `node -e "process.exit(1);"` exited with a non-zero status (1)' ); } expect(proc.exitCode).toBe(1); @@ -88,7 +88,7 @@ describe('exec (async)', () => { expect.assert(err instanceof NonZeroExitError); expect(err.exitCode).toBe(1); expect(err.message).toBe( - 'The command `node -e process.exit(1);` exited with a non-zero status (1)' + 'The command `node -e "console.log(\'foo\'); process.exit(1);"` exited with a non-zero status (1)' ); } expect(lines).toEqual(['foo']); @@ -156,7 +156,7 @@ describe('exec (sync)', () => { } catch (err) { expect(err instanceof NonZeroExitError).ok; expect((err as NonZeroExitError).message).toBe( - '`node -e process.exit(1);` exited with a non-zero status (1)' + 'The command `node -e "process.exit(1);"` exited with a non-zero status (1)' ); } }); From cd3102fee7d264858f41a06eac003ce56ca2696b Mon Sep 17 00:00:00 2001 From: bluwy Date: Fri, 26 Jun 2026 22:55:09 +0800 Subject: [PATCH 3/3] chore: update comment --- src/non-zero-exit-error.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/non-zero-exit-error.ts b/src/non-zero-exit-error.ts index a4c3660..5690878 100644 --- a/src/non-zero-exit-error.ts +++ b/src/non-zero-exit-error.ts @@ -17,17 +17,17 @@ export class NonZeroExitError extends Error { target = `The command \`${fullCommand}\``; } - // This error is normally only created when the exit code is non-zero, so it - // must exist here. However, due to types compatibility, we accept it being - // nullable and default to 1 in case. + // This error is normally only created when the exit code is non-nullable + // and non-zero, so it must exist here. However, due to types compatibility, + // we default to 1 in case. const exitCode = result.exitCode ?? 1; super(`${target} exited with a non-zero status (${exitCode})`); this.exitCode = exitCode; - // `result` is sometimes passed the entire child process object, which - // results in very large logs as it used to be typed `Result`. However, - // we don't manually subset it for now to keep compatibility. + // `result` is usually passed the entire instance of the exec process + // depending on the exec API so that handlers can interact with it fully. + // As such, its log can be very large so we hide it by making it non-enumerable. Object.defineProperty(this, 'result', { enumerable: false, writable: false,