[http-proxy-agent] Fix keepAlive: true (#190)

Based on #169.

Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com>

.changeset/four-carrots-help.md

@@ -0,0 +1,5 @@
+---
+'http-proxy-agent': patch
+---
+
+Fix `keepAlive: true`

packages/http-proxy-agent/src/index.ts

@@ -77,12 +77,18 @@ export class HttpProxyAgent<Uri extends string> extends Agent {
 		};
 	}
 
-	async connect(
+	addRequest(req: HttpProxyAgentClientRequest, opts: AgentConnectOpts): void {
+		req._header = null;
+		this.setRequestProps(req, opts);
+		// @ts-expect-error `addRequest()` isn't defined in `@types/node`
+		super.addRequest(req, opts);
+	}
+
+	setRequestProps(
 		req: HttpProxyAgentClientRequest,
 		opts: AgentConnectOpts
-	): Promise<net.Socket> {
+	): void {
 		const { proxy } = this;
-
 		const protocol = opts.secureEndpoint ? 'https:' : 'http:';
 		const hostname = req.getHeader('host') || 'localhost';
 		const base = `${protocol}//${hostname}`;
@@ -96,7 +102,7 @@ export class HttpProxyAgent<Uri extends string> extends Agent {
 		req.path = String(url);
 
 		// Inject the `Proxy-Authorization` header if necessary.
-		req._header = null;
+
 		const headers: OutgoingHttpHeaders =
 			typeof this.proxyHeaders === 'function'
 				? this.proxyHeaders()
@@ -121,15 +127,16 @@ export class HttpProxyAgent<Uri extends string> extends Agent {
 				req.setHeader(name, value);
 			}
 		}
+	}
 
-		// Create a socket connection to the proxy server.
-		let socket: net.Socket;
-		if (this.secureProxy) {
-			debug('Creating `tls.Socket`: %o', this.connectOpts);
-			socket = tls.connect(this.connectOpts);
-		} else {
-			debug('Creating `net.Socket`: %o', this.connectOpts);
-			socket = net.connect(this.connectOpts);
+	async connect(
+		req: HttpProxyAgentClientRequest,
+		opts: AgentConnectOpts
+	): Promise<net.Socket> {
+		req._header = null;
+
+		if (!req.path.includes('://')) {
+			this.setRequestProps(req, opts);
 		}
 
 		// At this point, the http ClientRequest's internal `_header` field
@@ -140,7 +147,6 @@ export class HttpProxyAgent<Uri extends string> extends Agent {
 		debug('Regenerating stored HTTP header string for request');
 		req._implicitHeader();
 		if (req.outputData && req.outputData.length > 0) {
-			// Node >= 12
 			debug(
 				'Patching connection write() output buffer with updated header'
 			);
@@ -151,6 +157,16 @@ export class HttpProxyAgent<Uri extends string> extends Agent {
 			debug('Output buffer: %o', req.outputData[0].data);
 		}
 
+		// Create a socket connection to the proxy server.
+		let socket: net.Socket;
+		if (this.secureProxy) {
+			debug('Creating `tls.Socket`: %o', this.connectOpts);
+			socket = tls.connect(this.connectOpts);
+		} else {
+			debug('Creating `net.Socket`: %o', this.connectOpts);
+			socket = net.connect(this.connectOpts);
+		}
+
 		// Wait for the socket's `connect` event, so that this `callback()`
 		// function throws instead of the `http` request machinery. This is
 		// important for i.e. `PacProxyAgent` which determines a failed proxy

packages/http-proxy-agent/test/test.ts

@@ -2,6 +2,7 @@ import * as fs from 'fs';
 import * as http from 'http';
 import * as https from 'https';
 import assert from 'assert';
+import { once } from 'events';
 import { createProxy, ProxyServer } from 'proxy';
 import { listen } from 'async-listen';
 import { json, req } from 'agent-base';
@@ -219,5 +220,32 @@ describe('HttpProxyAgent', () => {
 			const body = await json(res);
 			expect(body.host).toEqual(httpServerUrl.hostname);
 		});
+
+		it('should work with `keepAlive: true`', async () => {
+			httpServer.on('request', (req, res) => {
+				res.end(JSON.stringify(req.headers));
+			});
+
+			const agent = new HttpProxyAgent(proxyUrl, { keepAlive: true });
+
+			try {
+				const res = await req(httpServerUrl, { agent });
+				expect(res.headers.connection).toEqual('keep-alive');
+				expect(res.statusCode).toEqual(200);
+				res.resume();
+				const s1 = res.socket;
+				await once(s1, 'free');
+
+				const res2 = await req(httpServerUrl, { agent });
+				expect(res2.headers.connection).toEqual('keep-alive');
+				expect(res2.statusCode).toEqual(200);
+				res2.resume();
+				const s2 = res2.socket;
+				assert(s1 === s2);
+				await once(s2, 'free');
+			} finally {
+				agent.destroy();
+			}
+		});
 	});
 });