From e8c01d84e1ad0457134d97bfd08763f3522d6978 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Tue, 30 Nov 2021 16:01:49 -0500 Subject: [PATCH 01/12] fix: disable automatic request retries --- packages/proxy/lib/http/request-middleware.ts | 2 +- packages/server/lib/request.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/proxy/lib/http/request-middleware.ts b/packages/proxy/lib/http/request-middleware.ts index b857ad4fc3ca..e697fff4b8f9 100644 --- a/packages/proxy/lib/http/request-middleware.ts +++ b/packages/proxy/lib/http/request-middleware.ts @@ -148,7 +148,7 @@ const SendRequestOutgoing: RequestMiddleware = function () { timeout: this.req.responseTimeout, strictSSL: false, followRedirect: this.req.followRedirect || false, - retryIntervals: [0, 100, 200, 200], + retryIntervals: [], url: this.req.proxiedUrl, } diff --git a/packages/server/lib/request.js b/packages/server/lib/request.js index 8b99bf1d646e..d0661631a891 100644 --- a/packages/server/lib/request.js +++ b/packages/server/lib/request.js @@ -414,7 +414,7 @@ const setDefaults = (opts) => { .chain(opts) .defaults({ requestId: _.uniqueId('request'), - retryIntervals: [0, 1000, 2000, 2000], + retryIntervals: [], retryOnNetworkFailure: true, retryOnStatusCodeFailure: false, }) From ae7bda8ec1947087a34ea2d0be1ec33ddfcb6f0d Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Tue, 7 Dec 2021 10:11:49 -0500 Subject: [PATCH 02/12] fix TLSv1 --- packages/server/lib/request.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/server/lib/request.js b/packages/server/lib/request.js index d0661631a891..6bfb5225c49f 100644 --- a/packages/server/lib/request.js +++ b/packages/server/lib/request.js @@ -118,6 +118,11 @@ const maybeRetryOnNetworkFailure = function (err, options = {}) { // https://github.com/cypress-io/cypress/pull/6705 debug('detected TLS version error, setting min version to TLSv1') opts.minVersion = 'TLSv1' + + if (retryIntervals.length === 0) { + // normally, this request would not be retried, but we need to retry in order to support TLSv1 + return onNext(0, 1) + } } if (!isTlsVersionError && !isErrEmptyResponseError(err.originalErr || err) && !isRetriableError(err, retryOnNetworkFailure)) { From 97bad5e3c97e56e9a3a65fca51b08805fbe93968 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Tue, 7 Dec 2021 10:12:01 -0500 Subject: [PATCH 03/12] update request_spec --- packages/server/test/unit/request_spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/test/unit/request_spec.js b/packages/server/test/unit/request_spec.js index 7110a2b5b045..5984e8cd6be7 100644 --- a/packages/server/test/unit/request_spec.js +++ b/packages/server/test/unit/request_spec.js @@ -141,10 +141,10 @@ describe('lib/request', () => { expect(opts.delaysRemaining).to.deep.eq(retryIntervals) }) - it('retryIntervals to [0, 1000, 2000, 2000] by default', () => { + it('retryIntervals to [] by default', () => { const opts = request.setDefaults({}) - expect(opts.retryIntervals).to.deep.eq([0, 1000, 2000, 2000]) + expect(opts.retryIntervals).to.deep.eq([]) }) it('delaysRemaining can be overridden', () => { From 9c019a70e0a5ee575f0ddac3ff7e5dc1e394577e Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Tue, 7 Dec 2021 10:19:04 -0500 Subject: [PATCH 04/12] remove now invalid https_passthru_spec --- .../network_error_handling_spec.js | 62 --------------- .../integration/https_passthru_spec.js | 30 -------- .../test/network_error_handling_spec.js | 75 ------------------- 3 files changed, 167 deletions(-) delete mode 100644 system-tests/projects/e2e/cypress/integration/https_passthru_spec.js diff --git a/system-tests/__snapshots__/network_error_handling_spec.js b/system-tests/__snapshots__/network_error_handling_spec.js index 3c10aaff005e..c9f61fd46d2f 100644 --- a/system-tests/__snapshots__/network_error_handling_spec.js +++ b/system-tests/__snapshots__/network_error_handling_spec.js @@ -15,68 +15,6 @@ Cypress failed to verify that your server is running. Please start this server and then run Cypress again. -` - -exports['e2e network error handling Cypress retries HTTPS passthrough behind a proxy 1'] = ` - -==================================================================================================== - - (Run Starting) - - ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ - │ Cypress: 1.2.3 │ - │ Browser: FooBrowser 88 │ - │ Specs: 1 found (https_passthru_spec.js) │ - │ Searched: cypress/integration/https_passthru_spec.js │ - └────────────────────────────────────────────────────────────────────────────────────────────────┘ - - -──────────────────────────────────────────────────────────────────────────────────────────────────── - - Running: https_passthru_spec.js (1 of 1) - - - https passthru retries - ✓ retries when visiting a non-test domain - ✓ passes through the network error when it cannot connect to the proxy - - - 2 passing - - - (Results) - - ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ - │ Tests: 2 │ - │ Passing: 2 │ - │ Failing: 0 │ - │ Pending: 0 │ - │ Skipped: 0 │ - │ Screenshots: 0 │ - │ Video: true │ - │ Duration: X seconds │ - │ Spec Ran: https_passthru_spec.js │ - └────────────────────────────────────────────────────────────────────────────────────────────────┘ - - - (Video) - - - Started processing: Compressing to 32 CRF - - Finished processing: /XXX/XXX/XXX/cypress/videos/https_passthru_spec.js.mp4 (X second) - - -==================================================================================================== - - (Run Finished) - - - Spec Tests Passing Failing Pending Skipped - ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ - │ ✔ https_passthru_spec.js XX:XX 2 2 - - - │ - └────────────────────────────────────────────────────────────────────────────────────────────────┘ - ✔ All specs passed! XX:XX 2 2 - - - - - ` exports['e2e network error handling Cypress does not connect to the upstream proxy for the SNI server request 1'] = ` diff --git a/system-tests/projects/e2e/cypress/integration/https_passthru_spec.js b/system-tests/projects/e2e/cypress/integration/https_passthru_spec.js deleted file mode 100644 index b7315852265f..000000000000 --- a/system-tests/projects/e2e/cypress/integration/https_passthru_spec.js +++ /dev/null @@ -1,30 +0,0 @@ -describe('https passthru retries', () => { - it('retries when visiting a non-test domain', () => { - // cy.timeout(1e9) - - return new Cypress.Promise((resolve, reject) => { - const img = new Image() - - img.src = 'https://localhost:13372/javascript-logo.png' - img.onload = resolve - img.onerror = () => { - reject(new Error('onerror event fired, but should not have. expected onload to fire.')) - } - }) - }) - - it('passes through the network error when it cannot connect to the proxy', () => { - // cy.timeout(1e9) - - return new Cypress.Promise((resolve, reject) => { - const img = new Image() - - img.src = 'https://localhost:13373/expected-network-error' - img.onload = () => { - reject(new Error('onload event fired, but should not have. expected onerror to fire.')) - } - - img.onerror = resolve - }) - }) -}) diff --git a/system-tests/test/network_error_handling_spec.js b/system-tests/test/network_error_handling_spec.js index 99219f098d0c..c94ba3218683 100644 --- a/system-tests/test/network_error_handling_spec.js +++ b/system-tests/test/network_error_handling_spec.js @@ -17,7 +17,6 @@ let mitmProxy = require('http-mitm-proxy') const PORT = 13370 const PROXY_PORT = 13371 const HTTPS_PORT = 13372 -const ERR_HTTPS_PORT = 13373 const start = Number(new Date()) @@ -409,80 +408,6 @@ describe('e2e network error handling', function () { }) }) - it('retries HTTPS passthrough behind a proxy', function () { - // this tests retrying multiple times - // to connect to the upstream server - // as well as network errors when the - // upstream server is not accessible - - const connectCounts = {} - - const onConnect = function ({ host, port, socket }) { - const dest = `${host}:${port}` - - if (connectCounts[dest] == null) { - connectCounts[dest] = 0 - } - - connectCounts[dest] += 1 - - switch (port) { - case HTTPS_PORT: - // this tests network related errors - // when we do immediately destroy the - // socket and prevent connecting to the - // upstream server - // - // on the 3rd time around, don't destroy the socket. - if (connectCounts[`localhost:${HTTPS_PORT}`] >= 3) { - return true - } - - // else if this is the 1st or 2nd time destroy the - // socket so we retry connecting to the debug proxy - socket.destroy() - - return false - - case ERR_HTTPS_PORT: - // always destroy the socket attempting to connect - // to the upstream server to test that network errors - // are propagated correctly - socket.destroy() - - return false - - default: - // pass everything else on to the upstream - // server as expected - return true - } - } - - this.debugProxy = new DebugProxy({ - onConnect, - }) - - return this.debugProxy - .start(PROXY_PORT) - .then(() => { - process.env.HTTP_PROXY = `http://localhost:${PROXY_PORT}` - process.env.NO_PROXY = '<-loopback>' // proxy everything including localhost - - return systemTests.exec(this, { - spec: 'https_passthru_spec.js', - snapshot: true, - }) - .then(() => { - console.log('connect counts are', connectCounts) - - expect(connectCounts[`localhost:${HTTPS_PORT}`]).to.be.gte(3) - - expect(connectCounts[`localhost:${ERR_HTTPS_PORT}`]).to.be.gte(4) - }) - }) - }) - it('does not connect to the upstream proxy for the SNI server request', function () { const onConnect = sinon.spy(() => { return true From b15d8b7fbe9d7d7b5f22cbb2e32e7a9cfccc8912 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Tue, 7 Dec 2021 10:28:53 -0500 Subject: [PATCH 05/12] Explicitly set retryIntervals for visit/request --- packages/driver/src/cy/commands/navigation.ts | 3 ++- packages/driver/src/cy/commands/request.ts | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/driver/src/cy/commands/navigation.ts b/packages/driver/src/cy/commands/navigation.ts index 881d9759f862..f90afef2a4ef 100644 --- a/packages/driver/src/cy/commands/navigation.ts +++ b/packages/driver/src/cy/commands/navigation.ts @@ -19,7 +19,7 @@ let hasVisitedAboutBlank = null let currentlyVisitingAboutBlank = null let knownCommandCausedInstability = null -const REQUEST_URL_OPTS = 'auth failOnStatusCode retryOnNetworkFailure retryOnStatusCodeFailure method body headers' +const REQUEST_URL_OPTS = 'auth failOnStatusCode retryOnNetworkFailure retryOnStatusCodeFailure retryIntervals method body headers' .split(' ') const VISIT_OPTS = 'url log onBeforeLoad onLoad timeout requestTimeout' @@ -712,6 +712,7 @@ export default (Commands, Cypress, cy, state, config) => { failOnStatusCode: true, retryOnNetworkFailure: true, retryOnStatusCodeFailure: false, + retryIntervals: [0, 0, 500, 1000], method: 'GET', body: null, headers: {}, diff --git a/packages/driver/src/cy/commands/request.ts b/packages/driver/src/cy/commands/request.ts index f40dcae64f8f..5eb8d2bcfe8f 100644 --- a/packages/driver/src/cy/commands/request.ts +++ b/packages/driver/src/cy/commands/request.ts @@ -28,6 +28,7 @@ const REQUEST_DEFAULTS = { timeout: null, followRedirect: true, failOnStatusCode: true, + retryIntervals: [0, 0, 500, 1000], retryOnNetworkFailure: true, retryOnStatusCodeFailure: false, } From cf532de28b4663133524dc8bc5e3d8a16ed85fdf Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Tue, 7 Dec 2021 10:29:03 -0500 Subject: [PATCH 06/12] Update network_error_handling_spec --- system-tests/test/network_error_handling_spec.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/system-tests/test/network_error_handling_spec.js b/system-tests/test/network_error_handling_spec.js index c94ba3218683..140caaae53a0 100644 --- a/system-tests/test/network_error_handling_spec.js +++ b/system-tests/test/network_error_handling_spec.js @@ -383,19 +383,19 @@ describe('e2e network error handling', function () { }).then(({ stdout }) => { // sometimes ,