Repository: sindresorhus/execa
Pull request #653 (2024-01-06 16:52:22)

Refactor execa promise to use `async/await`
This PR refactors the child process `exit`/`error` events logic to use `async`/`await`. This does not change the behavior.


Patch:
diff --git a/lib/promise.js b/lib/promise.js
index a4773f3..73b0171 100644
--- a/lib/promise.js
+++ b/lib/promise.js
@@ -1,3 +1,5 @@
+import {once} from 'node:events';
+
 // eslint-disable-next-line unicorn/prefer-top-level-await
 const nativePromisePrototype = (async () => {})().constructor.prototype;
 
@@ -19,18 +21,7 @@ export const mergePromise = (spawned, promise) => {
 };
 
 // Use promises instead of `child_process` events
-export const getSpawnedPromise = spawned => new Promise((resolve, reject) => {
-	spawned.on('exit', (exitCode, signal) => {
-		resolve({exitCode, signal});
-	});
-
-	spawned.on('error', error => {
-		reject(error);
-	});
-
-	if (spawned.stdin) {
-		spawned.stdin.on('error', error => {
-			reject(error);
-		});
-	}
-});
+export const getSpawnedPromise = async spawned => {
+	const [exitCode, signal] = await once(spawned, 'exit');
+	return {exitCode, signal};
+};
diff --git a/lib/stream.js b/lib/stream.js
index 7c571be..4871a61 100644
--- a/lib/stream.js
+++ b/lib/stream.js
@@ -1,7 +1,8 @@
+import {Buffer} from 'node:buffer';
 import {once} from 'node:events';
 import {finished} from 'node:stream/promises';
 import process from 'node:process';
-import getStream, {getStreamAsBuffer, getStreamAsArrayBuffer} from 'get-stream';
+import getStream, {getStreamAsArrayBuffer} from 'get-stream';
 import mergeStreams from '@sindresorhus/merge-streams';
 
 // `all` interleaves `stdout` and `stderr`
@@ -24,40 +25,40 @@ export const makeAllStream = (spawned, {all}) => {
 // On failure, `result.stdout|stderr|all` should contain the currently buffered stream
 // They are automatically closed and flushed by Node.js when the child process exits
 // We guarantee this by calling `childProcess.kill()`
-const getBufferedData = async (stream, streamPromise) => {
-	// When `buffer` is `false`, `streamPromise` is `undefined` and there is no buffered data to retrieve
-	if (!stream || streamPromise === undefined) {
-		return;
-	}
-
+// When `buffer` is `false`, `streamPromise` is `undefined` and there is no buffered data to retrieve
+const getBufferedData = async (streamPromise, encoding) => {
 	try {
 		return await streamPromise;
 	} catch (error) {
-		return error.bufferedData;
+		return error.bufferedData === undefined ? undefined : applyEncoding(error.bufferedData, encoding);
 	}
 };
 
-const getStreamPromise = (stream, {encoding, buffer, maxBuffer}) => {
+const getStreamPromise = async (stream, {encoding, buffer, maxBuffer}) => {
 	if (!stream || !buffer) {
 		return;
 	}
 
-	// eslint-disable-next-line unicorn/text-encoding-identifier-case
-	if (encoding === 'utf8' || encoding === 'utf-8') {
-		return getStream(stream, {maxBuffer});
+	const contents = isUtf8Encoding(encoding)
+		? await getStream(stream, {maxBuffer})
+		: await getStreamAsArrayBuffer(stream, {maxBuffer});
+	return applyEncoding(contents, encoding);
+};
+
+const applyEncoding = (contents, encoding) => {
+	if (isUtf8Encoding(encoding)) {
+		return contents;
 	}
 
 	if (encoding === 'buffer') {
-		return getStreamAsArrayBuffer(stream, {maxBuffer}).then(arrayBuffer => new Uint8Array(arrayBuffer));
+		return new Uint8Array(contents);
 	}
 
-	return applyEncoding(stream, maxBuffer, encoding);
+	return Buffer.from(contents).toString(encoding);
 };
 
-const applyEncoding = async (stream, maxBuffer, encoding) => {
-	const buffer = await getStreamAsBuffer(stream, {maxBuffer});
-	return buffer.toString(encoding);
-};
+// eslint-disable-next-line unicorn/text-encoding-identifier-case
+const isUtf8Encoding = encoding => encoding === 'utf8' || encoding === 'utf-8';
 
 // Some `stdout`/`stderr` options create a stream, e.g. when passing a file path.
 // The `.pipe()` method automatically ends that stream when `childProcess.stdout|stderr` ends.
@@ -77,6 +78,10 @@ const waitForStreamEnd = ({value, type, direction}, processDone) => {
 		: Promise.race([processDone, throwOnStreamError(value)]);
 };
 
+const throwIfStreamError = (stream, processDone) => stream === null
+	? undefined
+	: Promise.race([processDone, throwOnStreamError(stream)]);
+
 const throwOnStreamError = async stream => {
 	const [error] = await once(stream, 'error');
 	throw error;
@@ -110,15 +115,16 @@ export const getSpawnedResult = async (
 			stdoutPromise,
 			stderrPromise,
 			allPromise,
+			throwIfStreamError(spawned.stdin, processDone),
 			...stdioStreams.map(stdioStream => waitForStreamEnd(stdioStream, processDone)),
 		]);
 	} catch (error) {
 		spawned.kill();
 		const results = await Promise.all([
 			{error, signal: error.signal, timedOut: error.timedOut},
-			getBufferedData(spawned.stdout, stdoutPromise),
-			getBufferedData(spawned.stderr, stderrPromise),
-			getBufferedData(spawned.all, allPromise),
+			getBufferedData(stdoutPromise, encoding),
+			getBufferedData(stderrPromise, encoding),
+			getBufferedData(allPromise, encoding),
 		]);
 		cleanupStdioStreams(stdioStreams, error);
 		return results;


Test patch:
diff --git a/test/stream.js b/test/stream.js
index 852c4e6..cb8370d 100644
--- a/test/stream.js
+++ b/test/stream.js
@@ -102,6 +102,21 @@ const testMaxBuffer = async (t, streamName) => {
 test('maxBuffer affects stdout', testMaxBuffer, 'stdout');
 test('maxBuffer affects stderr', testMaxBuffer, 'stderr');
 
+test('maxBuffer works with encoding buffer', async t => {
+	const {stdout} = await t.throwsAsync(
+		execa('max-buffer.js', ['stdout', '11'], {maxBuffer: 10, encoding: 'buffer'}),
+	);
+	t.true(stdout instanceof Uint8Array);
+	t.is(Buffer.from(stdout).toString(), '.'.repeat(10));
+});
+
+test('maxBuffer works with other encodings', async t => {
+	const {stdout} = await t.throwsAsync(
+		execa('max-buffer.js', ['stdout', '11'], {maxBuffer: 10, encoding: 'hex'}),
+	);
+	t.is(stdout, Buffer.from('.'.repeat(10)).toString('hex'));
+});
+
 const testNoMaxBuffer = async (t, streamName) => {
 	const promise = execa('max-buffer.js', [streamName, '10'], {buffer: false});
 	const [result, output] = await Promise.all([
diff --git a/test/test.js b/test/test.js
index 0693099..995b9dd 100644
--- a/test/test.js
+++ b/test/test.js
@@ -1,5 +1,6 @@
 import path from 'node:path';
 import process from 'node:process';
+import {setTimeout} from 'node:timers/promises';
 import {fileURLToPath, pathToFileURL} from 'node:url';
 import test from 'ava';
 import isRunning from 'is-running';
@@ -124,10 +125,17 @@ const testExecPath = async (t, mapPath) => {
 test.serial('execPath option', testExecPath, identity);
 test.serial('execPath option can be a file URL', testExecPath, pathToFileURL);
 
-test('stdin errors are handled', async t => {
-	const subprocess = execa('noop.js');
+const emitStdinError = async subprocess => {
+	await setTimeout(0);
 	subprocess.stdin.emit('error', new Error('test'));
-	await t.throwsAsync(subprocess, {message: /test/});
+};
+
+test('stdin errors are handled', async t => {
+	const subprocess = execa('forever.js');
+	await Promise.all([
+		t.throwsAsync(subprocess, {message: /test/}),
+		emitStdinError(subprocess),
+	]);
 });
 
 test('child process errors are handled', async t => {