Repository: sindresorhus/execa
Pull request #518 (2023-02-11 00:55:47)

Emit `end` event on streams when process fails
Fixes https://github.com/sindresorhus/execa/issues/517

When a process (or one of its standard streams) errors, we return what could have been buffered so far from stdout/stderr as `error.stdout` and `error.stderr` (which are also appended to `error.message`). To do this, we first `destroy()` those streams, then retrieve the buffered value from `get-stream`.

However, calling `destroy()` prevents the `end` event from being emitted on those streams. This behavior differs from how `child_process.spawn()` and might be unexpected.

This PR fixes this for the case where the `buffer` option is `false`. When that's the case, there is no need to destroy the streams since we do not need to do anything with them anymore. We can just let Node.js destroy them naturally as part of the child process exit.


Patch:
diff --git a/index.d.ts b/index.d.ts
index 983b443..359541e 100644
--- a/index.d.ts
+++ b/index.d.ts
@@ -575,7 +575,7 @@ export function execaNode(
 export function execaNode(
 	scriptPath: string,
 	arguments?: readonly string[],
-	options?: Options<null>
+	options?: NodeOptions<null>
 ): ExecaChildProcess<Buffer>;
-export function execaNode(scriptPath: string, options?: Options): ExecaChildProcess;
-export function execaNode(scriptPath: string, options?: Options<null>): ExecaChildProcess<Buffer>;
+export function execaNode(scriptPath: string, options?: NodeOptions): ExecaChildProcess;
+export function execaNode(scriptPath: string, options?: NodeOptions<null>): ExecaChildProcess<Buffer>;
diff --git a/lib/stream.js b/lib/stream.js
index 3dcc81d..32bdefe 100644
--- a/lib/stream.js
+++ b/lib/stream.js
@@ -36,7 +36,8 @@ export const makeAllStream = (spawned, {all}) => {
 
 // On failure, `result.stdout|stderr|all` should contain the currently buffered stream
 const getBufferedData = async (stream, streamPromise) => {
-	if (!stream) {
+	// When `buffer` is `false`, `streamPromise` is `undefined` and there is no buffered data to retrieve
+	if (!stream || streamPromise === undefined) {
 		return;
 	}
 


Test patch:
diff --git a/index.test-d.ts b/index.test-d.ts
index fc00f2c..24cc93c 100644
--- a/index.test-d.ts
+++ b/index.test-d.ts
@@ -210,3 +210,12 @@ expectType<ExecaReturnValue>(
 expectType<ExecaReturnValue<Buffer>>(
 	await execaNode('unicorns', ['foo'], {encoding: null}),
 );
+
+expectType<ExecaChildProcess>(execaNode('unicorns', {nodeOptions: ['--async-stack-traces']}));
+expectType<ExecaChildProcess>(execaNode('unicorns', ['foo'], {nodeOptions: ['--async-stack-traces']}));
+expectType<ExecaChildProcess<Buffer>>(
+	execaNode('unicorns', {nodeOptions: ['--async-stack-traces'], encoding: null}),
+);
+expectType<ExecaChildProcess<Buffer>>(
+	execaNode('unicorns', ['foo'], {nodeOptions: ['--async-stack-traces'], encoding: null}),
+);
diff --git a/test/stream.js b/test/stream.js
index 32c3d0c..aeceedb 100644
--- a/test/stream.js
+++ b/test/stream.js
@@ -4,6 +4,7 @@ import fs from 'node:fs';
 import Stream from 'node:stream';
 import test from 'ava';
 import getStream from 'get-stream';
+import {pEvent} from 'p-event';
 import tempfile from 'tempfile';
 import {execa, execaSync} from '../index.js';
 import {setFixtureDir} from './helpers/fixtures-dir.js';
@@ -173,6 +174,11 @@ test('buffer: false > promise rejects when process returns non-zero', async t =>
 	t.is(exitCode, 2);
 });
 
+test('buffer: false > emits end event when promise is rejected', async t => {
+	const subprocess = execa('wrong command', {buffer: false, reject: false});
+	await t.notThrowsAsync(Promise.all([subprocess, pEvent(subprocess.stdout, 'end')]));
+});
+
 test('can use all: true with stdout: ignore', async t => {
 	await t.notThrowsAsync(execa('max-buffer.js', {buffer: false, stdout: 'ignore', all: true}));
 });