Repository: sindresorhus/execa (js)

Issue:
Make it cancelable
Using: https://github.com/sindresorhus/p-cancelable

It's possible to call `cp.kill()`, but would be nicer if it was cancelable.

Pull request:
Make the spawned process cancelable
see if this is okay.
I was wondering whether there should we a check if oncancel property in options object passed by someone is actually a function. is this required?


Fixes #113

Patch:
diff --git a/index.js b/index.js
index d7c6058..f5a9ea8 100644
--- a/index.js
+++ b/index.js
@@ -143,7 +143,7 @@ function getStream(process, stream, {encoding, buffer, maxBuffer}) {
 function makeError(result, options) {
 	const {stdout, stderr, code, signal} = result;
 	let {error} = result;
-	const {joinedCommand, timedOut, parsed: {options: {timeout}}} = options;
+	const {joinedCommand, timedOut, isCanceled, parsed: {options: {timeout}}} = options;
 
 	const [exitCodeName, exitCode] = getCode(result, code);
 
@@ -152,7 +152,7 @@ function makeError(result, options) {
 		error = new Error(message);
 	}
 
-	const prefix = getErrorPrefix({timedOut, timeout, signal, exitCodeName, exitCode});
+	const prefix = getErrorPrefix({timedOut, timeout, signal, exitCodeName, exitCode, isCanceled});
 	error.message = `Command ${prefix}: ${error.message}`;
 
 	error.code = exitCode || exitCodeName;
@@ -166,6 +166,7 @@ function makeError(result, options) {
 	error.signal = signal || undefined;
 	error.cmd = joinedCommand;
 	error.timedOut = Boolean(timedOut);
+	error.isCanceled = isCanceled;
 
 	if ('all' in result) {
 		error.all = result.all;
@@ -186,11 +187,15 @@ function getCode({error = {}}, code) {
 	return [];
 }
 
-function getErrorPrefix({timedOut, timeout, signal, exitCodeName, exitCode}) {
+function getErrorPrefix({timedOut, timeout, signal, exitCodeName, exitCode, isCanceled}) {
 	if (timedOut) {
 		return `timed out after ${timeout} milliseconds`;
 	}
 
+	if (isCanceled) {
+		return 'was canceled';
+	}
+
 	if (signal) {
 		return `was killed with ${signal}`;
 	}
@@ -241,6 +246,7 @@ module.exports = (command, args, options) => {
 
 	let timeoutId;
 	let timedOut = false;
+	let isCanceled = false;
 
 	const cleanup = () => {
 		if (timeoutId !== undefined) {
@@ -306,11 +312,12 @@ module.exports = (command, args, options) => {
 		result.stderr = results[2];
 		result.all = results[3];
 
-		if (result.error || result.code !== 0 || result.signal !== null) {
+		if (result.error || result.code !== 0 || result.signal !== null || isCanceled) {
 			const error = makeError(result, {
 				joinedCommand,
 				parsed,
-				timedOut
+				timedOut,
+				isCanceled
 			});
 
 			// TODO: missing some timeout logic for killed
@@ -335,7 +342,8 @@ module.exports = (command, args, options) => {
 			failed: false,
 			killed: false,
 			cmd: joinedCommand,
-			timedOut: false
+			timedOut: false,
+			isCanceled: false
 		};
 	}), destroy);
 
@@ -348,6 +356,15 @@ module.exports = (command, args, options) => {
 	// eslint-disable-next-line promise/prefer-await-to-then
 	spawned.then = (onFulfilled, onRejected) => handlePromise().then(onFulfilled, onRejected);
 	spawned.catch = onRejected => handlePromise().catch(onRejected);
+	spawned.cancel = () => {
+		if (spawned.killed) {
+			return;
+		}
+
+		if (spawned.kill()) {
+			isCanceled = true;
+		}
+	};
 
 	// TOOD: Remove the `if`-guard when targeting Node.js 10
 	if (Promise.prototype.finally) {
diff --git a/readme.md b/readme.md
index 26cbdbc..2ffaecb 100644
--- a/readme.md
+++ b/readme.md
@@ -52,6 +52,15 @@ const execa = require('execa');
 	const {stdout} = await execa.shell('echo unicorns');
 	//=> 'unicorns'
 
+	// Cancelling a spawned process
+	const subprocess = execa('node');
+	setTimeout(() => { spawned.cancel() }, 1000);
+	try {
+		await subprocess;
+	} catch (error) {
+		console.log(subprocess.killed); // true
+		console.log(error.isCanceled); // true
+	}
 
 	// Catching an error
 	try {
@@ -110,6 +119,9 @@ Returns a [`child_process` instance](https://nodejs.org/api/child_process.html#c
 
 It exposes an additional `.all` stream, with `stdout` and `stderr` interleaved.
 
+It can be canceled with `.cancel` method which throws an error with `error.isCanceled` equal to `true`, provided that the process gets canceled.
+Process would not get canceled if it has already exited.
+
 The promise result is an `Object` with `stdout`, `stderr` and `all` properties.
 
 ### execa.stdout(file, [arguments], [options])


Test patch:
diff --git a/test.js b/test.js
index 1265c65..38f66e6 100644
--- a/test.js
+++ b/test.js
@@ -586,3 +586,57 @@ if (Promise.prototype.finally) {
 		t.is(result.message, 'called');
 	});
 }
+
+test('cancel method kills the spawned process', t => {
+	const spawned = execa('node');
+	spawned.cancel();
+	t.true(spawned.killed);
+});
+
+test('result.isCanceled is false when spawned.cancel isn\'t called', async t => {
+	const result = await execa('noop');
+	t.false(result.isCanceled);
+});
+
+test('calling cancel method throws an error with message "Command was canceled"', async t => {
+	const spawned = execa('noop');
+	spawned.cancel();
+	await t.throwsAsync(spawned, {message: /Command was canceled/});
+});
+
+test('error.isCanceled is true when cancel method is used', async t => {
+	const spawned = execa('noop');
+	spawned.cancel();
+	const error = await t.throwsAsync(spawned);
+	t.true(error.isCanceled);
+});
+
+test('error.isCanceled is false when kill method is used', async t => {
+	const spawned = execa('noop');
+	spawned.kill();
+	const error = await t.throwsAsync(spawned);
+	t.false(error.isCanceled);
+});
+
+test('calling cancel method twice should show the same behaviour as calling it once', async t => {
+	const spawned = execa('noop');
+	spawned.cancel();
+	spawned.cancel();
+	const error = await t.throwsAsync(spawned);
+	t.true(error.isCanceled);
+	t.true(spawned.killed);
+});
+
+test('calling cancel method on a successfuly completed process does not make result.cancel true', async t => {
+	const spawned = execa('noop');
+	const result = await spawned;
+	spawned.cancel();
+	t.false(result.isCanceled);
+});
+
+test('calling cancel method on a process which has been killed does not make error.isCanceled true', async t => {
+	const spawned = execa('noop');
+	spawned.kill();
+	const error = await t.throwsAsync(spawned);
+	t.false(error.isCanceled);
+});