Skip to content

Commit

Permalink
Fix error properties (#375)
Browse files Browse the repository at this point in the history
  • Loading branch information
ehmicky authored and sindresorhus committed Oct 14, 2019
1 parent 39c1968 commit c1b511b
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 105 deletions.
5 changes: 0 additions & 5 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
23 changes: 12 additions & 11 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -134,7 +134,6 @@ const execa = (file, args, options) => {
return {
command,
exitCode: 0,
exitCodeName: 'SUCCESS',
stdout,
stderr,
all,
Expand Down Expand Up @@ -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',
Expand All @@ -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,
Expand Down
4 changes: 0 additions & 4 deletions index.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand 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);
Expand 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);
Expand 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);
Expand Down
42 changes: 16 additions & 26 deletions lib/error.js
Original file line number Diff line number Diff line change
@@ -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`;
}
Expand All @@ -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';
Expand All @@ -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) {
Expand All @@ -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;

Expand All @@ -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;
};
Expand Down
4 changes: 2 additions & 2 deletions lib/promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
2 changes: 1 addition & 1 deletion lib/stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
39 changes: 17 additions & 22 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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: '',
Expand 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,
Expand Down Expand Up @@ -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`
Expand Down
74 changes: 41 additions & 33 deletions test/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down Expand Up @@ -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 => {
Expand All @@ -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');
});
2 changes: 1 addition & 1 deletion test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
});
}
Expand Down

0 comments on commit c1b511b

Please sign in to comment.