From 5f8bd529569f35b45b49e6b6f25304371e6b8d10 Mon Sep 17 00:00:00 2001 From: ehmicky Date: Fri, 10 May 2019 08:40:06 -0700 Subject: [PATCH] Make error messages more consistent (#230) --- fixtures/error-message.js | 5 ----- index.js | 11 +++++----- test.js | 43 ++++----------------------------------- 3 files changed, 10 insertions(+), 49 deletions(-) delete mode 100755 fixtures/error-message.js diff --git a/fixtures/error-message.js b/fixtures/error-message.js deleted file mode 100755 index 233f1d0c07..0000000000 --- a/fixtures/error-message.js +++ /dev/null @@ -1,5 +0,0 @@ -#!/usr/bin/env node -'use strict'; -console.log('stdout'); -console.error('stderr'); -process.exit(1); diff --git a/index.js b/index.js index 7c04cbad36..dc0f983c50 100644 --- a/index.js +++ b/index.js @@ -177,14 +177,15 @@ function makeError(result, options) { const [exitCodeName, exitCode] = getCode(result, code); - if (!(error instanceof Error)) { - const message = [joinedCommand, stderr, stdout].filter(Boolean).join('\n'); + const prefix = getErrorPrefix({timedOut, timeout, signal, exitCodeName, exitCode, isCanceled}); + const message = `Command ${prefix}: ${joinedCommand}`; + + if (error instanceof Error) { + error.message = `${message}${error.message}`; + } else { error = new Error(message); } - const prefix = getErrorPrefix({timedOut, timeout, signal, exitCodeName, exitCode, isCanceled}); - error.message = `Command ${prefix}: ${error.message}`; - error.code = exitCode || exitCodeName; error.exitCode = exitCode; error.exitCodeName = exitCodeName; diff --git a/test.js b/test.js index 855138ff7c..5d7a4b41a4 100644 --- a/test.js +++ b/test.js @@ -12,8 +12,6 @@ import execa from '.'; process.env.PATH = path.join(__dirname, 'fixtures') + path.delimiter + process.env.PATH; process.env.FOO = 'foo'; -const NO_NEWLINES_REGEXP = /^[^\n]*$/; -const STDERR_STDOUT_REGEXP = /stderr[^]*stdout/; const TIMEOUT_REGEXP = /timed out after/; const getExitRegExp = exitMessage => new RegExp(`failed with exit code ${exitMessage}`); @@ -63,32 +61,6 @@ test('stdout/stderr/all available on errors', async t => { t.is(typeof error.all, 'string'); }); -test('include stdout and stderr in errors for improved debugging', async t => { - await t.throwsAsync(execa('fixtures/error-message.js'), {message: STDERR_STDOUT_REGEXP, code: 1}); -}); - -test('do not include in errors when `stdio` is set to `inherit`', async t => { - await t.throwsAsync(execa('fixtures/error-message.js', {stdio: 'inherit'}), {message: NO_NEWLINES_REGEXP}); -}); - -test('do not include `stderr` and `stdout` in errors when set to `inherit`', async t => { - await t.throwsAsync(execa('fixtures/error-message.js', {stdout: 'inherit', stderr: 'inherit'}), {message: NO_NEWLINES_REGEXP}); -}); - -test('do not include `stderr` and `stdout` in errors when `stdio` is set to `inherit`', async t => { - await t.throwsAsync(execa('fixtures/error-message.js', {stdio: [undefined, 'inherit', 'inherit']}), {message: NO_NEWLINES_REGEXP}); -}); - -test('do not include `stdout` in errors when set to `inherit`', async t => { - const error = await t.throwsAsync(execa('fixtures/error-message.js', {stdout: 'inherit'}), {message: /stderr/}); - t.notRegex(error.message, /stdout/); -}); - -test('do not include `stderr` in errors when set to `inherit`', async t => { - const error = await t.throwsAsync(execa('fixtures/error-message.js', {stderr: 'inherit'}), {message: /stdout/}); - t.notRegex(error.message, /stderr/); -}); - test('pass `stdout` to a file descriptor', async t => { const file = tempfile('.txt'); await execa('fixtures/noop', ['foo bar'], {stdout: fs.openSync(file, 'w')}); @@ -145,19 +117,12 @@ test('execa.sync()', t => { test('execa.sync() throws error if written to stderr', t => { t.throws(() => { execa.sync('foo'); - }, process.platform === 'win32' ? /'foo' is not recognized as an internal or external command/ : /spawnSync foo ENOENT/); -}); - -test('execa.sync() includes stdout and stderr in errors for improved debugging', t => { - t.throws(() => { - execa.sync('node', ['fixtures/error-message.js']); - }, {message: STDERR_STDOUT_REGEXP, code: 1}); + }, process.platform === 'win32' ? /failed with exit code 1/ : /spawnSync foo ENOENT/); }); test('skip throwing when using reject option in execa.sync()', t => { - const error = execa.sync('node', ['fixtures/error-message.js'], {reject: false}); - t.is(typeof error.stdout, 'string'); - t.is(typeof error.stderr, 'string'); + const error = execa.sync('noop-err', ['foo'], {reject: false}); + t.is(error.stderr, 'foo'); }); test('stripEof option (legacy)', async t => { @@ -172,7 +137,7 @@ test('stripFinalNewline option', async t => { test('preferLocal option', async t => { await execa('ava', ['--version'], {env: {PATH: ''}}); - const errorRegExp = process.platform === 'win32' ? /not recognized/ : /spawn ava ENOENT/; + const errorRegExp = process.platform === 'win32' ? /failed with exit code 1/ : /spawn ava ENOENT/; await t.throwsAsync(execa('ava', ['--version'], {preferLocal: false, env: {PATH: ''}}), errorRegExp); });