Fix hang on stream request cleanup in Node.js (#741)

whddltjdwhd · sholladay · web-flow · commit 9a89a35ec3a1 · 2025-09-27T21:06:45.000+09:00
Co-authored-by: Seth Holladay &lt;me@seth-holladay.com&gt;
diff --git a/package.json b/package.json
@@ -67,7 +67,7 @@
 		"delay": "^6.0.0",
 		"expect-type": "^0.19.0",
 		"express": "^4.18.2",
-		"jest-leak-detector": "^29.7.0",
+		"jest-leak-detector": "^30.1.0",
 		"pify": "^6.1.0",
 		"playwright": "^1.45.3",
 		"raw-body": "^2.5.2",
diff --git a/source/core/Ky.ts b/source/core/Ky.ts
@@ -86,10 +86,18 @@ export class Ky {
 		const isRetriableMethod = ky._options.retry.methods.includes(ky.request.method.toLowerCase());
 		const result = (isRetriableMethod ? ky._retry(function_) : function_())
 			.finally(async () => {
-				// Now that we know a retry is not needed, close the ReadableStream of the cloned request.
+				const originalRequest = ky._originalRequest;
+				const cleanupPromises = [];
+
+				if (originalRequest && !originalRequest.bodyUsed) {
+					cleanupPromises.push(originalRequest.body?.cancel());
+				}
+
 				if (!ky.request.bodyUsed) {
-					await ky.request.body?.cancel();
+					cleanupPromises.push(ky.request.body?.cancel());
 				}
+
+				await Promise.all(cleanupPromises);
 			}) as ResponsePromise;
 
 		for (const [type, mimeType] of Object.entries(responseTypes) as ObjectEntries<typeof responseTypes>) {
@@ -148,6 +156,7 @@ export class Ky {
 	protected _retryCount = 0;
 	protected _input: Input;
 	protected _options: InternalOptions;
+	protected _originalRequest?: Request;
 
 	// eslint-disable-next-line complexity
 	constructor(input: Input, options: Options = {}) {
@@ -341,13 +350,13 @@ export class Ky {
 		const nonRequestOptions = findUnknownOptions(this.request, this._options);
 
 		// Cloning is done here to prepare in advance for retries
-		const mainRequest = this.request;
-		this.request = mainRequest.clone();
+		this._originalRequest = this.request;
+		this.request = this._originalRequest.clone();
 
 		if (this._options.timeout === false) {
-			return this._options.fetch(mainRequest, nonRequestOptions);
+			return this._options.fetch(this._originalRequest, nonRequestOptions);
 		}
 
-		return timeout(mainRequest, nonRequestOptions, this.abortController, this._options as TimeoutOptions);
+		return timeout(this._originalRequest, nonRequestOptions, this.abortController, this._options as TimeoutOptions);
 	}
 }
diff --git a/test/memory-leak.ts b/test/memory-leak.ts
@@ -1,10 +1,9 @@
+import {ReadableStream} from 'node:stream/web';
 import test from 'ava';
-import _LeakDetector from 'jest-leak-detector';
+import LeakDetector from 'jest-leak-detector';
 import ky, {type KyInstance} from '../source/index.js';
 import {createHttpTestServer} from './helpers/create-http-test-server.js';
 
-const LeakDetector = _LeakDetector.default as typeof _LeakDetector;
-
 test('shared abort signal must not cause memory leak of input', async t => {
 	const server = await createHttpTestServer();
 	server.get('/', (_request, response) => {
@@ -34,3 +33,24 @@ test('shared abort signal must not cause memory leak of input', async t => {
 		await server.close();
 	}
 });
+
+test('failed stream request must not cause memory leak', async t => {
+	async function isStreamLeaking() {
+		const stream: ReadableStream | undefined = ReadableStream.from([new TextEncoder().encode('Bell is Ringing.')]);
+		const detector = new LeakDetector(stream);
+
+		await t.throwsAsync(
+			ky.post('invalid:', {
+				body: stream,
+			}),
+			{
+				instanceOf: TypeError,
+				message: 'fetch failed',
+			},
+		);
+
+		return detector.isLeaking();
+	}
+
+	t.false(await isStreamLeaking());
+});
