8000 refactor: change how internal errors are handled by nfischer · Pull Request #1222 · shelljs/shelljs · GitHub
[go: up one dir, main page]
Skip to content

refactor: change how internal errors are handled #1222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 25, 2025
Merged

refactor: change how internal errors are handled #1222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions src/cmd.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function isNullOrUndefined(val) {
return val === null || val === undefined;
}

function commandNotFound(execaResult) {
function isCommandNotFound(execaResult) {
if (process.platform === 'win32') {
var str = 'is not recognized as an internal or external command';
return execaResult.code && execaResult.stderr.includes(str);
Expand All @@ -28,6 +28,16 @@ function commandNotFound(execaResult) {
}
}

function isExecaInternalError(result) {
if (typeof result.stdout !== 'string') return true;
if (typeof result.stderr !== 'string') return true;
if (typeof result.code !== 'number') return true;
if (result.code === 0 && result.failed) return true;
// Otherwise assume this executed correctly. The command may still have exited
// with non-zero status, but that's not due to anything execa did.
return false;
}

//@
//@ ### cmd(arg1[, arg2, ...] [, options])
//@
Expand Down Expand Up @@ -110,26 +120,24 @@ function _cmd(options, command, commandArgs, userOptions) {
var stdout;
var stderr;
var code;
if (commandNotFound(result)) {
if (isCommandNotFound(result)) {
// This can happen if `command` is not an executable binary, or possibly
// under other conditions.
stdout = '';
stderr = "'" + command + "': command not found";
code = COMMAND_NOT_FOUND_ERROR_CODE;
} else if (typeof result.stdout === 'string' &&
typeof result.stderr === 'string' &&
typeof result.code === 'number') {
// Normal exit: execa was able to execute `command` and get a return value.
stdout = result.stdout.toString();
stderr = result.stderr.toString();
code = result.code;
} else {
} else if (isExecaInternalError(result)) {
// Catch-all: execa tried to run `command` but it encountered some error
// (ex. maxBuffer, timeout).
stdout = result.stdout || '';
stderr = result.stderr ||
`'${command}' encountered an error during execution`;
code = result.code > 0 ? result.code : 1;
code = typeof result.code === 'number' && result.code > 0 ? result.code : 1;
} else {
// Normal exit: execa was able to execute `command` and get a return value.
stdout = result.stdout.toString();
stderr = result.stderr.toString();
code = result.code;
}

// Pass `continue: true` so we can specify a value for stdout.
Expand Down
8 changes: 8 additions & 0 deletions test/cmd.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,14 @@ test('set cwd', t => {
t.is(result.stdout, path.resolve('..') + '\n');
});

test('command fails silently with non-zero status', t => {
const result = shell.cmd('shx', 'false');
t.truthy(shell.error());
t.is(result.code, 1);
t.is(result.stdout, '');
t.is(result.stderr, '');
});

test('set maxBuffer (very small)', t => {
let result = shell.cmd('shx', 'echo', '1234567890'); // default maxBuffer is ok
t.falsy(shell.error());
Expand Down
0