From 1b38dc9754605c304828bdafc4f521233c85f1f4 Mon Sep 17 00:00:00 2001
From: ehmicky <ehmicky@users.noreply.github.com>
Date: Tue, 11 Jun 2019 19:57:23 -0700
Subject: [PATCH] Refactor `kill()` again (#280)

---
 index.js | 30 ++++++++++++++++++++----------
 test.js  | 12 ++++++++++++
 2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/index.js b/index.js
index b937e39c75..fe0d0a2148 100644
--- a/index.js
+++ b/index.js
@@ -14,6 +14,7 @@ const onExit = require('signal-exit');
 const stdio = require('./lib/stdio');
 
 const TEN_MEGABYTES = 1000 * 1000 * 10;
+const DEFAULT_FORCE_KILL_TIMEOUT = 1000 * 5;
 
 const SPACES_REGEXP = / +/g;
 
@@ -224,18 +225,27 @@ function setKillTimeout(kill, signal, options, killResult) {
 		return;
 	}
 
-	const forceKillAfter = Number.isInteger(options.forceKillAfter) ?
-		options.forceKillAfter :
-		5000;
-	setTimeout(() => kill('SIGKILL'), forceKillAfter).unref();
+	const timeout = getForceKillAfterTimeout(options);
+	setTimeout(() => {
+		kill('SIGKILL');
+	}, timeout).unref();
 }
 
-function shouldForceKill(signal, options, killResult) {
-	return ((typeof signal === 'string' &&
-		signal.toUpperCase() === 'SIGTERM') ||
-		signal === os.constants.signals.SIGTERM) &&
-		options.forceKill !== false &&
-		killResult;
+function shouldForceKill(signal, {forceKill}, killResult) {
+	return isSigterm(signal) && forceKill !== false && killResult;
+}
+
+function isSigterm(signal) {
+	return signal === os.constants.signals.SIGTERM ||
+		(typeof signal === 'string' && signal.toUpperCase() === 'SIGTERM');
+}
+
+function getForceKillAfterTimeout({forceKillAfter = DEFAULT_FORCE_KILL_TIMEOUT}) {
+	if (!Number.isInteger(forceKillAfter) || forceKillAfter < 0) {
+		throw new TypeError(`Expected the \`forceKillAfter\` option to be a non-negative integer, got \`${forceKillAfter}\` (${typeof forceKillAfter})`);
+	}
+
+	return forceKillAfter;
 }
 
 const execa = (file, args, options) => {
diff --git a/test.js b/test.js
index 04bd470981..718e32a0fb 100644
--- a/test.js
+++ b/test.js
@@ -175,6 +175,18 @@ if (process.platform !== 'win32') {
 		const {signal} = await t.throwsAsync(subprocess);
 		t.is(signal, 'SIGKILL');
 	});
+
+	test('.kill() `forceKillAfter` should not be a float', t => {
+		t.throws(() => {
+			execa('noop').kill('SIGTERM', {forceKillAfter: 0.5});
+		}, {instanceOf: TypeError, message: /non-negative integer/});
+	});
+
+	test('.kill() `forceKillAfter` should not be negative', t => {
+		t.throws(() => {
+			execa('noop').kill('SIGTERM', {forceKillAfter: -1});
+		}, {instanceOf: TypeError, message: /non-negative integer/});
+	});
 }
 
 test('stripFinalNewline: true', async t => {