10000 Remove separate "internal error" from exec (#802) · shelljs/shelljs@9077f41 · GitHub
[go: up one dir, main page]
Skip to content

Commit 9077f41

Browse files
freitagbrfreitagbr
authored
Remove separate "internal error" from exec (#802)
* Remove separate "internal error" from exec * Fix unknown command error regex * Add message about command not found regex * Silence errors while reading files in exec The stdout and stderr files may never be opened or written to in certain circumstances. In particular, if the timeout is short enough, the child node process does not have enough time to start, and the child script does not execute, so the files are not written to. So, catch errors form trying to read the files, and ignore them. * Do not silence errors due to short timeouts * Simplify test regex for missing command * Default error code to 1 if not set
1 parent 62ce4ba commit 9077f41

File tree

2 files changed

+14
-15
lines changed

2 files changed

+14
-15
lines changed

‎src/exec.js

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ var fs = require('fs');
66
var child = require('child_process');
77

88
var DEFAULT_MAXBUFFER_SIZE = 20 * 1024 * 1024;
9+
var DEFAULT_ERROR_CODE = 1;
910

1011
common.register('exec', _exec, {
1112
unix: false,
@@ -72,13 +73,15 @@ function execSync(cmd, opts, pipe) {
7273
child.execFileSync(common.config.execPath, execArgs, opts);
7374
} catch (e) {
7475
// Commands with non-zero exit code raise an exception.
75-
code = e.status;
76+
code = e.status || DEFAULT_ERROR_CODE;
7677
}
7778

7879
// fs.readFileSync uses buffer encoding by default, so call
79-
// it without the encoding option if the encoding is 'buffer'
80-
var stdout;
81-
var stderr;
80+
// it without the encoding option if the encoding is 'buffer'.
81+
// Also, if the exec timeout is too short for node to start up,
82+
// the files will not be created, so these calls will throw.
83+
var stdout = '';
84+
var stderr = '';
8285
if (opts.encoding === 'buffer') {
8386
stdout = fs.readFileSync(stdoutFile);
8487
stderr = fs.readFileSync(stderrFile);
@@ -93,7 +96,7 @@ function execSync(cmd, opts, pipe) {
9396
try { common.unlinkSync(stdoutFile); } catch (e) {}
9497

9598
if (code !== 0) {
96-
common.error('', code, { continue: true });
99+
common.error(stderr, code, { continue: true });
97100
}
98101
var obj = common.ShellString(stdout, stderr, code);
99102
return obj;
@@ -196,14 +199,10 @@ function _exec(command, options, callback) {
196199
async: false,
197200
}, options);
198201

199-
try {
200-
if (options.async) {
201-
return execAsync(command, options, pipe, callback);
202-
} else {
203-
return execSync(command, options, pipe);
204-
}
205-
} catch (e) {
206-
common.error('internal error');
202+
if (options.async) {
203+
return execAsync(command, options, pipe, callback);
204+
} else {
205+
return execSync(command, options, pipe);
207206
}
208207
}
209208
module.exports = _exec;

‎test/exec.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ test('config.fatal and unknown command', t => {
3535
shell.config.fatal = true;
3636
t.throws(() => {
3737
shell.exec('asdfasdf'); // could not find command
38-
}, /exec: internal error/);
38+
}, /asdfasdf/); // name of command should be in error message
3939
shell.config.fatal = oldFatal;
4040
});
4141

@@ -127,7 +127,7 @@ test('set timeout option', t => {
127127
const result = shell.exec(`${JSON.stringify(shell.config.execPath)} test/resources/exec/slow.js 100`); // default timeout is ok
128128
t.falsy(shell.error());
129129
t.is(result.code, 0);
130-
shell.exec(`${JSON.stringify(shell.config.execPath)} test/resources/exec/slow.js 100`, { timeout: 10 }); // times out
130+
shell.exec(`${JSON.stringify(shell.config.execPath)} test/resources/exec/slow.js 2000`, { timeout: 1000 }); // times out
131131
t.truthy(shell.error());
132132
});
133133

0 commit comments

Comments
 (0)
0