Skip to content

Commit

Permalink
Improve error handling of streams
Browse files Browse the repository at this point in the history
  • Loading branch information
ehmicky committed Jun 11, 2019
1 parent 4be4400 commit 5cf6b4e
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 39 deletions.
2 changes: 2 additions & 0 deletions index.d.ts
Expand Up @@ -41,6 +41,8 @@ declare namespace execa {
/**
Buffer the output from the spawned process. When buffering is disabled you must consume the output of the `stdout` and `stderr` streams because the promise will not be resolved/rejected until they have completed.
If the spawned process fails, `error.stdout`, `error.stderr` and `error.all` will contain the buffered data.
@default true
*/
readonly buffer?: boolean;
Expand Down
57 changes: 27 additions & 30 deletions index.js
Expand Up @@ -105,6 +105,16 @@ function makeAllStream(spawned) {
return mixed;
}

function getBufferedData(stream, streamPromise) {
if (!stream) {
return;
}

stream.destroy();

return streamPromise.catch(error => error.bufferedData);
}

function getStream(process, stream, {encoding, buffer, maxBuffer}) {
if (!process[stream]) {
return;
Expand All @@ -128,11 +138,7 @@ function getStream(process, stream, {encoding, buffer, maxBuffer}) {
ret = _getStream.buffer(process[stream], {maxBuffer});
}

return ret.catch(error => {
error.stream = stream;
error.message = `${stream} ${error.message}`;
throw error;
});
return ret;
}

function makeError(result, options) {
Expand Down Expand Up @@ -162,6 +168,10 @@ function makeError(result, options) {
error.all = result.all;
}

if ('bufferedData' in error) {
delete error.bufferedData;
}

error.failed = true;
error.timedOut = timedOut;
error.isCanceled = isCanceled;
Expand Down Expand Up @@ -311,35 +321,23 @@ const execa = (file, args, options) => {
}
}), cleanup);

function destroy() {
if (spawned.stdout) {
spawned.stdout.destroy();
}

if (spawned.stderr) {
spawned.stderr.destroy();
}

if (spawned.all) {
spawned.all.destroy();
}
}

const handlePromise = () => {
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})
]);
const stdoutPromise = getStream(spawned, 'stdout', {encoding, buffer, maxBuffer});
const stderrPromise = getStream(spawned, 'stderr', {encoding, buffer, maxBuffer});
const allPromise = getStream(spawned, 'all', {encoding, buffer, maxBuffer: maxBuffer * 2});

const finalize = async () => {
let results;
try {
results = await processComplete;
results = await Promise.all([processDone, stdoutPromise, stderrPromise, allPromise]);
} catch (error) {
const {stream, code, signal} = error;
results = [{error, stream, code, signal}, '', '', ''];
const {code, signal} = error;
results = await Promise.all([
{error, code, signal},
getBufferedData(spawned.stdout, stdoutPromise),
getBufferedData(spawned.stderr, stderrPromise),
getBufferedData(spawned.all, allPromise)
]);
}

const [result, stdout, stderr, all] = results;
Expand Down Expand Up @@ -378,8 +376,7 @@ const execa = (file, args, options) => {
};
};

// TODO: Use native "finally" syntax when targeting Node.js 10
return pFinally(finalize(), destroy);
return finalize();
};

crossSpawn._enoent.hookChildProcess(spawned, parsed.parsed);
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Expand Up @@ -287,6 +287,8 @@ Default: `true`

Buffer the output from the spawned process. When buffering is disabled you must consume the output of the `stdout` and `stderr` streams because the promise will not be resolved/rejected until they have completed.

If the spawned process fails, [`error.stdout`](#stdout), [`error.stderr`](#stderr) and [`error.all`](#all) will contain the buffered data.

#### input

Type: `string | Buffer | stream.Readable`
Expand Down
30 changes: 21 additions & 9 deletions test.js
Expand Up @@ -65,21 +65,23 @@ test('stdout/stderr/all are undefined if ignored in sync mode', t => {
t.is(all, undefined);
});

const WRONG_COMMAND = 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, '');
t.is(all, '');
t.is(stderr, WRONG_COMMAND);
t.is(all, WRONG_COMMAND);
});

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, process.platform === 'win32' ?
'\'wrong\' is not recognized as an internal or external command,\r\noperable program or batch file.' :
'');
t.is(stderr, WRONG_COMMAND);
t.is(all, undefined);
});

Expand Down Expand Up @@ -253,10 +255,16 @@ test('input option can be a Buffer - sync', t => {
t.is(stdout, 'testing12');
});

test('stdin errors are handled', async t => {
const child = execa('noop');
child.stdin.emit('error', new Error('test'));
await t.throwsAsync(child, /test/);
});

test('child process errors are handled', async t => {
const child = execa('noop');
child.emit('error', new Error('test'));
await t.throwsAsync(child, /Command failed.*\ntest/);
await t.throwsAsync(child, /test/);
});

test('opts.stdout:ignore - stdout will not collect data', async t => {
Expand Down Expand Up @@ -301,13 +309,17 @@ test('child_process.spawnSync() errors are propagated with a correct shape', t =
});

test('maxBuffer affects stdout', async t => {
await t.throwsAsync(execa('max-buffer', ['stdout', '11'], {maxBuffer: 10}), /stdout maxBuffer exceeded/);
await t.notThrowsAsync(execa('max-buffer', ['stdout', '10'], {maxBuffer: 10}));
const {stdout, all} = await t.throwsAsync(execa('max-buffer', ['stdout', '11'], {maxBuffer: 10}), /max-buffer stdout/);
t.is(stdout, '.'.repeat(10));
t.is(all, '.'.repeat(10));
});

test('maxBuffer affects stderr', async t => {
await t.throwsAsync(execa('max-buffer', ['stderr', '13'], {maxBuffer: 12}), /stderr maxBuffer exceeded/);
await t.notThrowsAsync(execa('max-buffer', ['stderr', '12'], {maxBuffer: 12}));
await t.notThrowsAsync(execa('max-buffer', ['stderr', '10'], {maxBuffer: 10}));
const {stderr, all} = await t.throwsAsync(execa('max-buffer', ['stderr', '11'], {maxBuffer: 10}), /max-buffer stderr/);
t.is(stderr, '.'.repeat(10));
t.is(all, '.'.repeat(10));
});

test('do not buffer stdout when `buffer` set to `false`', async t => {
Expand Down

0 comments on commit 5cf6b4e

Please sign in to comment.