Merge forceKill and forceKillAfter options (#285)

sindresorhus · Jun 14, 2019 · 0336882 · 0336882
1 parent 7e2c835
commit 0336882
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 38 deletions.
diff --git a/index.d.ts b/index.d.ts
@@ -280,19 +280,14 @@ declare namespace execa {
 	}
 
 	interface KillOptions {
-		/**
-		If the first signal does not terminate the child process after a specified timeout, a `SIGKILL` signal will be sent to the process.
-
-		@default true
-		*/
-		forceKill?: boolean;
-
 		/**
 		Milliseconds to wait for the child process to terminate before sending `SIGKILL`.
 
+		Can be disabled with `false`.
+
 		@default 5000
 		*/
-		forceKillAfter?: number;
+		forceKillAfterTimeout?: number | false;
 	}
 
 	interface ExecaChildPromise<StdoutErrorType> {

diff --git a/index.js b/index.js
@@ -231,21 +231,25 @@ function setKillTimeout(kill, signal, options, killResult) {
 	}, timeout).unref();
 }
 
-function shouldForceKill(signal, {forceKill}, killResult) {
-	return isSigterm(signal) && forceKill !== false && killResult;
+function shouldForceKill(signal, {forceKillAfterTimeout}, killResult) {
+	return isSigterm(signal) && forceKillAfterTimeout !== 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})`);
+function getForceKillAfterTimeout({forceKillAfterTimeout = true}) {
+	if (forceKillAfterTimeout === true) {
+		return DEFAULT_FORCE_KILL_TIMEOUT;
 	}
 
-	return forceKillAfter;
+	if (!Number.isInteger(forceKillAfterTimeout) || forceKillAfterTimeout < 0) {
+		throw new TypeError(`Expected the \`forceKillAfterTimeout\` option to be a non-negative integer, got \`${forceKillAfterTimeout}\` (${typeof forceKillAfterTimeout})`);
+	}
+
+	return forceKillAfterTimeout;
 }
 
 const execa = (file, args, options) => {

diff --git a/index.test-d.ts b/index.test-d.ts
@@ -123,10 +123,9 @@ execa('unicorns').kill();
 execa('unicorns').kill('SIGKILL');
 execa('unicorns').kill(undefined);
 execa('unicorns').kill('SIGKILL', {});
-execa('unicorns').kill('SIGKILL', {forceKill: true});
-execa('unicorns').kill('SIGKILL', {forceKill: false});
-execa('unicorns').kill('SIGKILL', {forceKillAfter: 42});
-execa('unicorns').kill('SIGKILL', {forceKillAfter: undefined});
+execa('unicorns').kill('SIGKILL', {forceKillAfterTimeout: false});
+execa('unicorns').kill('SIGKILL', {forceKillAfterTimeout: 42});
+execa('unicorns').kill('SIGKILL', {forceKillAfterTimeout: undefined});
 
 expectType<ExecaChildProcess<string>>(execa('unicorns'));
 expectType<ExecaReturnValue<string>>(await execa('unicorns'));

diff --git a/readme.md b/readme.md
@@ -117,7 +117,7 @@ try {
 const subprocess = execa('node');
 setTimeout(() => {
 	subprocess.kill('SIGTERM', {
-		forceKillAfter: 2000
+		forceKillAfterTimeout: 2000
 	});
 }, 1000);
 ```
@@ -140,20 +140,15 @@ Returns a [`child_process` instance](https://nodejs.org/api/child_process.html#c
 
 Same as the original [`child_process#kill()`](https://nodejs.org/api/child_process.html#child_process_subprocess_kill_signal) except: if `signal` is `SIGTERM` (the default value) and the child process is not terminated after 5 seconds, force it by sending `SIGKILL`.
 
-##### options.forceKill
+##### options.forceKillAfterTimeout
 
-Type: `boolean`<br>
-Default: `true`
-
-If the first signal does not terminate the child process after a specified timeout, a `SIGKILL` signal will be sent to the process.
-
-##### options.forceKillAfter
-
-Type: `string`<br>
+Type: `number | false`<br>
 Default: `5000`
 
 Milliseconds to wait for the child process to terminate before sending `SIGKILL`.
 
+Can be disabled with `false`.
+
 #### cancel()
 
 Similar to [`childProcess.kill()`](https://nodejs.org/api/child_process.html#child_process_subprocess_kill_signal). This is preferred when cancelling the child process execution as the error is more descriptive and [`childProcessResult.isCanceled`](#iscanceled) is set to `true`.

diff --git a/test.js b/test.js
@@ -129,31 +129,31 @@ test('kill("SIGKILL") should terminate cleanly', async t => {
 // `SIGTERM` cannot be caught on Windows, and it always aborts the process (like `SIGKILL` on Unix).
 // Therefore, this feature and those tests do not make sense on Windows.
 if (process.platform !== 'win32') {
-	test('`forceKill: false` should not kill after a timeout', async t => {
+	test('`forceKillAfterTimeout: false` should not kill after a timeout', async t => {
 		const subprocess = execa('node', ['fixtures/no-killable'], {stdio: ['ipc']});
 		await pEvent(subprocess, 'message');
 
-		subprocess.kill('SIGTERM', {forceKill: false, forceKillAfter: 50});
+		subprocess.kill('SIGTERM', {forceKillAfterTimeout: false});
 
 		t.true(isRunning(subprocess.pid));
 		subprocess.kill('SIGKILL');
 	});
 
-	test('`forceKillAfter: number` should kill after a timeout', async t => {
+	test('`forceKillAfterTimeout: number` should kill after a timeout', async t => {
 		const subprocess = execa('node', ['fixtures/no-killable'], {stdio: ['ipc']});
 		await pEvent(subprocess, 'message');
 
-		subprocess.kill('SIGTERM', {forceKillAfter: 50});
+		subprocess.kill('SIGTERM', {forceKillAfterTimeout: 50});
 
 		const {signal} = await t.throwsAsync(subprocess);
 		t.is(signal, 'SIGKILL');
 	});
 
-	test('`forceKill: true` should kill after a timeout', async t => {
+	test('`forceKillAfterTimeout: true` should kill after a timeout', async t => {
 		const subprocess = execa('node', ['fixtures/no-killable'], {stdio: ['ipc']});
 		await pEvent(subprocess, 'message');
 
-		subprocess.kill('SIGTERM', {forceKill: true});
+		subprocess.kill('SIGTERM', {forceKillAfterTimeout: true});
 
 		const {signal} = await t.throwsAsync(subprocess);
 		t.is(signal, 'SIGKILL');
@@ -169,15 +169,15 @@ if (process.platform !== 'win32') {
 		t.is(signal, 'SIGKILL');
 	});
 
-	test('`forceKillAfter` should not be a float', t => {
+	test('`forceKillAfterTimeout` should not be a float', t => {
 		t.throws(() => {
-			execa('noop').kill('SIGTERM', {forceKillAfter: 0.5});
+			execa('noop').kill('SIGTERM', {forceKillAfterTimeout: 0.5});
 		}, {instanceOf: TypeError, message: /non-negative integer/});
 	});
 
-	test('`forceKillAfter` should not be negative', t => {
+	test('`forceKillAfterTimeout` should not be negative', t => {
 		t.throws(() => {
-			execa('noop').kill('SIGTERM', {forceKillAfter: -1});
+			execa('noop').kill('SIGTERM', {forceKillAfterTimeout: -1});
 		}, {instanceOf: TypeError, message: /non-negative integer/});
 	});
 }