Improve cancelSignal option

lib/arguments/options.js

@@ -4,6 +4,7 @@ import crossSpawn from 'cross-spawn';
 import {npmRunPathEnv} from 'npm-run-path';
 import {normalizeForceKillAfterDelay} from '../terminate/kill.js';
 import {normalizeKillSignal} from '../terminate/signal.js';
+import {validateCancelSignal} from '../terminate/cancel.js';
 import {validateTimeout} from '../terminate/timeout.js';
 import {handleNodeOption} from '../methods/node.js';
 import {validateIpcInputOption} from '../ipc/ipc-input.js';
@@ -25,6 +26,7 @@ export const normalizeOptions = (filePath, rawArguments, rawOptions) => {
 	validateTimeout(options);
 	validateEncoding(options);
 	validateIpcInputOption(options);
+	validateCancelSignal(options);
 	options.shell = normalizeFileUrl(options.shell);
 	options.env = getEnv(options);
 	options.killSignal = normalizeKillSignal(options.killSignal);

lib/methods/main-async.js

@@ -71,12 +71,12 @@ const handleAsyncArguments = (rawFile, rawArguments, rawOptions) => {
 
 // Options normalization logic specific to async methods.
 // Prevent passing the `timeout` option directly to `child_process.spawn()`.
-const handleAsyncOptions = ({timeout, signal, cancelSignal, ...options}) => {
+const handleAsyncOptions = ({timeout, signal, ...options}) => {
 	if (signal !== undefined) {
 		throw new TypeError('The "signal" option has been renamed to "cancelSignal" instead.');
 	}
 
-	return {...options, timeoutDuration: timeout, signal: cancelSignal};
+	return {...options, timeoutDuration: timeout};
 };
 
 const spawnSubprocessAsync = ({file, commandArguments, options, startTime, verboseInfo, command, escapedCommand, fileDescriptors}) => {
@@ -130,7 +130,7 @@ const spawnSubprocessAsync = ({file, commandArguments, options, startTime, verbo
 
 // Asynchronous logic, as opposed to the previous logic which can be run synchronously, i.e. can be returned to user right away
 const handlePromise = async ({subprocess, options, startTime, verboseInfo, fileDescriptors, originalStreams, command, escapedCommand, onInternalError, controller}) => {
-	const context = {timedOut: false};
+	const context = {timedOut: false, isCanceled: false};
 
 	const [
 		errorInfo,
@@ -175,7 +175,7 @@ const getAsyncResult = ({errorInfo, exitCode, signal, stdio, all, ipcOutput, con
 		command,
 		escapedCommand,
 		timedOut: context.timedOut,
-		isCanceled: options.signal?.aborted === true,
+		isCanceled: context.isCanceled,
 		isMaxBuffer: errorInfo.error instanceof MaxBufferError,
 		exitCode,
 		signal,

lib/resolve/exit-async.js

@@ -2,7 +2,7 @@ import {once} from 'node:events';
 import {DiscardedError} from '../return/final-error.js';
 
 // If `error` is emitted before `spawn`, `exit` will never be emitted.
-// However, `error` might be emitted after `spawn`, e.g. with the `cancelSignal` option.
+// However, `error` might be emitted after `spawn`.
 // In that case, `exit` will still be emitted.
 // Since the `exit` event contains the signal name, we want to make sure we are listening for it.
 // This function also takes into account the following unlikely cases:

lib/resolve/wait-subprocess.js

@@ -1,6 +1,7 @@
 import {once} from 'node:events';
 import {isStream as isNodeStream} from 'is-stream';
 import {throwOnTimeout} from '../terminate/timeout.js';
+import {throwOnCancel} from '../terminate/cancel.js';
 import {isStandardStream} from '../utils/standard-stream.js';
 import {TRANSFORM_TYPES} from '../stdio/type.js';
 import {getBufferedData} from '../io/contents.js';
@@ -20,6 +21,7 @@ export const waitForSubprocessResult = async ({
 		maxBuffer,
 		lines,
 		timeoutDuration: timeout,
+		cancelSignal,
 		stripFinalNewline,
 		ipc,
 		ipcInput,
@@ -87,6 +89,7 @@ export const waitForSubprocessResult = async ({
 			onInternalError,
 			throwOnSubprocessError(subprocess, controller),
 			...throwOnTimeout(subprocess, timeout, context, controller),
+			...throwOnCancel(subprocess, cancelSignal, context, controller),
 		]);
 	} catch (error) {
 		return Promise.all([

lib/terminate/cancel.js

@@ -0,0 +1,26 @@
+import {once} from 'node:events';
+
+// Validate the `cancelSignal` option
+export const validateCancelSignal = ({cancelSignal}) => {
+	if (cancelSignal !== undefined && Object.prototype.toString.call(cancelSignal) !== '[object AbortSignal]') {
+		throw new Error(`The \`cancelSignal\` option must be an AbortSignal: ${String(cancelSignal)}`);
+	}
+};
+
+// Terminate the subprocess when aborting the `cancelSignal` option
+export const throwOnCancel = (subprocess, cancelSignal, context, controller) => cancelSignal === undefined
+	? []
+	: [terminateOnCancel(subprocess, cancelSignal, context, controller)];
+
+const terminateOnCancel = async (subprocess, cancelSignal, context, {signal}) => {
+	await onAbortedSignal(cancelSignal, signal);
+	context.isCanceled = true;
+	subprocess.kill();
+	throw cancelSignal.reason;
+};
+
+const onAbortedSignal = async (cancelSignal, signal) => {
+	if (!cancelSignal.aborted) {
+		await once(cancelSignal, 'abort', {signal});
+	}
+};

test/helpers/early-error.js

@@ -1,6 +1,6 @@
 import {execa, execaSync} from '../../index.js';
 
-export const earlyErrorOptions = {cancelSignal: false};
+export const earlyErrorOptions = {detached: 'true'};
 export const getEarlyErrorSubprocess = options => execa('empty.js', {...earlyErrorOptions, ...options});
 export const earlyErrorOptionsSync = {maxBuffer: false};
 export const getEarlyErrorSubprocessSync = options => execaSync('empty.js', {...earlyErrorOptionsSync, ...options});

test/return/final-error.js

@@ -99,12 +99,13 @@ test('error.cause is set on error event', async t => {
 test('error.cause is set if error.isCanceled', async t => {
 	const controller = new AbortController();
 	const subprocess = execa('forever.js', {cancelSignal: controller.signal});
-	const error = new Error('test');
-	controller.abort(error);
-	const {isCanceled, isTerminated, cause} = await t.throwsAsync(subprocess);
-	t.true(isCanceled);
-	t.false(isTerminated);
-	t.is(cause.cause, error);
+	const cause = new Error('test');
+	controller.abort(cause);
+	const error = await t.throwsAsync(subprocess);
+	t.true(error.isCanceled);
+	t.true(error.isTerminated);
+	t.is(error.signal, 'SIGTERM');
+	t.is(error.cause, cause);
 });
 
 test('error.cause is not set if error.isTerminated with .kill(error)', async t => {

test/terminate/cancel.js

@@ -1,10 +1,22 @@
-import {once} from 'node:events';
+import {once, getEventListeners} from 'node:events';
 import test from 'ava';
 import {execa, execaSync} from '../../index.js';
 import {setFixtureDirectory} from '../helpers/fixtures-directory.js';
+import {foobarString} from '../helpers/input.js';
 
 setFixtureDirectory();
 
+const testValidCancelSignal = (t, cancelSignal) => {
+	t.throws(() => {
+		execa('empty.js', {cancelSignal});
+	}, {message: /must be an AbortSignal/});
+};
+
+test('cancelSignal option cannot be AbortController', testValidCancelSignal, new AbortController());
+test('cancelSignal option cannot be {}', testValidCancelSignal, {});
+test('cancelSignal option cannot be null', testValidCancelSignal, null);
+test('cancelSignal option cannot be a symbol', testValidCancelSignal, Symbol('test'));
+
 test('result.isCanceled is false when abort isn\'t called (success)', async t => {
 	const {isCanceled} = await execa('noop.js');
 	t.false(isCanceled);
@@ -48,9 +60,59 @@ test('calling abort is considered a signal termination', async t => {
 	const subprocess = execa('forever.js', {cancelSignal: abortController.signal});
 	await once(subprocess, 'spawn');
 	abortController.abort();
-	const {isTerminated, signal} = await t.throwsAsync(subprocess);
+	const {isCanceled, isTerminated, signal} = await t.throwsAsync(subprocess);
+	t.true(isCanceled);
+	t.true(isTerminated);
+	t.is(signal, 'SIGTERM');
+});
+
+test('cancelSignal can already be aborted', async t => {
+	const cancelSignal = AbortSignal.abort();
+	const {isCanceled, isTerminated, signal} = await t.throwsAsync(execa('forever.js', {cancelSignal}));
+	t.true(isCanceled);
 	t.true(isTerminated);
 	t.is(signal, 'SIGTERM');
+	t.deepEqual(getEventListeners(cancelSignal, 'abort'), []);
+});
+
+test('calling abort does not emit the "error" event', async t => {
+	const abortController = new AbortController();
+	const subprocess = execa('forever.js', {cancelSignal: abortController.signal});
+	let error;
+	subprocess.once('error', errorArgument => {
+		error = errorArgument;
+	});
+	abortController.abort();
+	const {isCanceled} = await t.throwsAsync(subprocess);
+	t.true(isCanceled);
+	t.is(error, undefined);
+});
+
+test('calling abort cleans up listeners on cancelSignal, called', async t => {
+	const abortController = new AbortController();
+	const subprocess = execa('forever.js', {cancelSignal: abortController.signal});
+	t.is(getEventListeners(abortController.signal, 'abort').length, 1);
+	abortController.abort();
+	const {isCanceled} = await t.throwsAsync(subprocess);
+	t.true(isCanceled);
+	t.is(getEventListeners(abortController.signal, 'abort').length, 0);
+});
+
+test('calling abort cleans up listeners on cancelSignal, not called', async t => {
+	const abortController = new AbortController();
+	const subprocess = execa('noop.js', {cancelSignal: abortController.signal});
+	t.is(getEventListeners(abortController.signal, 'abort').length, 1);
+	await subprocess;
+	t.is(getEventListeners(abortController.signal, 'abort').length, 0);
+});
+
+test('calling abort cleans up listeners on cancelSignal, already aborted', async t => {
+	const cancelSignal = AbortSignal.abort();
+	const subprocess = execa('noop.js', {cancelSignal});
+	t.is(getEventListeners(cancelSignal, 'abort').length, 0);
+	const {isCanceled} = await t.throwsAsync(subprocess);
+	t.true(isCanceled);
+	t.is(getEventListeners(cancelSignal, 'abort').length, 0);
 });
 
 test('calling abort throws an error with message "Command was canceled"', async t => {
@@ -60,6 +122,43 @@ test('calling abort throws an error with message "Command was canceled"', async
 	await t.throwsAsync(subprocess, {message: /Command was canceled/});
 });
 
+test('calling abort with no argument keeps error properties', async t => {
+	const abortController = new AbortController();
+	const subprocess = execa('noop.js', {cancelSignal: abortController.signal});
+	abortController.abort();
+	const {cause, originalMessage, shortMessage, message} = await t.throwsAsync(subprocess);
+	t.is(cause.message, 'This operation was aborted');
+	t.is(cause.name, 'AbortError');
+	t.is(originalMessage, 'This operation was aborted');
+	t.is(shortMessage, 'Command was canceled: noop.js\nThis operation was aborted');
+	t.is(message, 'Command was canceled: noop.js\nThis operation was aborted');
+});
+
+test('calling abort with an error instance keeps error properties', async t => {
+	const abortController = new AbortController();
+	const subprocess = execa('noop.js', {cancelSignal: abortController.signal});
+	const error = new Error(foobarString);
+	error.code = foobarString;
+	abortController.abort(error);
+	const {cause, originalMessage, shortMessage, message, code} = await t.throwsAsync(subprocess);
+	t.is(cause, error);
+	t.is(originalMessage, foobarString);
+	t.is(shortMessage, `Command was canceled: noop.js\n${foobarString}`);
+	t.is(message, `Command was canceled: noop.js\n${foobarString}`);
+	t.is(code, foobarString);
+});
+
+test('calling abort with null keeps error properties', async t => {
+	const abortController = new AbortController();
+	const subprocess = execa('noop.js', {cancelSignal: abortController.signal});
+	abortController.abort(null);
+	const {cause, originalMessage, shortMessage, message} = await t.throwsAsync(subprocess);
+	t.is(cause, null);
+	t.is(originalMessage, 'null');
+	t.is(shortMessage, 'Command was canceled: noop.js\nnull');
+	t.is(message, 'Command was canceled: noop.js\nnull');
+});
+
 test('calling abort twice should show the same behaviour as calling it once', async t => {
 	const abortController = new AbortController();
 	const subprocess = execa('noop.js', {cancelSignal: abortController.signal});

test/terminate/kill-signal.js

@@ -134,6 +134,7 @@ test('subprocess double errors are handled after spawn', async t => {
 	subprocess.emit('error', cause);
 	await setImmediate();
 	abortController.abort();
+	subprocess.emit('error', cause);
 	const error = await t.throwsAsync(subprocess);
 	t.is(error.cause, cause);
 	t.is(error.exitCode, undefined);