fix(connect): make timeout cover the STOMP handshake (fix hung-CONNECT socket leak)

lib/connect.js

@@ -50,21 +50,34 @@ function connect() {
   };
 
   const onConnected = function() {
-    
+
     if (originalSocketDestroy) {
       socket.destroy = originalSocketDestroy;
     }
-
-    cleanup();
-
+
+    // Keep the connect timeout armed through the STOMP handshake (do not
+    // clear it on transport connect). Otherwise a server that accepts the
+    // socket but never sends CONNECTED hangs forever with the socket open
+    // and the callback never fires. Cleared when the handshake settles.
+    client.removeListener('error', onError);
+    client.removeListener('connect', onConnected);
+
     client.emit('socket-connect');
-    
+
     const connectOpts = Object.assign(
-      {host: options.host}, 
+      {host: options.host},
       options.connectHeaders
     );
-
-    client.connect(connectOpts, connectListener);
+
+    client.connect(connectOpts, function(error, connectedClient) {
+      if (timeout) {
+        clearTimeout(timeout);
+        timeout = null;
+      }
+      if (typeof connectListener === 'function') {
+        connectListener(error, connectedClient);
+      }
+    });
   };
 
   let transportConnect = net.connect;

test/connect.js

@@ -84,9 +84,9 @@ describe('connect(options, [connectionListener])', function(){
     });
 
     it('should callback on error', function(done){
-       
+
         var server = startBrokenServer();
-        
+
         connect({
             host:'127.0.0.1',
             port: server.address().port
@@ -95,6 +95,29 @@ describe('connect(options, [connectionListener])', function(){
             done();
         });
     });
+
+    it('should time out when the STOMP handshake stalls after the transport connects', function(done){
+
+        // A server that accepts the TCP socket but never completes the STOMP handshake
+        // (never sends CONNECTED). Before the fix, the connect timeout was cleared once the
+        // transport connected, so this hung forever with the socket left open.
+        var sockets = [];
+        var server = net.createServer({family: 4}, function(socket){
+            sockets.push(socket); // hold the socket open, never respond
+        });
+        server.listen(0);
+
+        connect({
+            host: '127.0.0.1',
+            port: server.address().port,
+            timeout: 150
+        }, function(error){
+            assert(error, 'expected a connect timeout error');
+            sockets.forEach(function(s){ s.destroy(); });
+            server.close();
+            done();
+        });
+    });
 
     it('should accept a transport connect function', function(done){