Skip to content

Commit

Permalink
Child process errors should reject the promise right away (#270)
Browse files Browse the repository at this point in the history
  • Loading branch information
ehmicky authored and sindresorhus committed May 30, 2019
1 parent 2307d95 commit 3b8df50
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 31 deletions.
36 changes: 12 additions & 24 deletions index.js
Expand Up @@ -262,34 +262,23 @@ const execa = (file, args, options) => {
}, parsed.options.timeout);
}

const resolvable = (() => {
let extracted;
const promise = new Promise(resolve => {
extracted = resolve;
});
promise.resolve = extracted;
return promise;
})();

// TODO: Use native "finally" syntax when targeting Node.js 10
const processDone = pFinally(new Promise(resolve => {
const processDone = pFinally(new Promise((resolve, reject) => {
spawned.on('exit', (code, signal) => {
if (timedOut) {
resolvable.resolve([
{code, signal}, '', '', ''
]);
return reject(Object.assign(new Error('Timed out'), {code, signal}));
}

resolve({code, signal});
});

spawned.on('error', error => {
resolve({error});
reject(error);
});

if (spawned.stdin) {
spawned.stdin.on('error', error => {
resolve({error});
reject(error);
});
}
}), cleanup);
Expand All @@ -309,22 +298,21 @@ const execa = (file, args, options) => {
}

const handlePromise = () => {
let processComplete = Promise.all([
const processComplete = Promise.all([
processDone,
getStream(spawned, 'stdout', {encoding, buffer, maxBuffer}),
getStream(spawned, 'stderr', {encoding, buffer, maxBuffer}),
getStream(spawned, 'all', {encoding, buffer, maxBuffer: maxBuffer * 2})
]);

if (timeoutId) {
processComplete = Promise.race([
processComplete,
resolvable
]);
}

const finalize = async () => {
const results = await processComplete;
let results;
try {
results = await processComplete;
} catch (error) {
const {stream, code, signal} = error;
results = [{error, stream, code, signal}, '', '', ''];
}

const [result, stdout, stderr, all] = results;
result.stdout = handleOutput(parsed.options, stdout);
Expand Down
18 changes: 11 additions & 7 deletions test.js
Expand Up @@ -65,23 +65,21 @@ test('stdout/stderr/all are undefined if ignored in sync mode', t => {
t.is(all, undefined);
});

const WRONG_COMMAND_STDERR = process.platform === 'win32' ?
'\'wrong\' is not recognized as an internal or external command,\r\noperable program or batch file.' :
'';

test('stdout/stderr/all on process errors', async t => {
const {stdout, stderr, all} = await t.throwsAsync(execa('wrong command'));
t.is(stdout, '');
t.is(stderr, WRONG_COMMAND_STDERR);
t.is(all, WRONG_COMMAND_STDERR);
t.is(stderr, '');
t.is(all, '');
});

test('stdout/stderr/all on process errors, in sync mode', t => {
const {stdout, stderr, all} = t.throws(() => {
execa.sync('wrong command');
});
t.is(stdout, '');
t.is(stderr, WRONG_COMMAND_STDERR);
t.is(stderr, process.platform === 'win32' ?
'\'wrong\' is not recognized as an internal or external command,\r\noperable program or batch file.' :
'');
t.is(all, undefined);
});

Expand Down Expand Up @@ -217,6 +215,12 @@ test('helpful error trying to provide an input stream in sync mode', t => {
);
});

test('child process errors rejects promise right away', async t => {
const child = execa('forever');
child.emit('error', new Error('test'));
await t.throwsAsync(child, /test/);
});

test('execa() returns a promise with kill() and pid', t => {
const {kill, pid} = execa('noop', ['foo']);
t.is(typeof kill, 'function');
Expand Down

0 comments on commit 3b8df50

Please sign in to comment.