fix(node-http-handler): begin socket timeout countdown before socket event (#4804)

Author: kuhe

packages/node-http-handler/src/set-connection-timeout.spec.ts

@@ -34,6 +34,7 @@ describe("setConnectionTimeout", () => {
     });
 
     afterEach(() => {
+      jest.advanceTimersByTime(10000);
       jest.useRealTimers();
     });
 
@@ -48,7 +49,7 @@ describe("setConnectionTimeout", () => {
         connecting: false,
       });
       expect(mockSocket.on).not.toHaveBeenCalled();
-      expect(setTimeout).not.toHaveBeenCalled();
+      expect(setTimeout).toHaveBeenCalled();
       expect(reject).not.toHaveBeenCalled();
     });
 
@@ -74,8 +75,6 @@ describe("setConnectionTimeout", () => {
     });
 
     it("clears timeout if socket gets connected", () => {
-      const mockTimeoutId = 42;
-      (setTimeout as unknown as jest.Mock).mockReturnValueOnce(mockTimeoutId);
       clientRequest.on.mock.calls[0][1](mockSocket);
 
       expect(clientRequest.destroy).not.toHaveBeenCalled();
@@ -87,7 +86,6 @@ describe("setConnectionTimeout", () => {
       mockSocket.on.mock.calls[0][1]();
 
       expect(clearTimeout).toHaveBeenCalled();
-      expect(clearTimeout).toHaveBeenCalledWith(mockTimeoutId);
 
       // Fast-forward until timer has been executed.
       jest.runAllTimers();

packages/node-http-handler/src/set-connection-timeout.ts

@@ -6,23 +6,23 @@ export const setConnectionTimeout = (request: ClientRequest, reject: (err: Error
     return;
   }
 
+  // Throw a connecting timeout error unless a connection is made within time.
+  const timeoutId = setTimeout(() => {
+    request.destroy();
+    reject(
+      Object.assign(new Error(`Socket timed out without establishing a connection within ${timeoutInMs} ms`), {
+        name: "TimeoutError",
+      })
+    );
+  }, timeoutInMs);
+
   request.on("socket", (socket: Socket) => {
     if (socket.connecting) {
-      // Throw a connecting timeout error unless a connection is made within x time.
-      const timeoutId = setTimeout(() => {
-        // destroy the request.
-        request.destroy();
-        reject(
-          Object.assign(new Error(`Socket timed out without establishing a connection within ${timeoutInMs} ms`), {
-            name: "TimeoutError",
-          })
-        );
-      }, timeoutInMs);
-
-      // if the connection was established, cancel the timeout.
       socket.on("connect", () => {
         clearTimeout(timeoutId);
       });
+    } else {
+      clearTimeout(timeoutId);
     }
   });
 };