From e96350d859dc03189aec270cf5b75bc95c33cbff Mon Sep 17 00:00:00 2001
From: James Talmage <james.talmage@jrtechnical.com>
Date: Thu, 5 Jan 2017 17:51:58 -0500
Subject: [PATCH] Reimplement missing features. Fixes #30 & #32 (#68)

* Reimplement stream cleanup behavior from node core. Fixes #32

* Implement timeout function. Fixes #30.

* add longer delay to timeout test, because AppVeyor.
---
 fixtures/delay |  9 ++++++++
 index.js       | 61 ++++++++++++++++++++++++++++++++++++++------------
 package.json   |  1 +
 test.js        | 26 +++++++++++++++++++++
 4 files changed, 83 insertions(+), 14 deletions(-)
 create mode 100755 fixtures/delay

diff --git a/fixtures/delay b/fixtures/delay
new file mode 100755
index 0000000000..103bc5ce9d
--- /dev/null
+++ b/fixtures/delay
@@ -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');
diff --git a/index.js b/index.js
index 0d56c0555b..0219cfa73f 100644
--- a/index.js
+++ b/index.js
@@ -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');
 
@@ -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;
 
@@ -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 => {
@@ -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;
@@ -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);
 
diff --git a/package.json b/package.json
index a604ec6acc..59f8d32d37 100644
--- a/package.json
+++ b/package.json
@@ -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"
   },
diff --git a/test.js b/test.js
index 4ab0bde9b6..d6c06aeb12 100644
--- a/test.js
+++ b/test.js
@@ -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));