Repository: sindresorhus/execa
Pull request #652 (2024-01-06 16:16:31)

Fix `error.stdout|stderr|all` type when `encoding` option is used
Right now, the `encoding` option is not correctly applied to `error.stdout|stderr|all`. Namely:
  - When `encoding` is `buffer`, `error.stdout|stderr|all` is an `ArrayBuffer` instead of an `Uint8Array`
  - When `encoding` is something else like `hex`, `error.stdout|stderr|all` is a Node.js `Buffer` instead of a string
  - In both cases, our output normalization logic eventually transforms this to `undefined`

This is because the encoding logic is applied after `get-stream` has succeeded, but it is not applied on `error.bufferedData`, i.e. when `get-stream` has failed or been aborted.

This PR fixes this.


Patch:
diff --git a/lib/stream.js b/lib/stream.js
index 7c571be..a41b324 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.
@@ -116,9 +117,9 @@ export const getSpawnedResult = async (
 		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([