Skip to content

Commit 74f1ff8

Browse files
nfischernfischer
committed
Commands that have multiple errors now produce cleaner log output. Errors are
echoed to stderr. exec() supports a new stderr field.
1 parent ace162a commit 74f1ff8

9 files changed

Lines changed: 76 additions & 52 deletions

File tree

‎README.md‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -488,16 +488,17 @@ child.stdout.on('data', function(data) {
488488
/* ... do something with data ... */
489489
});
490490

491-
exec('some_long_running_process', function(code, output) {
491+
exec('some_long_running_process', function(code, output, stderr) {
492492
console.log('Exit code:', code);
493493
console.log('Program output:', output);
494+
console.log('Program stderr:', stderr);
494495
});
495496
```
496497

497-
Executes the given `command` _synchronously_, unless otherwise specified.
498-
When in synchronous mode returns the object `{ code:..., output:... }`, containing the program's
499-
`output` (stdout + stderr) and its exit `code`. Otherwise returns the child process object, and
500-
the `callback` gets the arguments `(code, output)`.
498+
Executes the given `command` _synchronously_, unless otherwise specified. When in synchronous
499+
mode returns the object `{ code:..., output:... , stderr:... }`, containing the program's
500+
`output` (stdout), `stderr`, and its exit `code`. Otherwise returns the child process object,
501+
and the `callback` gets the arguments `(code, output, stderr)`.
501502

502503
**Note:** For long-lived processes, it's best to run `exec()` asynchronously as
503504
the current synchronous implementation uses a lot of CPU. This should be getting

‎src/common.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,22 @@ exports.platform = platform;
2121

2222
function log() {
2323
if (!config.silent)
24-
console.log.apply(this, arguments);
24+
console.error.apply(this, arguments);
2525
}
2626
exports.log = log;
2727

2828
// Shows error message. Throws unless _continue or config.fatal are true
2929
function error(msg, _continue) {
3030
if (state.error === null)
3131
state.error = '';
32-
state.error += state.currentCmd + ': ' + msg + '\n';
32+
var log_entry = state.currentCmd + ': ' + msg;
33+
if (state.error === '')
34+
state.error = log_entry;
35+
else
36+
state.error += '\n' + log_entry;
3337

3438
if (msg.length > 0)
35-
log(state.error);
39+
log(log_entry);
3640

3741
if (config.fatal)
3842
process.exit(1);

‎src/exec.js‎

Lines changed: 45 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ var child = require('child_process');
1313
function execSync(cmd, opts) {
1414
var tempDir = _tempDir();
1515
var stdoutFile = path.resolve(tempDir+'/'+common.randomFileName()),
16+
stderrFile = path.resolve(tempDir+'/'+common.randomFileName()),
1617
codeFile = path.resolve(tempDir+'/'+common.randomFileName()),
1718
scriptFile = path.resolve(tempDir+'/'+common.randomFileName()),
1819
sleepFile = path.resolve(tempDir+'/'+common.randomFileName());
@@ -21,19 +22,30 @@ function execSync(cmd, opts) {
2122
silent: common.config.silent
2223
}, opts);
2324

24-
var previousStdoutContent = '';
25-
// Echoes stdout changes from running process, if not silent
26-
function updateStdout() {
27-
if (options.silent || !fs.existsSync(stdoutFile))
25+
var previousStdoutContent = '',
26+
previousStderrContent = '';
27+
// Echoes stdout and stderr changes from running process, if not silent
28+
function updateStream(streamFile) {
29+
if (options.silent || !fs.existsSync(streamFile))
2830
return;
2931

30-
var stdoutContent = fs.readFileSync(stdoutFile, 'utf8');
32+
var previousStreamContent,
33+
proc_stream;
34+
if (streamFile === stdoutFile) {
35+
previousStreamContent = previousStdoutContent;
36+
proc_stream = process.stdout;
37+
} else { // assume stderr
38+
previousStreamContent = previousStderrContent;
39+
proc_stream = process.stderr;
40+
}
41+
42+
var streamContent = fs.readFileSync(streamFile, 'utf8');
3143
// No changes since last time?
32-
if (stdoutContent.length <= previousStdoutContent.length)
44+
if (streamContent.length <= previousStreamContent.length)
3345
return;
3446

35-
process.stdout.write(stdoutContent.substr(previousStdoutContent.length));
36-
previousStdoutContent = stdoutContent;
47+
proc_stream.write(streamContent.substr(previousStreamContent.length));
48+
previousStreamContent = streamContent;
3749
}
3850

3951
function escape(str) {
@@ -42,6 +54,7 @@ function execSync(cmd, opts) {
4254

4355
if (fs.existsSync(scriptFile)) common.unlinkSync(scriptFile);
4456
if (fs.existsSync(stdoutFile)) common.unlinkSync(stdoutFile);
57+
if (fs.existsSync(stderrFile)) common.unlinkSync(stderrFile);
4558
if (fs.existsSync(codeFile)) common.unlinkSync(codeFile);
4659

4760
var execCommand = '"'+process.execPath+'" '+scriptFile;
@@ -59,14 +72,16 @@ function execSync(cmd, opts) {
5972
" fs.writeFileSync('"+escape(codeFile)+"', err ? err.code.toString() : '0');",
6073
"});",
6174
"var stdoutStream = fs.createWriteStream('"+escape(stdoutFile)+"');",
75+
"var stderrStream = fs.createWriteStream('"+escape(stderrFile)+"');",
6276
"childProcess.stdout.pipe(stdoutStream, {end: false});",
63-
"childProcess.stderr.pipe(stdoutStream, {end: false});",
77+
"childProcess.stderr.pipe(stderrStream, {end: false});",
6478
"childProcess.stdout.pipe(process.stdout);",
6579
"childProcess.stderr.pipe(process.stderr);",
6680
"var stdoutEnded = false, stderrEnded = false;",
67-
"function tryClosing(){ if(stdoutEnded && stderrEnded){ stdoutStream.end(); } }",
68-
"childProcess.stdout.on('end', function(){ stdoutEnded = true; tryClosing(); });",
69-
"childProcess.stderr.on('end', function(){ stderrEnded = true; tryClosing(); });"
81+
"function tryClosingStdout(){ if(stdoutEnded){ stdoutStream.end(); } }",
82+
"function tryClosingStderr(){ if(stderrEnded){ stderrStream.end(); } }",
83+
"childProcess.stdout.on('end', function(){ stdoutEnded = true; tryClosingStdout(); });",
84+
"childProcess.stderr.on('end', function(){ stderrEnded = true; tryClosingStderr(); });"
7085
].join('\n');
7186

7287
fs.writeFileSync(scriptFile, script);
@@ -80,7 +95,7 @@ function execSync(cmd, opts) {
8095
// Welcome to the future
8196
child.execSync(execCommand, execOptions);
8297
} else {
83-
cmd += ' > '+stdoutFile+' 2>&1'; // works on both win/unix
98+
cmd += ' > '+stdoutFile+' 2> '+stderrFile; // works on both win/unix
8499

85100
var script = [
86101
"var child = require('child_process')",
@@ -98,8 +113,9 @@ function execSync(cmd, opts) {
98113
// sleepFile is used as a dummy I/O op to mitigate unnecessary CPU usage
99114
// (tried many I/O sync ops, writeFileSync() seems to be only one that is effective in reducing
100115
// CPU usage, though apparently not so much on Windows)
101-
while (!fs.existsSync(codeFile)) { updateStdout(); fs.writeFileSync(sleepFile, 'a'); }
102-
while (!fs.existsSync(stdoutFile)) { updateStdout(); fs.writeFileSync(sleepFile, 'a'); }
116+
while (!fs.existsSync(codeFile)) { updateStream(stdoutFile); fs.writeFileSync(sleepFile, 'a'); }
117+
while (!fs.existsSync(stdoutFile)) { updateStream(stdoutFile); fs.writeFileSync(sleepFile, 'a'); }
118+
while (!fs.existsSync(stderrFile)) { updateStream(stderrFile); fs.writeFileSync(sleepFile, 'a'); }
103119
}
104120

105121
// At this point codeFile exists, but it's not necessarily flushed yet.
@@ -110,10 +126,12 @@ function execSync(cmd, opts) {
110126
}
111127

112128
var stdout = fs.readFileSync(stdoutFile, 'utf8');
129+
var stderr = fs.readFileSync(stderrFile, 'utf8');
113130

114131
// No biggie if we can't erase the files now -- they're in a temp dir anyway
115132
try { common.unlinkSync(scriptFile); } catch(e) {}
116133
try { common.unlinkSync(stdoutFile); } catch(e) {}
134+
try { common.unlinkSync(stderrFile); } catch(e) {}
117135
try { common.unlinkSync(codeFile); } catch(e) {}
118136
try { common.unlinkSync(sleepFile); } catch(e) {}
119137

@@ -124,22 +142,24 @@ function execSync(cmd, opts) {
124142
// True if successful, false if not
125143
var obj = {
126144
code: code,
127-
output: stdout
145+
output: stdout,
146+
stderr: stderr
128147
};
129148
return obj;
130149
} // execSync()
131150

132151
// Wrapper around exec() to enable echoing output to console in real time
133152
function execAsync(cmd, opts, callback) {
134153
var output = '';
154+
var stderr = '';
135155

136156
var options = common.extend({
137157
silent: common.config.silent
138158
}, opts);
139159

140160
var c = child.exec(cmd, {env: process.env, maxBuffer: 20*1024*1024}, function(err) {
141161
if (callback)
142-
callback(err ? err.code : 0, output);
162+
callback(err ? err.code : 0, output, stderr);
143163
});
144164

145165
c.stdout.on('data', function(data) {
@@ -149,9 +169,9 @@ function execAsync(cmd, opts, callback) {
149169
});
150170

151171
c.stderr.on('data', function(data) {
152-
output += data;
172+
stderr += data;
153173
if (!options.silent)
154-
process.stdout.write(data);
174+
process.stderr.write(data);
155175
});
156176

157177
return c;
@@ -174,16 +194,17 @@ function execAsync(cmd, opts, callback) {
174194
//@ /* ... do something with data ... */
175195
//@ });
176196
//@
177-
//@ exec('some_long_running_process', function(code, output) {
197+
//@ exec('some_long_running_process', function(code, output, stderr) {
178198
//@ console.log('Exit code:', code);
179199
//@ console.log('Program output:', output);
200+
//@ console.log('Program stderr:', stderr);
180201
//@ });
181202
//@ ```
182203
//@
183-
//@ Executes the given `command` _synchronously_, unless otherwise specified.
184-
//@ When in synchronous mode returns the object `{ code:..., output:... }`, containing the program's
185-
//@ `output` (stdout + stderr) and its exit `code`. Otherwise returns the child process object, and
186-
//@ the `callback` gets the arguments `(code, output)`.
204+
//@ Executes the given `command` _synchronously_, unless otherwise specified. When in synchronous
205+
//@ mode returns the object `{ code:..., output:... , stderr:... }`, containing the program's
206+
//@ `output` (stdout), `stderr`, and its exit `code`. Otherwise returns the child process object,
207+
//@ and the `callback` gets the arguments `(code, output, stderr)`.
187208
//@
188209
//@ **Note:** For long-lived processes, it's best to run `exec()` asynchronously as
189210
//@ the current synchronous implementation uses a lot of CPU. This should be getting

‎test/cp.js‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,11 @@
11
var shell = require('..');
22

33
var assert = require('assert'),
4-
fs = require('fs');
4+
fs = require('fs'),
5+
numLines = require('./utils/utils').numLines;
56

67
shell.config.silent = true;
78

8-
function numLines(str) {
9-
return typeof str === 'string' ? str.match(/\n/g).length : 0;
10-
}
11-
129
shell.rm('-rf', 'tmp');
1310
shell.mkdir('tmp');
1411

‎test/exec.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,15 @@ assert.ok(result.output === '1234\n' || result.output === '1234\nundefined\n');
5050
var result = shell.exec('node -e \"console.error(1234);\"');
5151
assert.equal(shell.error(), null);
5252
assert.equal(result.code, 0);
53-
assert.ok(result.output === '1234\n' || result.output === '1234\nundefined\n'); // 'undefined' for v0.4
53+
assert.ok(result.output === '' || result.output === 'undefined\n'); // 'undefined' for v0.4
54+
assert.ok(result.stderr === '1234\n' || result.stderr === '1234\nundefined\n'); // 'undefined' for v0.4
5455

5556
// check if stdout + stderr go to output
5657
var result = shell.exec('node -e \"console.error(1234); console.log(666);\"');
5758
assert.equal(shell.error(), null);
5859
assert.equal(result.code, 0);
59-
assert.ok(result.output === '1234\n666\n' || result.output === '1234\n666\nundefined\n'); // 'undefined' for v0.4
60+
assert.ok(result.output === '666\n' || result.output === '666\nundefined\n'); // 'undefined' for v0.4
61+
assert.ok(result.stderr === '1234\n' || result.stderr === '1234\nundefined\n'); // 'undefined' for v0.4
6062

6163
// check exit code
6264
var result = shell.exec('node -e \"process.exit(12);\"');

‎test/mkdir.js‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,11 @@
11
var shell = require('..');
22

33
var assert = require('assert'),
4-
fs = require('fs');
4+
fs = require('fs'),
5+
numLines = require('./utils/utils').numLines;
56

67
shell.config.silent = true;
78

8-
function numLines(str) {
9-
return typeof str === 'string' ? str.match(/\n/g).length : 0;
10-
}
11-
129
shell.rm('-rf', 'tmp');
1310
shell.mkdir('tmp');
1411

‎test/mv.js‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,11 @@
11
var shell = require('..');
22

33
var assert = require('assert'),
4-
fs = require('fs');
4+
fs = require('fs'),
5+
numLines = require('./utils/utils').numLines;
56

67
shell.config.silent = true;
78

8-
function numLines(str) {
9-
return typeof str === 'string' ? str.match(/\n/g).length : 0;
10-
}
11-
129
shell.rm('-rf', 'tmp');
1310
shell.mkdir('tmp');
1411

‎test/pushd.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ assert.deepEqual(trail, [
183183

184184
// Push invalid directory
185185
shell.pushd('does/not/exist');
186-
assert.equal(shell.error(), 'pushd: no such file or directory: ' + path.resolve('.', 'does/not/exist') + '\n');
186+
assert.equal(shell.error(), 'pushd: no such file or directory: ' + path.resolve('.', 'does/not/exist'));
187187
assert.equal(process.cwd(), trail[0]);
188188

189189
// Push without arguments should swap top two directories when stack length is 2
@@ -219,6 +219,6 @@ assert.equal(process.cwd(), trail[0]);
219219

220220
// Push without arguments invalid when stack is empty
221221
reset(); shell.pushd();
222-
assert.equal(shell.error(), 'pushd: no other directory\n');
222+
assert.equal(shell.error(), 'pushd: no other directory');
223223

224-
shell.exit(123);
224+
shell.exit(123);

‎test/utils/utils.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
function _numLines(str) {
2+
return typeof str === 'string' ? (str.match(/\n/g)||[]).length+1 : 0;
3+
}
4+
5+
exports.numLines = _numLines;

0 commit comments

Comments
 (0)