Skip to content

Commit

Permalink
fix(exec): consistent error message for maxBuffer (#919)
Browse files Browse the repository at this point in the history
* fix(exec): consistent error message for maxBuffer

This explicitly checks for maxBuffer errors in exec-child.js and
provides a consistent error message. This modifies the test to verify
this.

This also supports the change in Node v10, which emits this as a
RangeError instead of a regular Error.

Although the error message is now explicit, this is not part of our API,
since it's just tacked onto the end of exec's stderr.

Fixes #915

* Remove debugging log, ignore uncovered lines
  • Loading branch information
nfischer committed Dec 3, 2018
1 parent 3bb72ed commit e606706
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 5 deletions.
38 changes: 34 additions & 4 deletions src/exec-child.js
Expand Up @@ -16,19 +16,49 @@ var pipe = params.pipe;
var stdoutFile = params.stdoutFile;
var stderrFile = params.stderrFile;

function isMaxBufferError(err) {
var maxBufferErrorPattern = /^.*\bmaxBuffer\b.*exceeded.*$/;
if (err instanceof Error && err.message &&
err.message.match(maxBufferErrorPattern)) {
// < v10
// Error: stdout maxBuffer exceeded
return true;
} else if (err instanceof RangeError && err.message &&
err.message.match(maxBufferErrorPattern)) {
// >= v10
// RangeError [ERR_CHILD_PROCESS_STDIO_MAXBUFFER]: stdout maxBuffer length
// exceeded
// TODO(nfischer): remove when we add v10 CI (Github issue #856).
/* istanbul ignore next */
return true;
}
return false;
}

var stdoutStream = fs.createWriteStream(stdoutFile);
var stderrStream = fs.createWriteStream(stderrFile);

function appendError(message, code) {
stderrStream.write(message);
process.exitCode = code;
}

var c = childProcess.exec(cmd, execOptions, function (err) {
if (!err) {
process.exitCode = 0;
} else if (isMaxBufferError(err)) {
appendError('maxBuffer exceeded', 1);
} else if (err.code === undefined && err.message) {
/* istanbul ignore next */
appendError(err.message, 1);
} else if (err.code === undefined) {
process.exitCode = 1;
/* istanbul ignore next */
appendError('Unknown issue', 1);
} else {
process.exitCode = err.code;
}
});

var stdoutStream = fs.createWriteStream(stdoutFile);
var stderrStream = fs.createWriteStream(stderrFile);

c.stdout.pipe(stdoutStream);
c.stderr.pipe(stderrStream);
c.stdout.pipe(process.stdout);
Expand Down
6 changes: 5 additions & 1 deletion test/exec.js
Expand Up @@ -133,8 +133,12 @@ test('set maxBuffer (very small)', t => {
t.falsy(shell.error());
t.is(result.code, 0);
t.is(result.stdout, '1234567890' + os.EOL);
shell.exec('echo 1234567890', { maxBuffer: 6 });
const result2 = shell.exec('echo 1234567890', { maxBuffer: 6 });
t.truthy(shell.error());
t.is(result2.code, 1);
t.is(result2.stdout, '1234567890' + os.EOL);
const maxBufferErrorPattern = /.*\bmaxBuffer\b.*\bexceeded\b.*/;
t.regex(result2.stderr, maxBufferErrorPattern);
});

test('set timeout option', t => {
Expand Down

0 comments on commit e606706

Please sign in to comment.