Fixes cypress crash (ECONNRESET) on socket hang up during attempted s…

…ocket upgrade (#1382) * Fixes cypress crash on socket hang up during attempted upgrade * Fixing broken tests * Do something with the error * Added unit test * Handle error with 500 status * Reverting to just end the response * handle web socket errors gracefully, send 502 bad gateway - adds e2e + integration tests * fixes failing tests * fixes failing e2e tests
cypress-io · Apr 23, 2018 · 746f62e · 746f62e
1 parent fc374e1
commit 746f62e
Show file tree

Hide file tree

Showing 9 changed files with 172 additions and 18 deletions.
diff --git a/packages/server/__snapshots__/websockets_spec.coffee b/packages/server/__snapshots__/websockets_spec.coffee
@@ -0,0 +1,35 @@
+exports['e2e websockets passes 1'] = `
+Started video recording: /foo/bar/.projects/e2e/cypress/videos/abc123.mp4
+
+  (Tests Starting)
+
+
+  websockets
+    ✓ does not crash
+
+
+  1 passing
+
+
+  (Tests Finished)
+
+  - Tests:           1
+  - Passes:          1
+  - Failures:        0
+  - Pending:         0
+  - Duration:        10 seconds
+  - Screenshots:     0
+  - Video Recorded:  true
+  - Cypress Version: 1.2.3
+
+
+  (Video)
+
+  - Started processing:   Compressing to 32 CRF
+  - Finished processing:  /foo/bar/.projects/e2e/cypress/videos/abc123.mp4 (0 seconds)
+
+
+  (All Done)
+
+`
+
diff --git a/packages/server/lib/modes/run.coffee b/packages/server/lib/modes/run.coffee
@@ -244,7 +244,9 @@ module.exports = {
 
   listenForProjectEnd: (project, headed, exit) ->
     new Promise (resolve) ->
-      return if exit is false
+      if exit is false
+        resolve = (arg) ->
+          console.log("not exiting due to options.exit being false")
 
       onEarlyExit = (errMsg) ->
         ## probably should say we ended

diff --git a/packages/server/lib/server.coffee b/packages/server/lib/server.coffee
@@ -530,14 +530,40 @@ class Server
       {protocol} = url.parse(remoteOrigin)
       {hostname} = url.parse("http://#{host}")
 
+      onProxyErr = (err, req, res) ->
+        ## by default http-proxy will call socket.end
+        ## with no data, so we need to override the end
+        ## function and write our own response
+        ## https://github.com/nodejitsu/node-http-proxy/blob/master/lib/http-proxy/passes/ws-incoming.js#L159
+        end = socket.end
+        socket.end = ->
+          socket.end = end
+
+          response = [
+            "HTTP/#{req.httpVersion} 502 #{statusCode.getText(502)}"
+            "X-Cypress-Proxy-Error-Message: #{err.message}"
+            "X-Cypress-Proxy-Error-Code: #{err.code}"
+          ].join("\r\n") + "\r\n\r\n"
+
+          proxiedUrl = "#{protocol}//#{hostname}:#{port}"
+
+          debug(
+            "Got ERROR proxying websocket connection to url: '%s' received error: '%s' with code '%s'",
+            proxiedUrl,
+            err.toString()
+            err.code
+          )
+
+          socket.end(response)
+
       proxy.ws(req, socket, head, {
         secure: false
         target: {
           host: hostname
           port: port
           protocol: protocol
         }
-      })
+      }, onProxyErr)
     else
       ## we can't do anything with this socket
       ## since we don't know how to proxy it!

diff --git a/packages/server/package.json b/packages/server/package.json
@@ -63,7 +63,7 @@
     "supertest-session": "0.0.7",
     "through2": "0.6.3",
     "vagrant": "0.0.1",
-    "ws": "^1.0.1",
+    "ws": "^5.1.1",
     "xvfb": "cypress-io/node-xvfb#22e3783c31d81ebe64d8c0df491ea00cdc74726a",
     "xvfb-maybe": "cypress-io/xvfb-maybe#c4a810c42d603949cd63b8cf245f6c239331d370"
   },

diff --git a/packages/server/test/e2e/websockets_spec.coffee b/packages/server/test/e2e/websockets_spec.coffee
@@ -0,0 +1,38 @@
+ws = require("ws")
+
+e2e = require("../support/helpers/e2e")
+
+onServer = (app) ->
+  app.get "/foo", (req, res) ->
+    res.send("<html>foo></html>")
+
+onWsServer = (app, server) ->
+  wss = new ws.Server({ server })
+  wss.on "connection", (ws) ->
+    ws.on "message", (msg) ->
+      ws.send(msg + "bar")
+
+onWssServer = (app) ->
+
+describe "e2e websockets", ->
+  e2e.setup({
+    servers: [{
+      port: 3038
+      static: true
+      onServer: onServer
+    }, {
+      port: 3039
+      onServer: onWsServer
+    }, {
+      port: 3040
+      onServer: onWssServer
+    }]
+  })
+
+  ## https://github.com/cypress-io/cypress/issues/556
+  it "passes", ->
+    e2e.exec(@, {
+      spec: "websockets_spec.coffee"
+      snapshot: true
+      expectedExitCode: 0
+    })
diff --git a/packages/server/test/integration/websockets_spec.coffee b/packages/server/test/integration/websockets_spec.coffee
@@ -59,6 +59,23 @@ describe "Web Sockets", ->
         expect(err.code).to.eq("ECONNRESET")
         done()
 
+    it "sends back 502 Bad Gateway when error upgrading", (done) ->
+      agent = new httpsAgent("http://localhost:#{cyPort}")
+
+      @server._onDomainSet("http://localhost:#{otherPort}")
+
+      client = new ws("ws://localhost:#{otherPort}", {
+        agent: agent
+      })
+
+      client.on "unexpected-response", (req, res) ->
+        expect(res.statusCode).to.eq(502)
+        expect(res.statusMessage).to.eq("Bad Gateway")
+        expect(res.headers).to.have.property("x-cypress-proxy-error-message")
+        expect(res.headers).to.have.property("x-cypress-proxy-error-code")
+
+        done()
+
     it "proxies https messages", (done) ->
       @server._onDomainSet("https://localhost:#{wssPort}")
 

diff --git a/...ages/server/test/support/fixtures/projects/e2e/cypress/integration/websockets_spec.coffee b/...ages/server/test/support/fixtures/projects/e2e/cypress/integration/websockets_spec.coffee
@@ -0,0 +1,39 @@
+urlClosesWithCode1006 = (win, url) ->
+  new Promise (resolve, reject) ->
+    ws = new win.WebSocket(url)
+
+    # ws.onerror = (err) ->
+      # debugger
+
+    ws.onclose = (evt) ->
+      if evt.code is 1006
+        resolve()
+      else
+        reject("websocket connection should have been closed with code 1006 for url: #{url} but was instead closed with code: #{evt.code}")
+
+    ws.onopen = (evt) ->
+      reject("websocket connection should not have opened for url: #{url}")
+
+describe "websockets", ->
+  it "does not crash", ->
+    cy.visit("http://localhost:3038/foo")
+    cy.log("should not crash on ECONNRESET websocket upgrade")
+    cy.window().then (win) ->
+      Cypress.Promise.all([
+        urlClosesWithCode1006(win, "ws://localhost:3038/websocket")
+        urlClosesWithCode1006(win, "wss://localhost:3040/websocket")
+      ])
+
+    cy.log("should be able to send websocket messages")
+
+    cy
+    .window()
+    .then (win) ->
+      new Promise (resolve, reject) ->
+        ws = new win.WebSocket("ws://localhost:3039/")
+        ws.onmessage = (evt) ->
+          resolve(evt.data)
+        ws.onerror = reject
+        ws.onopen = ->
+          ws.send("foo")
+    .should("eq", "foobar")
diff --git a/packages/server/test/support/helpers/e2e.coffee b/packages/server/test/support/helpers/e2e.coffee
@@ -74,7 +74,7 @@ startServer = (obj) ->
   new Promise (resolve) ->
     srv.listen port, =>
       console.log "listening on port: #{port}"
-      onServer?(app)
+      onServer?(app, srv)
 
       resolve(srv)
 
@@ -245,22 +245,16 @@ module.exports = {
 
     exit = (code) ->
       if (expected = options.expectedExitCode)?
-        try
-          expect(expected).to.eq(code)
-        catch err
-          return reject(err)
+        expect(expected).to.eq(code)
 
       ## snapshot the stdout!
       if options.snapshot
-        try
-          ## enable callback to modify stdout
-          if ostd = options.onStdout
-            stdout = ostd(stdout)
-
-          str = normalizeStdout(stdout)
-          snapshot(str)
-        catch err
-          reject(err)
+        ## enable callback to modify stdout
+        if ostd = options.onStdout
+          stdout = ostd(stdout)
+
+        str = normalizeStdout(stdout)
+        snapshot(str)
 
       return {
         code:   code

diff --git a/packages/server/test/unit/server_spec.coffee b/packages/server/test/unit/server_spec.coffee
@@ -207,7 +207,10 @@ describe "lib/server", ->
 
   context "#proxyWebsockets", ->
     beforeEach ->
-      @proxy  = @sandbox.stub({ws: ->})
+      @proxy  = @sandbox.stub({
+        ws: ->
+        on: ->
+      })
       @socket = @sandbox.stub({end: ->})
       @head   = {}