Repository: sindresorhus/execa
Pull request #375 (2019-10-10 11:34:16)

Fix error properties
The `exitCode` and `exitCodeName` are currently wrong. 

[We use `error.code` as the base](https://github.com/sindresorhus/execa/blob/07d2b76a32566d147c5ee44fd617e68a0e4f231e/lib/error.js#L5) for `exitCode` and `exitCodeName`. [We also use `util.getSystemErrorName()`](https://github.com/sindresorhus/execa/blob/07d2b76a32566d147c5ee44fd617e68a0e4f231e/lib/error.js#L10) on an exit code. Both are erroneous as they are mixing exit codes and `error.code`/`error.errno`, which are different things. `error.code` and `error.errno` are based on [OS-specific system calls](https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/errno-base.h). Those are sometimes used as exit codes, but an exit code does not always translate to an `error.errno`.

Assigning a name or description to exit codes is tricky. That's because exit codes meaning is both application-specific and OS-specific. For example, the exit code `2` means "Command line usage error" [if the command is `bash`](https://www.tldp.org/LDP/abs/html/exitcodes.html#EXITCODESREF). But it means "ENOENT - No such file or directory" if the error was due to [a Linux system call error](https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/errno-base.h).

This PR fixes this by:
  - removing `error.exitCodeName` (breaking change). 
  - not making `exitCode` use `error.code` anymore
  - keeping `error.code` when defined instead of deleting it
  - adding the error message `Command failed with ${error.code}` when `error.code` is defined
  - improving error-related tests
  - renaming the internal variable `code` to `exitCode` for clarity


Patch:
diff --git a/index.d.ts b/index.d.ts
index 2050cfa..f0c431b 100644
--- a/index.d.ts
+++ b/index.d.ts
@@ -228,11 +228,6 @@ declare namespace execa {
 		*/
 		exitCode: number;
 
-		/**
-		The textual exit code of the process that was run.
-		*/
-		exitCodeName: string;
-
 		/**
 		The output of the process on stdout.
 		*/
diff --git a/index.js b/index.js
index 0de3369..f60593f 100644
--- a/index.js
+++ b/index.js
@@ -104,15 +104,15 @@ const execa = (file, args, options) => {
 	spawned.cancel = spawnedCancel.bind(null, spawned, context);
 
 	const handlePromise = async () => {
-		const [{error, code, signal, timedOut}, stdoutResult, stderrResult, allResult] = await getSpawnedResult(spawned, parsed.options, processDone);
+		const [{error, exitCode, signal, timedOut}, stdoutResult, stderrResult, allResult] = await getSpawnedResult(spawned, parsed.options, processDone);
 		const stdout = handleOutput(parsed.options, stdoutResult);
 		const stderr = handleOutput(parsed.options, stderrResult);
 		const all = handleOutput(parsed.options, allResult);
 
-		if (error || code !== 0 || signal !== null) {
+		if (error || exitCode !== 0 || signal !== null) {
 			const returnedError = makeError({
 				error,
-				code,
+				exitCode,
 				signal,
 				stdout,
 				stderr,
@@ -134,7 +134,6 @@ const execa = (file, args, options) => {
 		return {
 			command,
 			exitCode: 0,
-			exitCodeName: 'SUCCESS',
 			stdout,
 			stderr,
 			all,
@@ -181,13 +180,16 @@ module.exports.sync = (file, args, options) => {
 		});
 	}
 
-	result.stdout = handleOutput(parsed.options, result.stdout, result.error);
-	result.stderr = handleOutput(parsed.options, result.stderr, result.error);
+	const stdout = handleOutput(parsed.options, result.stdout, result.error);
+	const stderr = handleOutput(parsed.options, result.stderr, result.error);
 
 	if (result.error || result.status !== 0 || result.signal !== null) {
 		const error = makeError({
-			...result,
-			code: result.status,
+			stdout,
+			stderr,
+			error: result.error,
+			signal: result.signal,
+			exitCode: result.status,
 			command,
 			parsed,
 			timedOut: result.error && result.error.code === 'ETIMEDOUT',
@@ -205,9 +207,8 @@ module.exports.sync = (file, args, options) => {
 	return {
 		command,
 		exitCode: 0,
-		exitCodeName: 'SUCCESS',
-		stdout: result.stdout,
-		stderr: result.stderr,
+		stdout,
+		stderr,
 		failed: false,
 		timedOut: false,
 		isCanceled: false,
diff --git a/lib/error.js b/lib/error.js
index f071c5f..b651823 100644
--- a/lib/error.js
+++ b/lib/error.js
@@ -1,20 +1,5 @@
 'use strict';
-const os = require('os');
-const util = require('util');
-
-const getCode = (error, code) => {
-	if (error && error.code) {
-		return [error.code, os.constants.errno[error.code]];
-	}
-
-	if (Number.isInteger(code)) {
-		return [util.getSystemErrorName(-code), code];
-	}
-
-	return [];
-};
-
-const getErrorPrefix = ({timedOut, timeout, signal, exitCodeName, exitCode, isCanceled}) => {
+const getErrorPrefix = ({timedOut, timeout, errorCode, signal, exitCode, isCanceled}) => {
 	if (timedOut) {
 		return `timed out after ${timeout} milliseconds`;
 	}
@@ -23,12 +8,16 @@ const getErrorPrefix = ({timedOut, timeout, signal, exitCodeName, exitCode, isCa
 		return 'was canceled';
 	}
 
-	if (signal) {
+	if (errorCode !== undefined) {
+		return `failed with ${errorCode}`;
+	}
+
+	if (signal !== undefined) {
 		return `was killed with ${signal}`;
 	}
 
 	if (exitCode !== undefined) {
-		return `failed with exit code ${exitCode} (${exitCodeName})`;
+		return `failed with exit code ${exitCode}`;
 	}
 
 	return 'failed';
@@ -40,16 +29,21 @@ const makeError = ({
 	all,
 	error,
 	signal,
-	code,
+	exitCode,
 	command,
 	timedOut,
 	isCanceled,
 	killed,
 	parsed: {options: {timeout}}
 }) => {
-	const [exitCodeName, exitCode] = getCode(error, code);
+	// `signal` and `exitCode` emitted on `spawned.on('exit')` event can be `null`.
+	// We normalize them to `undefined`
+	exitCode = exitCode === null ? undefined : exitCode;
+	signal = signal === null ? undefined : signal;
+
+	const errorCode = error && error.code;
 
-	const prefix = getErrorPrefix({timedOut, timeout, signal, exitCodeName, exitCode, isCanceled});
+	const prefix = getErrorPrefix({timedOut, timeout, errorCode, signal, exitCode, isCanceled});
 	const message = `Command ${prefix}: ${command}`;
 
 	if (error instanceof Error) {
@@ -60,9 +54,8 @@ const makeError = ({
 	}
 
 	error.command = command;
-	delete error.code;
 	error.exitCode = exitCode;
-	error.exitCodeName = exitCodeName;
+	error.signal = signal;
 	error.stdout = stdout;
 	error.stderr = stderr;
 
@@ -78,9 +71,6 @@ const makeError = ({
 	error.timedOut = Boolean(timedOut);
 	error.isCanceled = isCanceled;
 	error.killed = killed && !timedOut;
-	// `signal` emitted on `spawned.on('exit')` event can be `null`. We normalize
-	// it to `undefined`
-	error.signal = signal || undefined;
 
 	return error;
 };
diff --git a/lib/promise.js b/lib/promise.js
index 6bafb54..34773d5 100644
--- a/lib/promise.js
+++ b/lib/promise.js
@@ -29,8 +29,8 @@ const mergePromise = (spawned, promise) => {
 // Use promises instead of `child_process` events
 const getSpawnedPromise = spawned => {
 	return new Promise((resolve, reject) => {
-		spawned.on('exit', (code, signal) => {
-			resolve({code, signal});
+		spawned.on('exit', (exitCode, signal) => {
+			resolve({exitCode, signal});
 		});
 
 		spawned.on('error', error => {
diff --git a/lib/stream.js b/lib/stream.js
index 2a87068..102004c 100644
--- a/lib/stream.js
+++ b/lib/stream.js
@@ -74,7 +74,7 @@ const getSpawnedResult = async ({stdout, stderr, all}, {encoding, buffer, maxBuf
 		return await Promise.all([processDone, stdoutPromise, stderrPromise, allPromise]);
 	} catch (error) {
 		return Promise.all([
-			{error, code: error.code, signal: error.signal, timedOut: error.timedOut},
+			{error, signal: error.signal, timedOut: error.timedOut},
 			getBufferedData(stdout, stdoutPromise),
 			getBufferedData(stderr, stderrPromise),
 			getBufferedData(all, allPromise)
diff --git a/package.json b/package.json
index a030535..60eec3c 100644
--- a/package.json
+++ b/package.json
@@ -42,7 +42,7 @@
 		"get-stream": "^5.0.0",
 		"is-stream": "^2.0.0",
 		"merge-stream": "^2.0.0",
-		"npm-run-path": "^3.0.0",
+		"npm-run-path": "^4.0.0",
 		"onetime": "^5.1.0",
 		"p-finally": "^2.0.0",
 		"signal-exit": "^3.0.2",
diff --git a/readme.md b/readme.md
index 44b9ee2..3e494e7 100644
--- a/readme.md
+++ b/readme.md
@@ -53,19 +53,19 @@ const execa = require('execa');
 
 	// Catching an error
 	try {
-		await execa('wrong', ['command']);
+		await execa('unknown', ['command']);
 	} catch (error) {
 		console.log(error);
 		/*
 		{
-			message: 'Command failed with exit code 2 (ENOENT): wrong command spawn wrong ENOENT',
-			errno: -2,
-			syscall: 'spawn wrong',
-			path: 'wrong',
+			message: 'Command failed with ENOENT: unknown command spawn unknown ENOENT',
+			errno: 'ENOENT',
+			code: 'ENOENT',
+			syscall: 'spawn unknown',
+			path: 'unknown',
 			spawnargs: ['command'],
-			command: 'wrong command',
-			exitCode: 2,
-			exitCodeName: 'ENOENT',
+			originalMessage: 'spawn unknown ENOENT',
+			command: 'unknown command',
 			stdout: '',
 			stderr: '',
 			all: '',
@@ -92,21 +92,22 @@ const execa = require('execa');
 
 // Catching an error with a sync method
 try {
-	execa.sync('wrong', ['command']);
+	execa.sync('unknown', ['command']);
 } catch (error) {
 	console.log(error);
 	/*
 	{
-		message: 'Command failed with exit code 2 (ENOENT): wrong command spawnSync wrong ENOENT',
-		errno: -2,
-		syscall: 'spawnSync wrong',
-		path: 'wrong',
+		message: 'Command failed with ENOENT: unknown command spawnSync unknown ENOENT',
+		errno: 'ENOENT',
+		code: 'ENOENT',
+		syscall: 'spawnSync unknown',
+		path: 'unknown',
 		spawnargs: ['command'],
-		command: 'wrong command',
-		exitCode: 2,
-		exitCodeName: 'ENOENT',
+		originalMessage: 'spawnSync unknown ENOENT',
+		command: 'unknown command',
 		stdout: '',
 		stderr: '',
+		all: '',
 		failed: true,
 		timedOut: false,
 		isCanceled: false,
@@ -219,12 +220,6 @@ Type: `number`
 
 The numeric exit code of the process that was run.
 
-#### exitCodeName
-
-Type: `string`
-
-The textual exit code of the process that was run.
-
 #### stdout
 
 Type: `string | Buffer`


Test patch:
diff --git a/index.test-d.ts b/index.test-d.ts
index 74ff581..938067f 100644
--- a/index.test-d.ts
+++ b/index.test-d.ts
@@ -17,7 +17,6 @@ try {
 	const unicornsResult = await execaPromise;
 	expectType<string>(unicornsResult.command);
 	expectType<number>(unicornsResult.exitCode);
-	expectType<string>(unicornsResult.exitCodeName);
 	expectType<string>(unicornsResult.stdout);
 	expectType<string>(unicornsResult.stderr);
 	expectType<string | undefined>(unicornsResult.all);
@@ -31,7 +30,6 @@ try {
 
 	expectType<string>(execaError.message);
 	expectType<number>(execaError.exitCode);
-	expectType<string>(execaError.exitCodeName);
 	expectType<string>(execaError.stdout);
 	expectType<string>(execaError.stderr);
 	expectType<string | undefined>(execaError.all);
@@ -47,7 +45,6 @@ try {
 	const unicornsResult = execa.sync('unicorns');
 	expectType<string>(unicornsResult.command);
 	expectType<number>(unicornsResult.exitCode);
-	expectType<string>(unicornsResult.exitCodeName);
 	expectType<string>(unicornsResult.stdout);
 	expectType<string>(unicornsResult.stderr);
 	expectError(unicornsResult.all);
@@ -61,7 +58,6 @@ try {
 
 	expectType<string>(execaError.message);
 	expectType<number>(execaError.exitCode);
-	expectType<string>(execaError.exitCodeName);
 	expectType<string>(execaError.stdout);
 	expectType<string>(execaError.stderr);
 	expectError(execaError.all);
diff --git a/test/error.js b/test/error.js
index 4a8288b..79cad52 100644
--- a/test/error.js
+++ b/test/error.js
@@ -36,24 +36,36 @@ test('stdout/stderr/all on process errors, in sync mode', t => {
 	t.is(all, undefined);
 });
 
-test('allow unknown exit code', async t => {
-	const {exitCode, exitCodeName} = await t.throwsAsync(execa('exit', ['255']), {message: /exit code 255 \(Unknown system error -255\)/});
-	t.is(exitCode, 255);
-	t.is(exitCodeName, 'Unknown system error -255');
+test('exitCode is 0 on success', async t => {
+	const {exitCode} = await execa('noop', ['foo']);
+	t.is(exitCode, 0);
 });
 
-test('execa() does not return code and failed properties on success', async t => {
-	const {exitCode, exitCodeName, failed} = await execa('noop', ['foo']);
-	t.is(exitCode, 0);
-	t.is(exitCodeName, 'SUCCESS');
+const testExitCode = async (t, num) => {
+	const {exitCode} = await t.throwsAsync(execa('exit', [`${num}`]), {message: getExitRegExp(num)});
+	t.is(exitCode, num);
+};
+
+test('exitCode is 2', testExitCode, 2);
+test('exitCode is 3', testExitCode, 3);
+test('exitCode is 4', testExitCode, 4);
+
+test('error.message contains the command', async t => {
+	await t.throwsAsync(execa('exit', ['2', 'foo', 'bar'], {message: /exit 2 foo bar/}));
+});
+
+test('Original error.message is kept', async t => {
+	const {originalMessage} = await t.throwsAsync(execa('wrong command'));
+	t.is(originalMessage, 'spawn wrong command ENOENT');
+});
+
+test('failed is false on success', async t => {
+	const {failed} = await execa('noop', ['foo']);
 	t.false(failed);
 });
 
-test('execa() returns code and failed properties', async t => {
-	const {exitCode, exitCodeName, failed} = await t.throwsAsync(execa('exit', ['2']), {message: getExitRegExp('2')});
-	t.is(exitCode, 2);
-	const expectedName = process.platform === 'win32' ? 'Unknown system error -2' : 'ENOENT';
-	t.is(exitCodeName, expectedName);
+test('failed is true on failure', async t => {
+	const {failed} = await t.throwsAsync(execa('exit', ['2']));
 	t.true(failed);
 });
 
@@ -134,6 +146,15 @@ if (process.platform !== 'win32') {
 		const {signal} = await t.throwsAsync(execa('noop', {killSignal: 'SIGHUP', timeout: 1, message: TIMEOUT_REGEXP}));
 		t.is(signal, 'SIGHUP');
 	});
+
+	test('exitCode is undefined on signal termination', async t => {
+		const cp = execa('noop');
+
+		process.kill(cp.pid);
+
+		const {exitCode} = await t.throwsAsync(cp);
+		t.is(exitCode, undefined);
+	});
 }
 
 test('result.signal is undefined for successful execution', async t => {
@@ -146,25 +167,12 @@ test('result.signal is undefined if process failed, but was not killed', async t
 	t.is(signal, undefined);
 });
 
-const testExitCode = async (t, num) => {
-	const {exitCode} = await t.throwsAsync(execa('exit', [`${num}`]), {message: getExitRegExp(num)});
-	t.is(exitCode, num);
-};
-
-test('error.exitCode is 2', testExitCode, 2);
-test('error.exitCode is 3', testExitCode, 3);
-test('error.exitCode is 4', testExitCode, 4);
-
-const errorMessage = async (t, expected, ...args) => {
-	await t.throwsAsync(execa('exit', args), {message: expected});
-};
-
-errorMessage.title = (message, expected) => `error.message matches: ${expected}`;
-
-test(errorMessage, /Command failed with exit code 2.*: exit 2 foo bar/, 2, 'foo', 'bar');
-test(errorMessage, /Command failed with exit code 3.*: exit 3 baz quz/, 3, 'baz', 'quz');
+test('error.code is undefined on success', async t => {
+	const {code} = await execa('noop');
+	t.is(code, undefined);
+});
 
-test('Original error message is kept', async t => {
-	const {originalMessage} = await t.throwsAsync(execa('wrong command'));
-	t.is(originalMessage, 'spawn wrong command ENOENT');
+test('error.code is defined on failure if applicable', async t => {
+	const {code} = await t.throwsAsync(execa('invalid'));
+	t.is(code, 'ENOENT');
 });
diff --git a/test/test.js b/test/test.js
index 96a4a71..8105a22 100644
--- a/test/test.js
+++ b/test/test.js
@@ -169,7 +169,7 @@ if (process.platform !== 'win32') {
 			await execa(`fast-exit-${process.platform}`, [], {input: 'data'});
 			t.pass();
 		} catch (error) {
-			t.is(error.exitCode, 32);
+			t.is(error.code, 'EPIPE');
 		}
 	});
 }