From c892ddfed00cfea816812e58c8cc7186a7b7b35b Mon Sep 17 00:00:00 2001 From: Nate Fischer Date: Sun, 2 Dec 2018 20:52:04 -0800 Subject: [PATCH 1/2] 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 --- src/exec-child.js | 27 ++++++++++++++++++++++++--- test/exec.js | 6 +++++- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/exec-child.js b/src/exec-child.js index eab86ed3..cf2e4f49 100644 --- a/src/exec-child.js +++ b/src/exec-child.js @@ -16,9 +16,33 @@ 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 + return true; + } + return false; +} + +var stdoutStream = fs.createWriteStream(stdoutFile); +var stderrStream = fs.createWriteStream(stderrFile); + var c = childProcess.exec(cmd, execOptions, function (err) { if (!err) { process.exitCode = 0; + } else if (isMaxBufferError(err)) { + console.warn('I hit the case I want to hit.'); + stderrStream.write('maxBuffer exceeded'); + process.exitCode = 1; } else if (err.code === undefined) { process.exitCode = 1; } else { @@ -26,9 +50,6 @@ var c = childProcess.exec(cmd, execOptions, function (err) { } }); -var stdoutStream = fs.createWriteStream(stdoutFile); -var stderrStream = fs.createWriteStream(stderrFile); - c.stdout.pipe(stdoutStream); c.stderr.pipe(stderrStream); c.stdout.pipe(process.stdout); diff --git a/test/exec.js b/test/exec.js index 18c9b1b5..2e09658a 100644 --- a/test/exec.js +++ b/test/exec.js @@ -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 => { From 5b4417b8c3bd50e523f2cc27005c33e8830e34e7 Mon Sep 17 00:00:00 2001 From: Nate Fischer Date: Sun, 2 Dec 2018 21:27:40 -0800 Subject: [PATCH 2/2] Remove debugging log, ignore uncovered lines --- src/exec-child.js | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/exec-child.js b/src/exec-child.js index cf2e4f49..999618cb 100644 --- a/src/exec-child.js +++ b/src/exec-child.js @@ -28,6 +28,8 @@ function isMaxBufferError(err) { // >= 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; @@ -36,15 +38,22 @@ function isMaxBufferError(err) { 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)) { - console.warn('I hit the case I want to hit.'); - stderrStream.write('maxBuffer exceeded'); - process.exitCode = 1; + 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; }