Merge pull request #258 from thomasbecker/fix/connection-timeout-issue-228

fix: use true connection timeout instead of idle timeout

packages/safe-chain/src/registryProxy/registryProxy.connect-tunnel.spec.js

@@ -182,13 +182,13 @@ describe("registryProxy.connectTunnel", () => {
 
       const duration = Date.now() - startTime;
 
-      // Should return 502 Bad Gateway
+      // Should return 504 Gateway Timeout (not 502 - 504 is for actual timeouts)
       assert.ok(
-        responseData.includes("HTTP/1.1 502 Bad Gateway"),
+        responseData.includes("HTTP/1.1 504 Gateway Timeout"),
-        "Should return 502 for timeout"
+        "Should return 504 for timeout"
       );
 
-      // Should timeout around 3 seconds for IMDS endpoints (allow some margin)
+      // Should timeout around 100ms for IMDS endpoints (allow some margin)
       assert.ok(
         duration >= 80 && duration < 200,
         `IMDS timeout should be ~80-200ms, got ${duration}ms`
@@ -280,10 +280,10 @@ describe("registryProxy.connectTunnel", () => {
 
       const duration = Date.now() - startTime;
 
-      // Should return 502 Bad Gateway (timeout)
+      // Should return 504 Gateway Timeout (not 502 - 504 is for actual timeouts)
       assert.ok(
-        responseData.includes("HTTP/1.1 502 Bad Gateway"),
+        responseData.includes("HTTP/1.1 504 Gateway Timeout"),
-        "Should return 502 for timeout"
+        "Should return 504 for timeout"
       );
 
       // Should NOT be instant - it should retry the connection (taking ~500ms due to mock timeout)

packages/safe-chain/src/registryProxy/tunnelRequestHandler.js

@@ -43,6 +43,7 @@ export function tunnelRequest(req, clientSocket, head) {
 function tunnelRequestToDestination(req, clientSocket, head) {
   const { port, hostname } = new URL(`http://${req.url}`);
   const isImds = isImdsEndpoint(hostname);
+  const targetPort = Number.parseInt(port) || 443;
 
   if (timedoutImdsEndpoints.includes(hostname)) {
     clientSocket.end("HTTP/1.1 502 Bad Gateway\r\n\r\n");
@@ -58,64 +59,77 @@ function tunnelRequestToDestination(req, clientSocket, head) {
     return;
   }
 
-  const serverSocket = net.connect(
-    Number.parseInt(port) || 443,
-    hostname,
-    () => {
-      clientSocket.write("HTTP/1.1 200 Connection Established\r\n\r\n");
-      serverSocket.write(head);
-      serverSocket.pipe(clientSocket);
-      clientSocket.pipe(serverSocket);
-    }
-  );
-
-  // Set explicit connection timeout to avoid waiting for OS default (~2 minutes).
-  // IMDS endpoints get shorter timeout (3s) since they're commonly unreachable outside cloud environments.
   const connectTimeout = getConnectTimeout(hostname);
-  serverSocket.setTimeout(connectTimeout);
 
-  serverSocket.on("timeout", () => {
+  // Use JS setTimeout for true connection timeout (not idle timeout).
-    // Suppress error logging for IMDS endpoints - timeouts are expected when not in cloud
+  // socket.setTimeout() measures inactivity, not time since connection attempt.
+  const connectTimer = setTimeout(() => {
     if (isImds) {
       timedoutImdsEndpoints.push(hostname);
       ui.writeVerbose(
-        `Safe-chain: connect to ${hostname}:${
+        `Safe-chain: connect to ${hostname}:${targetPort} timed out after ${connectTimeout}ms`
-          port || 443
-        } timed out after ${connectTimeout}ms`
       );
     } else {
       ui.writeError(
-        `Safe-chain: connect to ${hostname}:${
+        `Safe-chain: connect to ${hostname}:${targetPort} timed out after ${connectTimeout}ms`
-          port || 443
-        } timed out after ${connectTimeout}ms`
       );
     }
-    serverSocket.destroy(); // Clean up socket to prevent event loop hanging
+    serverSocket.destroy();
-    clientSocket.end("HTTP/1.1 502 Bad Gateway\r\n\r\n");
+    if (clientSocket.writable) {
+      clientSocket.end("HTTP/1.1 504 Gateway Timeout\r\n\r\n");
+    }
+  }, connectTimeout);
+
+  const serverSocket = net.connect(targetPort, hostname, () => {
+    // Clear timer to prevent false timeout errors after successful connection
+    clearTimeout(connectTimer);
+
+    clientSocket.write("HTTP/1.1 200 Connection Established\r\n\r\n");
+    serverSocket.write(head);
+    serverSocket.pipe(clientSocket);
+    clientSocket.pipe(serverSocket);
   });
 
   clientSocket.on("error", () => {
     // This can happen if the client TCP socket sends RST instead of FIN.
     // Not subscribing to 'error' event will cause node to throw and crash.
+    clearTimeout(connectTimer);
+    if (serverSocket.writable) {
+      serverSocket.end();
+    }
+  });
+
+  clientSocket.on("close", () => {
+    // Client closed connection - clean up server socket
+    clearTimeout(connectTimer);
     if (serverSocket.writable) {
       serverSocket.end();
     }
   });
 
   serverSocket.on("error", (err) => {
+    clearTimeout(connectTimer);
     if (isImds) {
       ui.writeVerbose(
-        `Safe-chain: error connecting to ${hostname}:${port} - ${err.message}`
+        `Safe-chain: error connecting to ${hostname}:${targetPort} - ${err.message}`
       );
     } else {
       ui.writeError(
-        `Safe-chain: error connecting to ${hostname}:${port} - ${err.message}`
+        `Safe-chain: error connecting to ${hostname}:${targetPort} - ${err.message}`
       );
     }
     if (clientSocket.writable) {
       clientSocket.end("HTTP/1.1 502 Bad Gateway\r\n\r\n");
     }
   });
+
+  serverSocket.on("close", () => {
+    // Server closed connection - clean up client socket
+    clearTimeout(connectTimer);
+    if (clientSocket.writable) {
+      clientSocket.end();
+    }
+  });
 }
 
 /**