From faeab2e3fea6ede710d1be23e1a97eac7e54c0c3 Mon Sep 17 00:00:00 2001 From: Wij Skinner Date: Tue, 27 Feb 2018 12:49:16 +0000 Subject: [PATCH 1/9] Fixes cypress crash on socket hang up during attempted upgrade --- packages/server/lib/server.coffee | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/server/lib/server.coffee b/packages/server/lib/server.coffee index 7f89c62c2d6a..30057b0da173 100644 --- a/packages/server/lib/server.coffee +++ b/packages/server/lib/server.coffee @@ -538,6 +538,9 @@ class Server protocol: protocol } }) + + proxy.on "error", (err, req, res) => + res.end() else ## we can't do anything with this socket ## since we don't know how to proxy it! From 6acd366baea78a8c40550f803a2e3a0f4ec5384d Mon Sep 17 00:00:00 2001 From: Wij Skinner Date: Thu, 1 Mar 2018 02:34:02 +0000 Subject: [PATCH 2/9] Fixing broken tests --- packages/server/test/unit/server_spec.coffee | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 = {} From 6a53452f17ee178c837f115b46596b9800d8a67d Mon Sep 17 00:00:00 2001 From: wijwoj Date: Fri, 6 Apr 2018 14:50:35 +0100 Subject: [PATCH 3/9] Do something with the error --- packages/server/lib/server.coffee | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/server/lib/server.coffee b/packages/server/lib/server.coffee index 30057b0da173..aeeac83b6654 100644 --- a/packages/server/lib/server.coffee +++ b/packages/server/lib/server.coffee @@ -539,8 +539,14 @@ class Server } }) - proxy.on "error", (err, req, res) => - res.end() + ## handle errors gracefully + proxy.on "error", (err, req, res) -> + if (!res.headersSent) + res.writeHead({"contentType":"application/json"}) + + json = {error: "proxy_error", reason: err.message} + res.end(JSON.stringify(json)) + else ## we can't do anything with this socket ## since we don't know how to proxy it! From e735b2f1209be1d85577a1f3e8f5e6d4c7fb7e37 Mon Sep 17 00:00:00 2001 From: wijwoj Date: Fri, 6 Apr 2018 14:51:09 +0100 Subject: [PATCH 4/9] Added unit test --- packages/server/test/unit/server_spec.coffee | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/server/test/unit/server_spec.coffee b/packages/server/test/unit/server_spec.coffee index 66d86387cb2c..91ee5ccbc62a 100644 --- a/packages/server/test/unit/server_spec.coffee +++ b/packages/server/test/unit/server_spec.coffee @@ -242,6 +242,26 @@ describe "lib/server", -> } }) + it "handles errors gracefully", -> + error = new Error("Websocket error") + @proxy = @sandbox.stub({ + ws: -> + on: -> throw error + }) + + @server._onDomainSet("https://www.google.com") + + req = { + url: "/" + headers: { + host: "www.google.com" + } + } + + @server.proxyWebsockets(@proxy, "/foo", req, @socket, @head) + + expect(@proxy.on).to.be.calledWith("error") + it "ends the socket if its writable and there is no __cypress.remoteHost", -> req = { url: "/" From b1f512934ec40e55d5df6525f505522c9f386c9d Mon Sep 17 00:00:00 2001 From: wijwoj Date: Fri, 6 Apr 2018 18:08:14 +0100 Subject: [PATCH 5/9] Handle error with 500 status --- packages/server/lib/server.coffee | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/server/lib/server.coffee b/packages/server/lib/server.coffee index aeeac83b6654..c2221785568a 100644 --- a/packages/server/lib/server.coffee +++ b/packages/server/lib/server.coffee @@ -539,13 +539,12 @@ class Server } }) - ## handle errors gracefully - proxy.on "error", (err, req, res) -> - if (!res.headersSent) - res.writeHead({"contentType":"application/json"}) - - json = {error: "proxy_error", reason: err.message} - res.end(JSON.stringify(json)) + ## Listen to proxy errors + ## An ECONNRESET can happen frequently when the connection is closed by the + ## user, e.g. reloading the page when it's still loading. We handle them with + ## just a 500 code response, so that the server doesn't crash when they occur + proxy.on 'error', (err, req, res) -> + res.status(500).end() else ## we can't do anything with this socket From 3759558fb9ed90e71017b2102868dd5a043ad636 Mon Sep 17 00:00:00 2001 From: wijwoj Date: Fri, 6 Apr 2018 19:30:06 +0100 Subject: [PATCH 6/9] Reverting to just end the response --- packages/server/lib/server.coffee | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/server/lib/server.coffee b/packages/server/lib/server.coffee index c2221785568a..d598d04fb390 100644 --- a/packages/server/lib/server.coffee +++ b/packages/server/lib/server.coffee @@ -540,11 +540,10 @@ class Server }) ## Listen to proxy errors - ## An ECONNRESET can happen frequently when the connection is closed by the - ## user, e.g. reloading the page when it's still loading. We handle them with - ## just a 500 code response, so that the server doesn't crash when they occur + ## An ECONNRESET error can happen frequently when the connection is closed by the + ## user, e.g. reloading the page when it's still loading, so we just end the request proxy.on 'error', (err, req, res) -> - res.status(500).end() + res.end() else ## we can't do anything with this socket From de3c622750487d5cd92b8ccb87df85bbde571ab5 Mon Sep 17 00:00:00 2001 From: Brian Mann Date: Mon, 23 Apr 2018 00:26:55 -0400 Subject: [PATCH 7/9] handle web socket errors gracefully, send 502 bad gateway - adds e2e + integration tests --- .../__snapshots__/websockets_spec.coffee | 35 +++++++++++++++++ packages/server/lib/modes/run.coffee | 4 +- packages/server/lib/server.coffee | 35 +++++++++++++---- packages/server/package.json | 2 +- .../server/test/e2e/websockets_spec.coffee | 33 ++++++++++++++++ .../test/integration/websockets_spec.coffee | 19 +++++++++ .../integration/websockets_spec.coffee | 39 +++++++++++++++++++ .../server/test/support/helpers/e2e.coffee | 22 ++++------- 8 files changed, 165 insertions(+), 24 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 d598d04fb390..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,14 +563,7 @@ class Server port: port protocol: protocol } - }) - - ## Listen to proxy errors - ## An ECONNRESET error can happen frequently when the connection is closed by the - ## user, e.g. reloading the page when it's still loading, so we just end the request - proxy.on 'error', (err, req, res) -> - res.end() - + }, 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..c400163a9cd3 --- /dev/null +++ b/packages/server/test/e2e/websockets_spec.coffee @@ -0,0 +1,33 @@ +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") + +describe "e2e websockets", -> + e2e.setup({ + servers: [{ + port: 3838 + static: true + onServer: onServer + }, { + port: 3939 + onServer: onWsServer + }] + }) + + ## 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..f41d61de8e11 100644 --- a/packages/server/test/integration/websockets_spec.coffee +++ b/packages/server/test/integration/websockets_spec.coffee @@ -59,6 +59,25 @@ 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.deep.eq({ + 'x-cypress-proxy-error-message': 'connect ECONNREFUSED 127.0.0.1:5555', + 'x-cypress-proxy-error-code': 'ECONNREFUSED' + }) + + 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..19858ca95282 --- /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:3838/foo") + cy.log("should not crash on ECONNRESET websocket upgrade") + cy.window().then (win) -> + Cypress.Promise.all([ + urlClosesWithCode1006(win, "ws://localhost:3838/websocket") + urlClosesWithCode1006(win, "wss://localhost:3006/websocket") + ]) + + cy.log("should be able to send websocket messages") + + cy + .window() + .then (win) -> + new Promise (resolve, reject) -> + ws = new win.WebSocket("ws://localhost:3939/") + 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 From 71a6b0a1dc87200c1fd914071913afb4e34ffae6 Mon Sep 17 00:00:00 2001 From: Brian Mann Date: Mon, 23 Apr 2018 00:51:43 -0400 Subject: [PATCH 8/9] fixes failing tests --- .../test/integration/websockets_spec.coffee | 6 ++---- .../integration/websockets_spec.coffee | 4 ++-- packages/server/test/unit/server_spec.coffee | 20 ------------------- 3 files changed, 4 insertions(+), 26 deletions(-) diff --git a/packages/server/test/integration/websockets_spec.coffee b/packages/server/test/integration/websockets_spec.coffee index f41d61de8e11..7668c5df4dcf 100644 --- a/packages/server/test/integration/websockets_spec.coffee +++ b/packages/server/test/integration/websockets_spec.coffee @@ -71,10 +71,8 @@ describe "Web Sockets", -> client.on "unexpected-response", (req, res) -> expect(res.statusCode).to.eq(502) expect(res.statusMessage).to.eq("Bad Gateway") - expect(res.headers).to.deep.eq({ - 'x-cypress-proxy-error-message': 'connect ECONNREFUSED 127.0.0.1:5555', - 'x-cypress-proxy-error-code': 'ECONNREFUSED' - }) + expect(res.headers).to.have.property("x-cypress-proxy-error-message") + expect(res.headers).to.have.property("x-cypress-proxy-error-code") done() 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 index 19858ca95282..0cbee858502c 100644 --- 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 @@ -18,7 +18,7 @@ describe "websockets", -> it "does not crash", -> cy.visit("http://localhost:3838/foo") cy.log("should not crash on ECONNRESET websocket upgrade") - cy.window().then (win) -> + cy.window().then { timeout: 10000 }, (win) -> Cypress.Promise.all([ urlClosesWithCode1006(win, "ws://localhost:3838/websocket") urlClosesWithCode1006(win, "wss://localhost:3006/websocket") @@ -28,7 +28,7 @@ describe "websockets", -> cy .window() - .then (win) -> + .then { timeout: 10000 }, (win) -> new Promise (resolve, reject) -> ws = new win.WebSocket("ws://localhost:3939/") ws.onmessage = (evt) -> diff --git a/packages/server/test/unit/server_spec.coffee b/packages/server/test/unit/server_spec.coffee index 91ee5ccbc62a..66d86387cb2c 100644 --- a/packages/server/test/unit/server_spec.coffee +++ b/packages/server/test/unit/server_spec.coffee @@ -242,26 +242,6 @@ describe "lib/server", -> } }) - it "handles errors gracefully", -> - error = new Error("Websocket error") - @proxy = @sandbox.stub({ - ws: -> - on: -> throw error - }) - - @server._onDomainSet("https://www.google.com") - - req = { - url: "/" - headers: { - host: "www.google.com" - } - } - - @server.proxyWebsockets(@proxy, "/foo", req, @socket, @head) - - expect(@proxy.on).to.be.calledWith("error") - it "ends the socket if its writable and there is no __cypress.remoteHost", -> req = { url: "/" From 108ded58a6bf6bedd05e7365f2adcec05efaa643 Mon Sep 17 00:00:00 2001 From: Brian Mann Date: Mon, 23 Apr 2018 01:22:29 -0400 Subject: [PATCH 9/9] fixes failing e2e tests --- packages/server/test/e2e/websockets_spec.coffee | 9 +++++++-- .../e2e/cypress/integration/websockets_spec.coffee | 12 ++++++------ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/server/test/e2e/websockets_spec.coffee b/packages/server/test/e2e/websockets_spec.coffee index c400163a9cd3..0e2f5fd4c67a 100644 --- a/packages/server/test/e2e/websockets_spec.coffee +++ b/packages/server/test/e2e/websockets_spec.coffee @@ -12,15 +12,20 @@ onWsServer = (app, server) -> ws.on "message", (msg) -> ws.send(msg + "bar") +onWssServer = (app) -> + describe "e2e websockets", -> e2e.setup({ servers: [{ - port: 3838 + port: 3038 static: true onServer: onServer }, { - port: 3939 + port: 3039 onServer: onWsServer + }, { + port: 3040 + onServer: onWssServer }] }) 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 index 0cbee858502c..1332717954d0 100644 --- 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 @@ -16,21 +16,21 @@ urlClosesWithCode1006 = (win, url) -> describe "websockets", -> it "does not crash", -> - cy.visit("http://localhost:3838/foo") + cy.visit("http://localhost:3038/foo") cy.log("should not crash on ECONNRESET websocket upgrade") - cy.window().then { timeout: 10000 }, (win) -> + cy.window().then (win) -> Cypress.Promise.all([ - urlClosesWithCode1006(win, "ws://localhost:3838/websocket") - urlClosesWithCode1006(win, "wss://localhost:3006/websocket") + urlClosesWithCode1006(win, "ws://localhost:3038/websocket") + urlClosesWithCode1006(win, "wss://localhost:3040/websocket") ]) cy.log("should be able to send websocket messages") cy .window() - .then { timeout: 10000 }, (win) -> + .then (win) -> new Promise (resolve, reject) -> - ws = new win.WebSocket("ws://localhost:3939/") + ws = new win.WebSocket("ws://localhost:3039/") ws.onmessage = (evt) -> resolve(evt.data) ws.onerror = reject