From da259b60d89fb23d5b3e7e358957b08b7145bdd4 Mon Sep 17 00:00:00 2001
From: ehmicky <ehmicky@gmail.com>
Date: Sun, 25 Aug 2024 15:55:53 +0100
Subject: [PATCH] Improve stream error handling

---
 spawn.js | 39 ++++++++++++++++++++++++---------------
 test.js  | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/spawn.js b/spawn.js
index 53542b3..ac4e900 100644
--- a/spawn.js
+++ b/spawn.js
@@ -1,9 +1,8 @@
 import {spawn} from 'node:child_process';
-import {once} from 'node:events';
+import {once, on} from 'node:events';
 import {stripVTControlCharacters} from 'node:util';
 import path from 'node:path';
 import process from 'node:process';
-import {finished} from 'node:stream/promises';
 import {fileURLToPath} from 'node:url';
 import {lineIterator, combineAsyncIterators} from './iterable.js';
 import {getForcedShell, escapeArguments} from './windows.js';
@@ -124,16 +123,16 @@ const getResult = async (instancePromise, input, start, command) => {
 	const instance = await instancePromise;
 	useInput(instance, input);
 	const result = initResult();
-	const onExit = once(instance, 'close');
-	const onStdoutDone = bufferOutput(instance.stdout, result, 'stdout');
-	const onStderrDone = bufferOutput(instance.stderr, result, 'stderr');
+	const onClose = once(instance, 'close');
+	bufferOutput(instance.stdout, result, 'stdout');
+	bufferOutput(instance.stderr, result, 'stderr');
 
 	try {
-		await Promise.all([onExit, onStdoutDone, onStderrDone]);
+		await Promise.race([onClose, ...onStreamErrors(instance)]);
 		checkFailure(command, getErrorOutput(instance));
 		return getOutput(result, command, start);
 	} catch (error) {
-		await Promise.allSettled([onExit, onStdoutDone, onStderrDone]);
+		await Promise.allSettled([onClose]);
 		throw getResultError({
 			error,
 			result,
@@ -152,18 +151,28 @@ const useInput = (instance, input) => {
 
 const initResult = () => ({stdout: '', stderr: ''});
 
-const bufferOutput = async (stream, result, streamName) => {
-	if (!stream) {
-		return;
+const bufferOutput = (stream, result, streamName) => {
+	if (stream) {
+		stream.setEncoding('utf8');
+		stream.on('data', chunk => {
+			result[streamName] += chunk;
+		});
 	}
+};
 
-	stream.setEncoding('utf8');
-	stream.on('data', chunk => {
-		result[streamName] += chunk;
-	});
-	await finished(stream, {cleanup: true});
+const onStreamErrors = ({stdio}) => stdio.filter(Boolean).map(stream => onStreamError(stream));
+
+const onStreamError = async stream => {
+	for await (const [error] of on(stream, 'error')) {
+		if (!IGNORED_CODES.has(error?.code)) {
+			throw error;
+		}
+	}
 };
 
+// Ignore errors that are due to closing errors when the subprocesses exit normally, or due to piping
+const IGNORED_CODES = new Set(['ERR_STREAM_PREMATURE_CLOSE', 'EPIPE']);
+
 const getResultError = ({error, result, instance, start, command}) => Object.assign(
 	getErrorInstance(error, command),
 	getErrorOutput(instance),
diff --git a/test.js b/test.js
index 19b2c48..b1d1b78 100644
--- a/test.js
+++ b/test.js
@@ -216,6 +216,42 @@ if (isLinux) {
 	});
 }
 
+test('Error on stdin', async t => {
+	const error = new Error(testString);
+	const promise = nanoSpawn(...nodePrintStdout);
+	const subprocess = await promise.nodeChildProcess;
+	subprocess.stdin.destroy(error);
+	const {exitCode, signalName, message, cause} = await t.throwsAsync(promise);
+	t.is(exitCode, undefined);
+	t.is(signalName, undefined);
+	t.true(message.startsWith('Command failed: node -e'));
+	t.is(cause, error);
+});
+
+test('Error on stdout', async t => {
+	const error = new Error(testString);
+	const promise = nanoSpawn(...nodePrintStderr);
+	const subprocess = await promise.nodeChildProcess;
+	subprocess.stdout.destroy(error);
+	const {exitCode, signalName, message, cause} = await t.throwsAsync(promise);
+	t.is(exitCode, undefined);
+	t.is(signalName, undefined);
+	t.true(message.startsWith('Command failed: node -e'));
+	t.is(cause, error);
+});
+
+test('Error on stderr', async t => {
+	const error = new Error(testString);
+	const promise = nanoSpawn(...nodePrintStdout);
+	const subprocess = await promise.nodeChildProcess;
+	subprocess.stderr.destroy(error);
+	const {exitCode, signalName, message, cause} = await t.throwsAsync(promise);
+	t.is(exitCode, undefined);
+	t.is(signalName, undefined);
+	t.true(message.startsWith('Command failed: node -e'));
+	t.is(cause, error);
+});
+
 test('promise.stdout can be iterated', async t => {
 	const promise = nanoSpawn(...nodePrintStdout);
 	const lines = await arrayFromAsync(promise.stdout);