[fix doc] Add error handler to reverseProxy request when proxying WebSockets to prevent unhandled ParseError. Rename some variables in proxyWebSocketRequest to make the code more readable

Author: indexzero

lib/node-http-proxy.js

@@ -543,56 +543,56 @@ HttpProxy.prototype.proxyWebSocketRequest = function (req, socket, head, options
     }
   }
 
-  function onUpgrade(out, reverseProxy) {
-    if (!out) {
-      reverseProxy.end();
+  function onUpgrade(reverseProxy, proxySocket) {
+    if (!reverseProxy) {
+      proxySocket.end();
       socket.end();
       return;
     }
 
     var listeners = {};
 
     // We're now connected to the server, so lets change server socket
-    reverseProxy.on('data', listeners._r_data = function(data) {
+    proxySocket.on('data', listeners._r_data = function(data) {
       // Pass data to client
-      if (out.incoming.socket.writable) {
+      if (reverseProxy.incoming.socket.writable) {
         try {
-          out.incoming.socket.write(data);
+          reverseProxy.incoming.socket.write(data);
         } 
         catch (e) {
-          out.incoming.socket.end();
-          reverseProxy.end();
+          reverseProxy.incoming.socket.end();
+          proxySocket.end();
         }
       }
     });
 
-    out.incoming.socket.on('data', listeners._data = function(data) {
+    reverseProxy.incoming.socket.on('data', listeners._data = function(data) {
       // Pass data from client to server
       try {
-        reverseProxy.write(data);
+        proxySocket.write(data);
       } 
       catch (e) {
-        reverseProxy.end();
+        proxySocket.end();
         socket.end();
       }
     });
 
     // Detach event listeners from reverseProxy
     function detach() {
-      reverseProxy.removeListener('end', listeners._r_close);
-      reverseProxy.removeListener('data', listeners._r_data);
-      out.incoming.socket.removeListener('data', listeners._data);
-      out.incoming.socket.removeListener('end', listeners._close);
+      proxySocket.removeListener('end', listeners._r_close);
+      proxySocket.removeListener('data', listeners._r_data);
+      reverseProxy.incoming.socket.removeListener('data', listeners._data);
+      reverseProxy.incoming.socket.removeListener('end', listeners._close);
     }
 
     // Hook disconnections
-    reverseProxy.on('end', listeners._r_close = function() {
-      out.incoming.socket.end();
+    proxySocket.on('end', listeners._r_close = function() {
+      reverseProxy.incoming.socket.end();
       detach();
     });
 
-    out.incoming.socket.on('end', listeners._close = function() {
-      reverseProxy.end();
+    reverseProxy.incoming.socket.on('end', listeners._close = function() {
+      proxySocket.end();
       detach();
     });
   };
@@ -620,16 +620,16 @@ HttpProxy.prototype.proxyWebSocketRequest = function (req, socket, head, options
   };
 
   // Make the outgoing WebSocket request
-  var request = agent.appendMessage(outgoing);
+  var reverseProxy = agent.appendMessage(outgoing);
 
   //
   // Here we set the incoming `req`, `socket` and `head` data to the outgoing
   // request so that we can reuse this data later on in the closure scope
   // available to the `upgrade` event. This bookkeeping is not tracked anywhere 
   // in nodejs core and is **very** specific to proxying WebSockets.
   //
-  request.agent = agent;
-  request.incoming = {
+  reverseProxy.agent = agent;
+  reverseProxy.incoming = {
     request: req,
     socket: socket,
     head: head
@@ -644,69 +644,84 @@ HttpProxy.prototype.proxyWebSocketRequest = function (req, socket, head, options
   // there is no mapping of the 
   //
   if (!agent._events || agent._events['upgrade'].length === 0) {
-    agent.on('upgrade', function (out, remoteSocket, head) {
-      // Prepare socket
+    agent.on('upgrade', function (_, remoteSocket, head) {
+      //
+      // Prepare the socket for the reverseProxy request and begin to 
+      // stream data between the two sockets
+      //
       _socket(remoteSocket, true);
-      
-      // Emit event
       onUpgrade(remoteSocket._httpMessage, remoteSocket);
     });
   }
 
-  if (typeof request.socket !== 'undefined') {
-    request.socket.on('data', function handshake (data) {
-      // Handshaking
-
-      // Ok, kind of harmfull part of code
-      // Socket.IO is sending hash at the end of handshake
-      // If protocol = 76
-      // But we need to replace 'host' and 'origin' in response
-      // So we split data to printable data and to non-printable
-      // (Non-printable will come after double-CRLF)
+  //
+  // If the reverseProxy connection has an underlying socket,
+  // then behing the handshake.
+  //
+  if (typeof reverseProxy.socket !== 'undefined') {
+    reverseProxy.socket.on('data', function handshake (data) {
+      //
+      // Ok, kind of harmfull part of code. Socket.IO sends a hash
+      // at the end of handshake if protocol === 76, but we need 
+      // to replace 'host' and 'origin' in response so we split 
+      // data to printable data and to non-printable. (Non-printable 
+      // will come after double-CRLF).
+      //
       var sdata = data.toString();
 
-      // Get Printable
+      // Get the Printable data
       sdata = sdata.substr(0, sdata.search(CRLF + CRLF));
 
-      // Get Non-Printable
+      // Get the Non-Printable data
       data = data.slice(Buffer.byteLength(sdata), data.length);
 
-      // Replace host and origin
+      // Replace the host and origin headers in the Printable data
       sdata = sdata.replace(remoteHost, options.host)
                    .replace(remoteHost, options.host);
 
       try {
-        // Write printable
+        //
+        // Write the printable and non-printable data to the socket
+        // from the original incoming request. 
+        // 
         socket.write(sdata);
-
-        // Write non-printable
         socket.write(data);
       } 
       catch (e) {
-        request.end();
+        reverseProxy.end();
         socket.end();
       }
 
       // Catch socket errors
       socket.on('error', function() {
-        request.end();
+        reverseProxy.end();
+        socket.end();
       });
 
       // Remove data listener now that the 'handshake' is complete
-      request.socket.removeListener('data', handshake);
+      reverseProxy.socket.removeListener('data', handshake);
     });
   }
+  
+  reverseProxy.on('error', function (err) {
+    reverseProxy.end();
+    socket.end();
+  });
 
-  // Write upgrade-head
   try {
-    request.write(head);
+    //
+    // Attempt to write the upgrade-head to the reverseProxy request.
+    //
+    reverseProxy.write(head);
   } 
   catch (ex) {
-    request.end();
+    reverseProxy.end();
     socket.end();
   }
 
+  //
   // If we have been passed buffered data, resume it.
+  //
   if (options.buffer && !errState) {
     options.buffer.resume();
   }