Skip to content

Commit

Permalink
Reimplement missing features. Fixes #30 & #32 (#68)
Browse files Browse the repository at this point in the history
* Reimplement stream cleanup behavior from node core. Fixes #32

* Implement timeout function. Fixes #30.

* add longer delay to timeout test, because AppVeyor.
  • Loading branch information
jamestalmage authored and sindresorhus committed Jan 5, 2017
1 parent 842be7c commit e96350d
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 14 deletions.
9 changes: 9 additions & 0 deletions fixtures/delay
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/usr/bin/env node
'use strict';

setTimeout(() => {
console.log('delay completed');
process.exit(Number(process.argv[3] || 0));
}, Number(process.argv[2]));

console.log('delay started');
61 changes: 47 additions & 14 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const stripEof = require('strip-eof');
const npmRunPath = require('npm-run-path');
const isStream = require('is-stream');
const _getStream = require('get-stream');
const pFinally = require('p-finally');
const onExit = require('signal-exit');
const errname = require('./lib/errname');

Expand Down Expand Up @@ -113,16 +114,6 @@ function getStream(process, stream, encoding, maxBuffer) {
});
}

const processDone = spawned => new Promise(resolve => {
spawned.on('exit', (code, signal) => {
resolve({code, signal});
});

spawned.on('error', err => {
resolve({err});
});
});

module.exports = (cmd, args, opts) => {
let joinedCmd = cmd;

Expand All @@ -148,8 +139,48 @@ module.exports = (cmd, args, opts) => {
});
}

const promise = Promise.all([
processDone(spawned),
let timeoutId = null;
let timedOut = false;

const cleanupTimeout = () => {
if (timeoutId) {
clearTimeout(timeoutId);
timeoutId = null;
}
};

if (parsed.opts.timeout > 0) {
timeoutId = setTimeout(() => {
timeoutId = null;
timedOut = true;
spawned.kill(parsed.killSignal);
}, parsed.opts.timeout);
}

const processDone = new Promise(resolve => {
spawned.on('exit', (code, signal) => {
cleanupTimeout();
resolve({code, signal});
});

spawned.on('error', err => {
cleanupTimeout();
resolve({err});
});
});

function destroy() {
if (spawned.stdout) {
spawned.stdout.destroy();
}

if (spawned.stderr) {
spawned.stderr.destroy();
}
}

const promise = pFinally(Promise.all([
processDone,
getStream(spawned, 'stdout', encoding, maxBuffer),
getStream(spawned, 'stderr', encoding, maxBuffer)
]).then(arr => {
Expand Down Expand Up @@ -181,6 +212,7 @@ module.exports = (cmd, args, opts) => {
err.failed = true;
err.signal = signal || null;
err.cmd = joinedCmd;
err.timedOut = timedOut;

if (!parsed.opts.reject) {
return err;
Expand All @@ -196,9 +228,10 @@ module.exports = (cmd, args, opts) => {
failed: false,
killed: false,
signal: null,
cmd: joinedCmd
cmd: joinedCmd,
timedOut: false
};
});
}), destroy);

crossSpawn._enoent.hookChildProcess(spawned, parsed);

Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
"get-stream": "^2.2.0",
"is-stream": "^1.1.0",
"npm-run-path": "^2.0.0",
"p-finally": "^1.0.0",
"signal-exit": "^3.0.0",
"strip-eof": "^1.0.0"
},
Expand Down
26 changes: 26 additions & 0 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,32 @@ test('err.code is 2', code, 2);
test('err.code is 3', code, 3);
test('err.code is 4', code, 4);

test('timeout will kill the process early', async t => {
const err = await t.throws(m('delay', ['3000', '0'], {timeout: 1500}));

t.true(err.timedOut);
t.not(err.code, 22);
});

test('timeout will not kill the process early', async t => {
const err = await t.throws(m('delay', ['3000', '22'], {timeout: 9000}));

t.false(err.timedOut);
t.is(err.code, 22);
});

test('timedOut will be false if no timeout was set and zero exit code', async t => {
const result = await m('delay', ['1000', '0']);

t.false(result.timedOut);
});

test('timedOut will be false if no timeout was set and non-zero exit code', async t => {
const err = await t.throws(m('delay', ['1000', '3']));

t.false(err.timedOut);
});

async function errorMessage(t, expected, ...args) {
const err = await t.throws(m('exit', args));

Expand Down

0 comments on commit e96350d

Please sign in to comment.