Improve validation and typing of signals

lib/arguments/options.js

@@ -3,6 +3,7 @@ import process from 'node:process';
 import crossSpawn from 'cross-spawn';
 import {npmRunPathEnv} from 'npm-run-path';
 import {normalizeForceKillAfterDelay} from '../terminate/kill.js';
+import {normalizeKillSignal} from '../terminate/signal.js';
 import {validateTimeout} from '../terminate/timeout.js';
 import {handleNodeOption} from '../methods/node.js';
 import {validateEncoding, BINARY_ENCODINGS} from './encoding-option.js';
@@ -24,6 +25,7 @@ export const normalizeOptions = (filePath, rawArguments, rawOptions) => {
 	validateEncoding(options);
 	options.shell = normalizeFileUrl(options.shell);
 	options.env = getEnv(options);
+	options.killSignal = normalizeKillSignal(options.killSignal);
 	options.forceKillAfterDelay = normalizeForceKillAfterDelay(options.forceKillAfterDelay);
 	options.lines = options.lines.map((lines, fdNumber) => lines && !BINARY_ENCODINGS.has(options.encoding) && options.buffer[fdNumber]);
 

lib/terminate/kill.js

@@ -1,6 +1,6 @@
-import os from 'node:os';
 import {setTimeout} from 'node:timers/promises';
 import {isErrorInstance} from '../return/final-error.js';
+import {normalizeSignalArgument} from './signal.js';
 
 // Normalize the `forceKillAfterDelay` option
 export const normalizeForceKillAfterDelay = forceKillAfterDelay => {
@@ -46,15 +46,15 @@ const parseKillArguments = (signalOrError, errorArgument, killSignal) => {
 		? [undefined, signalOrError]
 		: [signalOrError, errorArgument];
 
-	if (typeof signal !== 'string' && typeof signal !== 'number') {
-		throw new TypeError(`The first argument must be an error instance or a signal name string/number: ${signal}`);
+	if (typeof signal !== 'string' && !Number.isInteger(signal)) {
+		throw new TypeError(`The first argument must be an error instance or a signal name string/integer: ${String(signal)}`);
 	}
 
 	if (error !== undefined && !isErrorInstance(error)) {
 		throw new TypeError(`The second argument is optional. If specified, it must be an error instance: ${error}`);
 	}
 
-	return {signal, error};
+	return {signal: normalizeSignalArgument(signal), error};
 };
 
 // Fails right away when calling `subprocess.kill(error)`.
@@ -77,11 +77,6 @@ const setKillTimeout = async ({kill, signal, forceKillAfterDelay, killSignal, ki
 	} catch {}
 };
 
-const shouldForceKill = (signal, forceKillAfterDelay, killSignal, killResult) =>
-	normalizeSignal(signal) === normalizeSignal(killSignal)
+const shouldForceKill = (signal, forceKillAfterDelay, killSignal, killResult) => signal === killSignal
 	&& forceKillAfterDelay !== false
 	&& killResult;
-
-const normalizeSignal = signal => typeof signal === 'string'
-	? os.constants.signals[signal.toUpperCase()]
-	: signal;

lib/terminate/signal.js

@@ -0,0 +1,66 @@
+import os from 'node:os';
+
+// Normalize signals for comparison purpose.
+// Also validate the signal exists.
+export const normalizeKillSignal = killSignal => {
+	const optionName = 'option `killSignal`';
+	if (killSignal === 0) {
+		throw new TypeError(`Invalid ${optionName}: 0 cannot be used.`);
+	}
+
+	return normalizeSignal(killSignal, optionName);
+};
+
+export const normalizeSignalArgument = signal => signal === 0
+	? signal
+	: normalizeSignal(signal, '`subprocess.kill()`\'s argument');
+
+const normalizeSignal = (signalNameOrInteger, optionName) => {
+	if (Number.isInteger(signalNameOrInteger)) {
+		return normalizeSignalInteger(signalNameOrInteger, optionName);
+	}
+
+	if (typeof signalNameOrInteger === 'string') {
+		return normalizeSignalName(signalNameOrInteger, optionName);
+	}
+
+	throw new TypeError(`Invalid ${optionName} ${String(signalNameOrInteger)}: it must be a string or an integer.\n${getAvailableSignals()}`);
+};
+
+const normalizeSignalInteger = (signalInteger, optionName) => {
+	if (signalsIntegerToName.has(signalInteger)) {
+		return signalsIntegerToName.get(signalInteger);
+	}
+
+	throw new TypeError(`Invalid ${optionName} ${signalInteger}: this signal integer does not exist.\n${getAvailableSignals()}`);
+};
+
+const getSignalsIntegerToName = () => new Map(Object.entries(os.constants.signals)
+	.reverse()
+	.map(([signalName, signalInteger]) => [signalInteger, signalName]));
+
+const signalsIntegerToName = getSignalsIntegerToName();
+
+const normalizeSignalName = (signalName, optionName) => {
+	if (signalName in os.constants.signals) {
+		return signalName;
+	}
+
+	if (signalName.toUpperCase() in os.constants.signals) {
+		throw new TypeError(`Invalid ${optionName} '${signalName}': please rename it to '${signalName.toUpperCase()}'.`);
+	}
+
+	throw new TypeError(`Invalid ${optionName} '${signalName}': this signal name does not exist.\n${getAvailableSignals()}`);
+};
+
+const getAvailableSignals = () => `Available signal names: ${getAvailableSignalNames()}.
+Available signal numbers: ${getAvailableSignalIntegers()}.`;
+
+const getAvailableSignalNames = () => Object.keys(os.constants.signals)
+	.sort()
+	.map(signalName => `'${signalName}'`)
+	.join(', ');
+
+const getAvailableSignalIntegers = () => [...new Set(Object.values(os.constants.signals)
+	.sort((signalInteger, signalIntegerTwo) => signalInteger - signalIntegerTwo))]
+	.join(', ');

package.json

@@ -52,7 +52,7 @@
 		"cross-spawn": "^7.0.3",
 		"figures": "^6.1.0",
 		"get-stream": "^9.0.0",
-		"human-signals": "^6.0.0",
+		"human-signals": "^7.0.0",
 		"is-plain-obj": "^4.1.0",
 		"is-stream": "^4.0.1",
 		"npm-run-path": "^5.2.0",

test-d/arguments/options.test-d.ts

@@ -135,6 +135,18 @@ await execa('unicorns', {killSignal: 9});
 execaSync('unicorns', {killSignal: 9});
 expectError(await execa('unicorns', {killSignal: false}));
 expectError(execaSync('unicorns', {killSignal: false}));
+expectError(await execa('unicorns', {killSignal: 'Sigterm'}));
+expectError(execaSync('unicorns', {killSignal: 'Sigterm'}));
+expectError(await execa('unicorns', {killSignal: 'sigterm'}));
+expectError(execaSync('unicorns', {killSignal: 'sigterm'}));
+expectError(await execa('unicorns', {killSignal: 'SIGOTHER'}));
+expectError(execaSync('unicorns', {killSignal: 'SIGOTHER'}));
+expectError(await execa('unicorns', {killSignal: 'SIGEMT'}));
+expectError(execaSync('unicorns', {killSignal: 'SIGEMT'}));
+expectError(await execa('unicorns', {killSignal: 'SIGCLD'}));
+expectError(execaSync('unicorns', {killSignal: 'SIGCLD'}));
+expectError(await execa('unicorns', {killSignal: 'SIGRT1'}));
+expectError(execaSync('unicorns', {killSignal: 'SIGRT1'}));
 
 await execa('unicorns', {forceKillAfterDelay: false});
 expectError(execaSync('unicorns', {forceKillAfterDelay: false}));

test-d/return/result-main.test-d.ts

@@ -28,7 +28,7 @@ expectType<boolean>(unicornsResult.timedOut);
 expectType<boolean>(unicornsResult.isCanceled);
 expectType<boolean>(unicornsResult.isTerminated);
 expectType<boolean>(unicornsResult.isMaxBuffer);
-expectType<string | undefined>(unicornsResult.signal);
+expectType<NodeJS.Signals | undefined>(unicornsResult.signal);
 expectType<string | undefined>(unicornsResult.signalDescription);
 expectType<string>(unicornsResult.cwd);
 expectType<number>(unicornsResult.durationMs);
@@ -44,7 +44,7 @@ expectType<boolean>(unicornsResultSync.timedOut);
 expectType<boolean>(unicornsResultSync.isCanceled);
 expectType<boolean>(unicornsResultSync.isTerminated);
 expectType<boolean>(unicornsResultSync.isMaxBuffer);
-expectType<string | undefined>(unicornsResultSync.signal);
+expectType<NodeJS.Signals | undefined>(unicornsResultSync.signal);
 expectType<string | undefined>(unicornsResultSync.signalDescription);
 expectType<string>(unicornsResultSync.cwd);
 expectType<number>(unicornsResultSync.durationMs);
@@ -61,7 +61,7 @@ if (error instanceof ExecaError) {
 	expectType<boolean>(error.isCanceled);
 	expectType<boolean>(error.isTerminated);
 	expectType<boolean>(error.isMaxBuffer);
-	expectType<string | undefined>(error.signal);
+	expectType<NodeJS.Signals | undefined>(error.signal);
 	expectType<string | undefined>(error.signalDescription);
 	expectType<string>(error.cwd);
 	expectType<number>(error.durationMs);
@@ -83,7 +83,7 @@ if (errorSync instanceof ExecaSyncError) {
 	expectType<boolean>(errorSync.isCanceled);
 	expectType<boolean>(errorSync.isTerminated);
 	expectType<boolean>(errorSync.isMaxBuffer);
-	expectType<string | undefined>(errorSync.signal);
+	expectType<NodeJS.Signals | undefined>(errorSync.signal);
 	expectType<string | undefined>(errorSync.signalDescription);
 	expectType<string>(errorSync.cwd);
 	expectType<number>(errorSync.durationMs);

test-d/subprocess/subprocess.test-d.ts

@@ -14,6 +14,12 @@ subprocess.kill('SIGKILL', new Error('test'));
 subprocess.kill(undefined, new Error('test'));
 expectError(subprocess.kill(null));
 expectError(subprocess.kill(0n));
+expectError(subprocess.kill('Sigkill'));
+expectError(subprocess.kill('sigkill'));
+expectError(subprocess.kill('SIGOTHER'));
+expectError(subprocess.kill('SIGEMT'));
+expectError(subprocess.kill('SIGCLD'));
+expectError(subprocess.kill('SIGRT1'));
 expectError(subprocess.kill([new Error('test')]));
 expectError(subprocess.kill({message: 'test'}));
 expectError(subprocess.kill(undefined, {}));

test/helpers/early-error.js

@@ -1,7 +1,9 @@
 import {execa, execaSync} from '../../index.js';
 
-export const earlyErrorOptions = {killSignal: false};
+export const earlyErrorOptions = {cancelSignal: false};
 export const getEarlyErrorSubprocess = options => execa('empty.js', {...earlyErrorOptions, ...options});
-export const getEarlyErrorSubprocessSync = options => execaSync('empty.js', {...earlyErrorOptions, ...options});
+export const earlyErrorOptionsSync = {maxBuffer: false};
+export const getEarlyErrorSubprocessSync = options => execaSync('empty.js', {...earlyErrorOptionsSync, ...options});
 
 export const expectedEarlyError = {code: 'ERR_INVALID_ARG_TYPE'};
+export const expectedEarlyErrorSync = {code: 'ERR_OUT_OF_RANGE'};

test/io/max-buffer.js

@@ -4,7 +4,7 @@ import getStream from 'get-stream';
 import {execa, execaSync} from '../../index.js';
 import {setFixtureDirectory} from '../helpers/fixtures-directory.js';
 import {fullStdio} from '../helpers/stdio.js';
-import {getEarlyErrorSubprocess, getEarlyErrorSubprocessSync} from '../helpers/early-error.js';
+import {getEarlyErrorSubprocess} from '../helpers/early-error.js';
 import {maxBuffer, assertErrorMessage} from '../helpers/max-buffer.js';
 
 setFixtureDirectory();
@@ -260,11 +260,9 @@ test('abort stream when hitting maxBuffer with stdout', testMaxBufferAbort, 1);
 test('abort stream when hitting maxBuffer with stderr', testMaxBufferAbort, 2);
 test('abort stream when hitting maxBuffer with stdio[*]', testMaxBufferAbort, 3);
 
-const testEarlyError = async (t, getSubprocess) => {
-	const {failed, isMaxBuffer} = await getSubprocess({reject: false, maxBuffer: 1});
+test('error.isMaxBuffer is false on early errors', async t => {
+	const {failed, isMaxBuffer} = await getEarlyErrorSubprocess({reject: false, maxBuffer: 1});
 	t.true(failed);
 	t.false(isMaxBuffer);
-};
+});
 
-test('error.isMaxBuffer is false on early errors', testEarlyError, getEarlyErrorSubprocess);
-test('error.isMaxBuffer is false on early errors, sync', testEarlyError, getEarlyErrorSubprocessSync);

test/return/early-error.js

@@ -8,6 +8,7 @@ import {
 	getEarlyErrorSubprocess,
 	getEarlyErrorSubprocessSync,
 	expectedEarlyError,
+	expectedEarlyErrorSync,
 } from '../helpers/early-error.js';
 
 setFixtureDirectory();
@@ -43,7 +44,7 @@ test('child_process.spawn() early errors are returned', async t => {
 });
 
 test('child_process.spawnSync() early errors are propagated with a correct shape', t => {
-	t.throws(getEarlyErrorSubprocessSync, expectedEarlyError);
+	t.throws(getEarlyErrorSubprocessSync, expectedEarlyErrorSync);
 });
 
 test('child_process.spawnSync() early errors are propagated with a correct shape - reject false', t => {

test/terminate/kill-force.js

@@ -90,7 +90,9 @@ if (isWindows) {
 	test('`forceKillAfterDelay: undefined` should kill after a timeout', testForceKill, undefined);
 	test('`forceKillAfterDelay` should kill after a timeout with SIGTERM', testForceKill, 50, 'SIGTERM');
 	test('`forceKillAfterDelay` should kill after a timeout with the killSignal string', testForceKill, 50, 'SIGINT', {killSignal: 'SIGINT'});
+	test('`forceKillAfterDelay` should kill after a timeout with the killSignal string, mixed', testForceKill, 50, 'SIGINT', {killSignal: constants.signals.SIGINT});
 	test('`forceKillAfterDelay` should kill after a timeout with the killSignal number', testForceKill, 50, constants.signals.SIGINT, {killSignal: constants.signals.SIGINT});
+	test('`forceKillAfterDelay` should kill after a timeout with the killSignal number, mixed', testForceKill, 50, constants.signals.SIGINT, {killSignal: 'SIGINT'});
 	test('`forceKillAfterDelay` should kill after a timeout with an error', testForceKill, 50, new Error('test'));
 	test('`forceKillAfterDelay` should kill after a timeout with an error and a killSignal', testForceKill, 50, new Error('test'), {killSignal: 'SIGINT'});
 

test/terminate/kill-signal.js

@@ -1,15 +1,60 @@
 import {once} from 'node:events';
 import {platform, version} from 'node:process';
+import {constants} from 'node:os';
 import {setImmediate} from 'node:timers/promises';
 import test from 'ava';
-import {execa} from '../../index.js';
+import {execa, execaSync} from '../../index.js';
 import {setFixtureDirectory} from '../helpers/fixtures-directory.js';
 
 setFixtureDirectory();
 
 const isWindows = platform === 'win32';
 const majorNodeVersion = Number(version.split('.')[0].slice(1));
 
+const testKillSignal = async (t, killSignal) => {
+	const {isTerminated, signal} = await t.throwsAsync(execa('forever.js', {killSignal, timeout: 1}));
+	t.true(isTerminated);
+	t.is(signal, 'SIGINT');
+};
+
+test('Can use killSignal: "SIGINT"', testKillSignal, 'SIGINT');
+test('Can use killSignal: 2', testKillSignal, constants.signals.SIGINT);
+
+const testKillSignalSync = (t, killSignal) => {
+	const {isTerminated, signal} = t.throws(() => {
+		execaSync('forever.js', {killSignal, timeout: 1});
+	});
+	t.true(isTerminated);
+	t.is(signal, 'SIGINT');
+};
+
+test('Can use killSignal: "SIGINT", sync', testKillSignalSync, 'SIGINT');
+test('Can use killSignal: 2, sync', testKillSignalSync, constants.signals.SIGINT);
+
+test('Can call .kill("SIGTERM")', async t => {
+	const subprocess = execa('forever.js');
+	subprocess.kill('SIGTERM');
+	const {isTerminated, signal} = await t.throwsAsync(subprocess);
+	t.true(isTerminated);
+	t.is(signal, 'SIGTERM');
+});
+
+test('Can call .kill(15)', async t => {
+	const subprocess = execa('forever.js');
+	subprocess.kill(constants.signals.SIGTERM);
+	const {isTerminated, signal} = await t.throwsAsync(subprocess);
+	t.true(isTerminated);
+	t.is(signal, 'SIGTERM');
+});
+
+test('Can call .kill(0)', async t => {
+	const subprocess = execa('forever.js');
+	t.true(subprocess.kill(0));
+	subprocess.kill();
+	await t.throwsAsync(subprocess);
+	t.false(subprocess.kill(0));
+});
+
 test('Can call `.kill()` multiple times', async t => {
 	const subprocess = execa('forever.js');
 	subprocess.kill();
@@ -50,9 +95,6 @@ const testInvalidKillArgument = async (t, killArgument, secondKillArgument) => {
 	await subprocess;
 };
 
-test('Cannot call .kill(null)', testInvalidKillArgument, null);
-test('Cannot call .kill(0n)', testInvalidKillArgument, 0n);
-test('Cannot call .kill(true)', testInvalidKillArgument, true);
 test('Cannot call .kill(errorObject)', testInvalidKillArgument, {name: '', message: '', stack: ''});
 test('Cannot call .kill(errorArray)', testInvalidKillArgument, [new Error('test')]);
 test('Cannot call .kill(undefined, true)', testInvalidKillArgument, undefined, true);