Repository: sindresorhus/execa (js)

Issue:
[Feature proposal] Improved error messages
The current code has [some logic](https://github.com/sindresorhus/execa/blob/master/index.js#L155) aimed at providing better error messages. It would be nice to add the following error messages:
  - `Command timed out after 5 seconds: ...` if `timedOut` is `true`.
  - `Command was killed with SIGTERM` if `signal` is not `null`.
  - `Command failed with exit code 2 (ENOENT)` otherwise.

The values (e.g. `SIGTERM`) in the messages above are for example purpose.

I can provide with a PR if this feature proposal is approved.

Pull request description:
Improve returned `error.message`
Fix #177 

This improves returned `error.message` by adding information about potential timeouts, signals and exit codes.

This also fixes few issues with `result.code`:
  - it was an integer instead of a string most of time because of the line `error.code = code < 0 ? errname(code) : code`. `code` comes from `childProcess.on('exit')` which is a positive integer most of the times. The fixed code avoids breaking backward compatibility by using the integer first if it's available.
  - `error.errno` was not used. There can be instances where `process.exit()` is not called but `error.errno` is available, which can be used as error `code` instead.
  - `os.constants.errno` is used to retrieve the code string when possible.

This also fixes `stderr` and `stdout` being concatenated with a newline between them when `options.stdio` was not a string.

Patch:
diff --git a/index.js b/index.js
index b4b5e34..db5dbd0 100644
--- a/index.js
+++ b/index.js
@@ -1,5 +1,6 @@
 'use strict';
 const path = require('path');
+const os = require('os');
 const childProcess = require('child_process');
 const crossSpawn = require('cross-spawn');
 const stripFinalNewline = require('strip-final-newline');
@@ -120,43 +121,67 @@ function getStream(process, stream, {encoding, buffer, maxBuffer}) {
 }
 
 function makeError(result, options) {
-	const {stdout, stderr} = result;
-
+	const {stdout, stderr, code, signal} = result;
 	let {error} = result;
-	const {code, signal} = result;
-
-	const {parsed, joinedCommand} = options;
-	const timedOut = options.timedOut || false;
+	const {joinedCommand, timedOut, parsed: {options: {timeout}}} = options;
 
-	if (!error) {
-		let output = '';
-
-		if (Array.isArray(parsed.options.stdio)) {
-			if (parsed.options.stdio[2] !== 'inherit') {
-				output += output.length > 0 ? stderr : `\n${stderr}`;
-			}
-
-			if (parsed.options.stdio[1] !== 'inherit') {
-				output += `\n${stdout}`;
-			}
-		} else if (parsed.options.stdio !== 'inherit') {
-			output = `\n${stderr}${stdout}`;
-		}
+	const [codeString, codeNumber] = getCode(result, code);
 
-		error = new Error(`Command failed: ${joinedCommand}${output}`);
-		error.code = code < 0 ? errname(code) : code;
+	if (!(error instanceof Error)) {
+		const message = [joinedCommand, stderr, stdout].filter(Boolean).join('\n');
+		error = new Error(message);
 	}
 
+	const prefix = getErrorPrefix({timedOut, timeout, signal, codeString, codeNumber});
+	error.message = `Command ${prefix}: ${error.message}`;
+
+	error.code = codeNumber || codeString;
 	error.stdout = stdout;
 	error.stderr = stderr;
 	error.failed = true;
 	error.signal = signal || null;
 	error.cmd = joinedCommand;
-	error.timedOut = timedOut;
+	error.timedOut = Boolean(timedOut);
 
 	return error;
 }
 
+function getCode({error = {}}, code) {
+	if (error.code) {
+		return [error.code, os.constants.errno[error.code]];
+	}
+
+	if (Number.isInteger(code)) {
+		return [errname(-Math.abs(code)), Math.abs(code)];
+	}
+
+	return [];
+}
+
+function getErrorPrefix({timedOut, timeout, signal, codeString, codeNumber}) {
+	if (timedOut) {
+		return `timed out after ${timeout} milliseconds`;
+	}
+
+	if (signal) {
+		return `was killed with ${signal}`;
+	}
+
+	if (codeString !== undefined && codeNumber !== undefined) {
+		return `failed with exit code ${codeNumber} (${codeString})`;
+	}
+
+	if (codeString !== undefined) {
+		return `failed with exit code ${codeString}`;
+	}
+
+	if (codeNumber !== undefined) {
+		return `failed with exit code ${codeNumber}`;
+	}
+
+	return 'failed';
+}
+
 function joinCommand(command, args) {
 	let joinedCommand = command;
 


Test patch:
diff --git a/test.js b/test.js
index 91fd135..7cffaee 100644
--- a/test.js
+++ b/test.js
@@ -14,6 +14,11 @@ process.env.FOO = 'foo';
 
 const NO_NEWLINES_REGEXP = /^[^\n]*$/;
 const STDERR_STDOUT_REGEXP = /stderr[^]*stdout/;
+const TIMEOUT_REGEXP = /timed out after/;
+
+function getExitRegExp(exitMessage) {
+	return new RegExp(`failed with exit code ${exitMessage}`);
+}
 
 test('execa()', async t => {
 	const {stdout} = await m('noop', ['foo']);
@@ -45,7 +50,7 @@ test('execa.stderr()', async t => {
 });
 
 test('stdout/stderr available on errors', async t => {
-	const err = await t.throwsAsync(m('exit', ['2']));
+	const err = await t.throwsAsync(m('exit', ['2']), {message: getExitRegExp('2')});
 	t.is(typeof err.stdout, 'string');
 	t.is(typeof err.stderr, 'string');
 });
@@ -99,7 +104,7 @@ test('execa.sync()', t => {
 });
 
 test('execa.sync() throws error if written to stderr', t => {
-	t.throws(() => m.sync('foo'), process.platform === 'win32' ? /'foo' is not recognized as an internal or external command/ : 'spawnSync foo ENOENT');
+	t.throws(() => m.sync('foo'), process.platform === 'win32' ? /'foo' is not recognized as an internal or external command/ : /spawnSync foo ENOENT/);
 });
 
 test('execa.sync() includes stdout and stderr in errors for improved debugging', t => {
@@ -256,9 +261,13 @@ test('skip throwing when using reject option', async t => {
 	t.is(typeof error.stderr, 'string');
 });
 
+test('allow unknown exit code', async t => {
+	await t.throwsAsync(m('exit', ['255']), {message: /exit code 255 \(Unknown system error -255\)/});
+});
+
 test('execa() returns code and failed properties', async t => {
 	const {code, failed} = await m('noop', ['foo']);
-	const error = await t.throwsAsync(m('exit', ['2']), {code: 2});
+	const error = await t.throwsAsync(m('exit', ['2']), {code: 2, message: getExitRegExp('2')});
 	t.is(code, 0);
 	t.false(failed);
 	t.true(error.failed);
@@ -284,7 +293,7 @@ test('error.killed is true if process was killed directly', async t => {
 		cp.kill();
 	}, 100);
 
-	const error = await t.throwsAsync(cp);
+	const error = await t.throwsAsync(cp, {message: /was killed with SIGTERM/});
 	t.true(error.killed);
 });
 
@@ -296,7 +305,9 @@ test('error.killed is false if process was killed indirectly', async t => {
 		process.kill(cp.pid, 'SIGINT');
 	}, 100);
 
-	const error = await t.throwsAsync(cp);
+	// `process.kill()` is emulated by Node.js on Windows
+	const message = process.platform === 'win32' ? /failed with exit code 1/ : /was killed with SIGINT/;
+	const error = await t.throwsAsync(cp, {message});
 	t.false(error.killed);
 });
 
@@ -322,7 +333,7 @@ if (process.platform !== 'win32') {
 			process.kill(cp.pid, 'SIGINT');
 		}, 100);
 
-		const error = await t.throwsAsync(cp);
+		const error = await t.throwsAsync(cp, {message: /was killed with SIGINT/});
 		t.is(error.signal, 'SIGINT');
 	});
 
@@ -333,12 +344,12 @@ if (process.platform !== 'win32') {
 			process.kill(cp.pid, 'SIGTERM');
 		}, 100);
 
-		const error = await t.throwsAsync(cp);
+		const error = await t.throwsAsync(cp, {message: /was killed with SIGTERM/});
 		t.is(error.signal, 'SIGTERM');
 	});
 
 	test('custom error.signal', async t => {
-		const error = await t.throwsAsync(m('delay', ['3000', '0'], {killSignal: 'SIGHUP', timeout: 1500}));
+		const error = await t.throwsAsync(m('delay', ['3000', '0'], {killSignal: 'SIGHUP', timeout: 1500, message: TIMEOUT_REGEXP}));
 		t.is(error.signal, 'SIGHUP');
 	});
 }
@@ -348,12 +359,12 @@ test('result.signal is null for successful execution', async t => {
 });
 
 test('result.signal is null if process failed, but was not killed', async t => {
-	const error = await t.throwsAsync(m('exit', [2]));
+	const error = await t.throwsAsync(m('exit', [2]), {message: getExitRegExp('2')});
 	t.is(error.signal, null);
 });
 
 async function code(t, num) {
-	await t.throwsAsync(m('exit', [`${num}`]), {code: num});
+	await t.throwsAsync(m('exit', [`${num}`]), {code: num, message: getExitRegExp(num)});
 }
 
 test('error.code is 2', code, 2);
@@ -361,14 +372,14 @@ test('error.code is 3', code, 3);
 test('error.code is 4', code, 4);
 
 test('timeout will kill the process early', async t => {
-	const error = await t.throwsAsync(m('delay', ['60000', '0'], {timeout: 1500}));
+	const error = await t.throwsAsync(m('delay', ['60000', '0'], {timeout: 1500, message: TIMEOUT_REGEXP}));
 
 	t.true(error.timedOut);
 	t.not(error.code, 22);
 });
 
 test('timeout will not kill the process early', async t => {
-	const error = await t.throwsAsync(m('delay', ['3000', '22'], {timeout: 30000}), {code: 22});
+	const error = await t.throwsAsync(m('delay', ['3000', '22'], {timeout: 30000}), {code: 22, message: getExitRegExp('22')});
 	t.false(error.timedOut);
 });
 
@@ -378,7 +389,7 @@ test('timedOut will be false if no timeout was set and zero exit code', async t
 });
 
 test('timedOut will be false if no timeout was set and non-zero exit code', async t => {
-	const error = await t.throwsAsync(m('delay', ['1000', '3']));
+	const error = await t.throwsAsync(m('delay', ['1000', '3']), {message: getExitRegExp('3')});
 	t.false(error.timedOut);
 });
 
@@ -388,8 +399,8 @@ async function errorMessage(t, expected, ...args) {
 
 errorMessage.title = (message, expected) => `error.message matches: ${expected}`;
 
-test(errorMessage, /Command failed: exit 2 foo bar/, 2, 'foo', 'bar');
-test(errorMessage, /Command failed: exit 3 baz quz/, 3, 'baz', 'quz');
+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');
 
 async function cmd(t, expected, ...args) {
 	const error = await t.throwsAsync(m('fail', args));
@@ -438,10 +449,8 @@ if (process.platform !== 'win32') {
 	test('cleanup false - SIGKILL', spawnAndKill, 'SIGKILL', false);
 }
 
-// See: https://github.com/sindresorhus/execa/issues/56
-const onlyWinFailing = test[process.platform === 'win32' ? 'failing' : 'serial'];
-onlyWinFailing('execa.shell() supports the `shell` option', async t => {
-	const {stdout} = await m.shell('noop foo', {
+test('execa.shell() supports the `shell` option', async t => {
+	const {stdout} = await m.shell('node fixtures/noop foo', {
 		shell: process.platform === 'win32' ? 'cmd.exe' : '/bin/bash'
 	});
 	t.is(stdout, 'foo');
@@ -455,7 +464,7 @@ if (process.platform !== 'win32') {
 			await m(`fast-exit-${process.platform}`, [], {input: 'data'});
 			t.pass();
 		} catch (error) {
-			t.is(error.code, 'EPIPE');
+			t.is(error.code, 32);
 		}
 	});
 }