Fix endless loop (#20)

Author: SimonSiefke

index.js

@@ -4,7 +4,97 @@ import http2 from 'node:http2';
 const RETRY_DELAY = 200;
 const HTTP1_ATTEMPTS = 6; // 3 rounds × 2 IP versions
 
-export default function waitForLocalhost({port = 80, path = '/', useGet, statusCodes = [200], signal} = {}) {
+const tryHttp1 = ({
+	ipVersion,
+	onError,
+	method,
+	port,
+	path,
+	signal,
+	handleResponse,
+}) => {
+	const request = http.request(
+		{
+			method,
+			port,
+			path,
+			family: ipVersion,
+			signal,
+		},
+		response => {
+			handleResponse(response.statusCode, ipVersion, onError);
+		},
+	);
+
+	request.on('error', onError);
+	request.end();
+};
+
+const noop = () => {};
+
+const tryHttp2 = ({
+	ipVersion,
+	onError,
+	method,
+	port,
+	path,
+	signal,
+	handleResponse,
+}) => {
+	const hostname = ipVersion === 6 ? '[::1]' : 'localhost';
+	const client = http2.connect(`http://${hostname}:${port}`);
+
+	const cleanup = () => {
+		signal?.removeEventListener('abort', cleanup, {once: true});
+		client.off('error', handleClientError);
+		request.off('response', handleRequestResponse);
+		request.off('error', handleRequestError);
+		client.on('error', noop);
+		request.on('error', noop);
+		if (!client.destroyed) {
+			client.destroy();
+		}
+	};
+
+	// Cleanup on abort
+	signal?.addEventListener('abort', cleanup, {once: true});
+
+	const handleClientError = () => {
+		cleanup();
+		onError();
+	};
+
+	client.on('error', handleClientError);
+
+	const request = client.request({
+		':method': method,
+		':path': path,
+	});
+
+	const handleRequestResponse = headers => {
+		cleanup();
+		handleResponse(headers[':status'], ipVersion, onError);
+	};
+
+	request.on('response', handleRequestResponse);
+
+	const handleRequestError = () => {
+		cleanup();
+		onError();
+	};
+
+	request.on('error', handleRequestError);
+
+	request.end();
+};
+
+export default function waitForLocalhost({
+	port = 80,
+	path = '/',
+	useGet,
+	statusCodes = [200],
+	signal,
+} = {}) {
 	return new Promise((resolve, reject) => {
 		let attemptCount = 0;
 		const method = useGet ? 'GET' : 'HEAD';
@@ -21,10 +111,14 @@ export default function waitForLocalhost({port = 80, path = '/', useGet, statusC
 			}
 		};
 
-		signal?.addEventListener('abort', () => {
-			cleanup();
-			reject(signal.reason);
-		}, {once: true});
+		signal?.addEventListener(
+			'abort',
+			() => {
+				cleanup();
+				reject(signal.reason);
+			},
+			{once: true},
+		);
 
 		const handleResponse = (statusCode, ipVersion, onError) => {
 			if (statusCodes.includes(statusCode)) {
@@ -34,61 +128,18 @@ export default function waitForLocalhost({port = 80, path = '/', useGet, statusC
 			}
 		};
 
-		const tryHttp1 = (ipVersion, onError) => {
-			const request = http.request({
+		const tryRequest = (ipVersion, onError) => {
+			const useHttp2 = attemptCount > HTTP1_ATTEMPTS;
+			const requestFunction = useHttp2 ? tryHttp2 : tryHttp1;
+			requestFunction({
+				ipVersion,
+				onError,
 				method,
 				port,
 				path,
-				family: ipVersion,
 				signal,
-			}, response => {
-				handleResponse(response.statusCode, ipVersion, onError);
+				handleResponse,
 			});
-
-			request.on('error', onError);
-			request.end();
-		};
-
-		const tryHttp2 = (ipVersion, onError) => {
-			const hostname = ipVersion === 6 ? '[::1]' : 'localhost';
-			const client = http2.connect(`http://${hostname}:${port}`);
-
-			const cleanupClient = () => {
-				if (!client.destroyed) {
-					client.destroy();
-				}
-			};
-
-			// Cleanup on abort
-			signal?.addEventListener('abort', cleanupClient, {once: true});
-
-			client.on('error', () => {
-				cleanupClient();
-				onError();
-			});
-
-			const request = client.request({
-				':method': method,
-				':path': path,
-			});
-
-			request.on('response', headers => {
-				cleanupClient();
-				handleResponse(headers[':status'], ipVersion, onError);
-			});
-
-			request.on('error', () => {
-				cleanupClient();
-				onError();
-			});
-
-			request.end();
-		};
-
-		const tryRequest = (ipVersion, onError) => {
-			const useHttp2 = attemptCount > HTTP1_ATTEMPTS;
-			const requestFunction = useHttp2 ? tryHttp2 : tryHttp1;
-			requestFunction(ipVersion, onError);
 		};
 
 		const retry = () => {

test.js

@@ -184,3 +184,18 @@ test('should support AbortSignal.timeout()', async t => {
 		t.is(error.name, 'TimeoutError');
 	}
 });
+
+test('should not loop on error', async t => {
+	t.timeout(40_000);
+	try {
+		await waitForLocalhost({
+			port: 5879,
+			signal: AbortSignal.timeout(30_000),
+			path: '/json/list',
+			useGet: true,
+		});
+		t.fail('should have timeout out');
+	} catch (error) {
+		t.is(error.name, 'TimeoutError');
+	}
+});