From 746f62ebcaff44ff65602fc8d74e5cf0d6b8194f Mon Sep 17 00:00:00 2001 From: wijwoj Date: Mon, 23 Apr 2018 07:00:41 +0100 Subject: [PATCH] Fixes cypress crash (ECONNRESET) on socket hang up during attempted socket 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 --- .../__snapshots__/websockets_spec.coffee | 35 +++++++++++++++++ packages/server/lib/modes/run.coffee | 4 +- packages/server/lib/server.coffee | 28 ++++++++++++- packages/server/package.json | 2 +- .../server/test/e2e/websockets_spec.coffee | 38 ++++++++++++++++++ .../test/integration/websockets_spec.coffee | 17 ++++++++ .../integration/websockets_spec.coffee | 39 +++++++++++++++++++ .../server/test/support/helpers/e2e.coffee | 22 ++++------- packages/server/test/unit/server_spec.coffee | 5 ++- 9 files changed, 172 insertions(+), 18 deletions(-) create mode 100644 packages/server/__snapshots__/websockets_spec.coffee create mode 100644 packages/server/test/e2e/websockets_spec.coffee create mode 100644 packages/server/test/support/fixtures/projects/e2e/cypress/integration/websockets_spec.coffee diff --git a/packages/server/__snapshots__/websockets_spec.coffee b/packages/server/__snapshots__/websockets_spec.coffee new file mode 100644 index 000000000000..c7cbf611ad15 --- /dev/null +++ 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 index c1b316f651d4..377b21077c22 100644 --- 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 index 7f89c62c2d6a..83273a06a74b 100644 --- a/packages/server/lib/server.coffee +++ b/packages/server/lib/server.coffee @@ -530,6 +530,32 @@ 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: { @@ -537,7 +563,7 @@ class Server 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 index 767608d415e0..ec080c4fbdb8 100644 --- 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 new file mode 100644 index 000000000000..0e2f5fd4c67a --- /dev/null +++ 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("foo>") + +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 index 7a7463993a35..7668c5df4dcf 100644 --- 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/packages/server/test/support/fixtures/projects/e2e/cypress/integration/websockets_spec.coffee b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/websockets_spec.coffee new file mode 100644 index 000000000000..1332717954d0 --- /dev/null +++ b/packages/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 index 923997c3239e..4b1dfe2c0269 100644 --- 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 index ee6dc9c3a5bb..66d86387cb2c 100644 --- 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 = {}