cacheable-request - fix: keyv error listener leak on every request

packages/cacheable-request/src/index.ts

@@ -315,12 +315,29 @@ class CacheableRequest {
 					ee.emit("error", new CacheError(error));
 				if (this.cache instanceof Keyv) {
 					const cachek = this.cache;
-					cachek.once("error", errorHandler);
-					ee.on("error", () => {
-						cachek.removeListener("error", errorHandler);
-					});
-					ee.on("response", () => {
+					// Keyv's event manager wraps once() listeners, so removeListener()
+					// with the original function would never unregister them. Register
+					// with on() and remove the same reference on every termination path
+					// so the listener count stays bounded.
+					let pendingRequest: any;
+					const removeErrorHandler = () => {
 						cachek.removeListener("error", errorHandler);
+						pendingRequest?.removeListener("error", removeErrorHandler);
+						pendingRequest?.removeListener("abort", removeErrorHandler);
+						pendingRequest?.removeListener("close", removeErrorHandler);
+						pendingRequest = undefined;
+					};
+
+					cachek.on("error", errorHandler);
+					ee.on("error", removeErrorHandler);
+					ee.on("response", removeErrorHandler);
+					// Aborted or destroyed requests never emit "response", so clean up
+					// on the underlying request's termination events as well.
+					ee.on("request", (request_: any) => {
+						pendingRequest = request_;
+						request_.once("error", removeErrorHandler);
+						request_.once("abort", removeErrorHandler);
+						request_.once("close", removeErrorHandler);
 					});
 				}
 

packages/cacheable-request/test/cacheable-request-instance.test.ts

@@ -451,3 +451,55 @@ test("cacheableRequest emits CacheError when underlying Keyv emits error event",
 		});
 	});
 });
+
+test("cacheableRequest removes Keyv error listener once the request settles", async () => {
+	const keyv = new Keyv();
+	const cacheableRequest = new CacheableRequest(request, keyv).request();
+	const baseline = keyv.listeners("error").length;
+
+	const get = async (path: string) =>
+		new Promise<void>((resolve, reject) => {
+			cacheableRequest(parseWithWhatwg(`${s.url}${path}`), (response: any) => {
+				response.resume();
+				response.once("end", resolve);
+			})
+				.on("error", reject)
+				.on("request", (request_: any) => request_.end());
+		});
+
+	// Repeated keys are served from cache while fresh keys hit the network;
+	// neither path may leave its error listener behind (issue #1685)
+	for (let index = 0; index < 6; index++) {
+		await get(`/?index=${index % 3}`);
+	}
+
+	await new Promise((resolve) => {
+		setImmediate(resolve);
+	});
+	expect(keyv.listeners("error").length).toBe(baseline);
+});
+
+test("cacheableRequest removes Keyv error listener when the request is aborted", async () => {
+	const keyv = new Keyv();
+	const cacheableRequest = new CacheableRequest(request, keyv).request();
+	const baseline = keyv.listeners("error").length;
+
+	await new Promise<void>((resolve) => {
+		cacheableRequest(parseWithWhatwg(s.url))
+			.on("error", () => {
+				/* swallow abort-induced errors */
+			})
+			.on("request", (request_: any) => {
+				request_.on("error", () => {
+					/* swallow abort-induced socket errors */
+				});
+				request_.once("close", resolve);
+				request_.abort();
+			});
+	});
+
+	await new Promise((resolve) => {
+		setImmediate(resolve);
+	});
+	expect(keyv.listeners("error").length).toBe(baseline);
+});