From c672581a719e331eaea6fa54cb8bf4ba8606b8f2 Mon Sep 17 00:00:00 2001 From: Matt Schile Date: Mon, 22 Jan 2024 15:35:49 -0700 Subject: [PATCH] perf: fix proxy correlation timeout issues (#28751) --- .circleci/workflows.yml | 16 +-- cli/CHANGELOG.md | 6 ++ .../cypress/e2e/commands/navigation.cy.js | 36 ++----- .../driver/cypress/e2e/issues/28545.cy.ts | 38 ------- packages/driver/src/cy/commands/navigation.ts | 2 +- packages/proxy/lib/http/index.ts | 21 ++-- packages/proxy/lib/http/request-middleware.ts | 16 ++- packages/proxy/lib/http/util/prerequests.ts | 6 +- packages/proxy/lib/network-proxy.ts | 6 +- packages/proxy/test/unit/http/index.spec.ts | 79 +++++++++++++- .../test/unit/http/request-middleware.spec.ts | 102 ++++++++++++++++++ packages/server/lib/project-base.ts | 3 + packages/server/lib/server-base.ts | 10 +- packages/server/lib/socket-base.ts | 2 +- system-tests/lib/system-tests.ts | 36 ++++--- .../e2e/cypress/e2e/proxy_correlation.cy.js | 62 +++++++++++ system-tests/test/proxy_correlation_spec.js | 76 +++++++++++++ 17 files changed, 406 insertions(+), 111 deletions(-) delete mode 100644 packages/driver/cypress/e2e/issues/28545.cy.ts create mode 100644 system-tests/projects/e2e/cypress/e2e/proxy_correlation.cy.js create mode 100644 system-tests/test/proxy_correlation_spec.js diff --git a/.circleci/workflows.yml b/.circleci/workflows.yml index b852f9f140..ddf103eef6 100644 --- a/.circleci/workflows.yml +++ b/.circleci/workflows.yml @@ -29,9 +29,7 @@ mainBuildFilters: &mainBuildFilters - develop - /^release\/\d+\.\d+\.\d+$/ # use the following branch as well to ensure that v8 snapshot cache updates are fully tested - - 'fix/set_module_resolution_with_commonjs' - - 'publish-binary' - - 'em/circle2' + - 'update-v8-snapshot-cache-on-develop' # usually we don't build Mac app - it takes a long time # but sometimes we want to really confirm we are doing the right thing @@ -42,8 +40,7 @@ macWorkflowFilters: &darwin-workflow-filters - equal: [ develop, << pipeline.git.branch >> ] # use the following branch as well to ensure that v8 snapshot cache updates are fully tested - equal: [ 'update-v8-snapshot-cache-on-develop', << pipeline.git.branch >> ] - - equal: [ 'fix/set_module_resolution_with_commonjs', << pipeline.git.branch >> ] - - equal: [ 'ryanm/fix/service-worker-capture', << pipeline.git.branch >> ] + - equal: [ 'mschile/protocol/proxy_correlation', << pipeline.git.branch >> ] - matches: pattern: /^release\/\d+\.\d+\.\d+$/ value: << pipeline.git.branch >> @@ -54,8 +51,7 @@ linuxArm64WorkflowFilters: &linux-arm64-workflow-filters - equal: [ develop, << pipeline.git.branch >> ] # use the following branch as well to ensure that v8 snapshot cache updates are fully tested - equal: [ 'update-v8-snapshot-cache-on-develop', << pipeline.git.branch >> ] - - equal: [ 'fix/set_module_resolution_with_commonjs', << pipeline.git.branch >> ] - - equal: [ 'em/circle2', << pipeline.git.branch >> ] + - equal: [ 'mschile/protocol/proxy_correlation', << pipeline.git.branch >> ] - matches: pattern: /^release\/\d+\.\d+\.\d+$/ value: << pipeline.git.branch >> @@ -78,9 +74,7 @@ windowsWorkflowFilters: &windows-workflow-filters - equal: [ develop, << pipeline.git.branch >> ] # use the following branch as well to ensure that v8 snapshot cache updates are fully tested - equal: [ 'update-v8-snapshot-cache-on-develop', << pipeline.git.branch >> ] - - equal: [ 'fix/set_module_resolution_with_commonjs', << pipeline.git.branch >> ] - - equal: [ 'lerna-optimize-tasks', << pipeline.git.branch >> ] - - equal: [ 'mschile/mochaEvents_win_sep', << pipeline.git.branch >> ] + - equal: [ 'mschile/protocol/proxy_correlation', << pipeline.git.branch >> ] - matches: pattern: /^release\/\d+\.\d+\.\d+$/ value: << pipeline.git.branch >> @@ -150,7 +144,7 @@ commands: name: Set environment variable to determine whether or not to persist artifacts command: | echo "Setting SHOULD_PERSIST_ARTIFACTS variable" - echo 'if ! [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "publish-binary" && "$CIRCLE_BRANCH" != "fix/set_module_resolution_with_commonjs" ]]; then + echo 'if ! [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "mschile/protocol/proxy_correlation" ]]; then export SHOULD_PERSIST_ARTIFACTS=true fi' >> "$BASH_ENV" # You must run `setup_should_persist_artifacts` command and be using bash before running this command diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 9ebea2a9bf..dbebd3805d 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -1,4 +1,10 @@ +## 13.6.4 + +**Performance:** + +- Fixed a performance regression from [`13.3.2`](https://docs.cypress.io/guides/references/changelog#13.3.2) where aborted requests may not correlate correctly. Fixes [#28734](https://github.com/cypress-io/cypress/issues/28734). + ## 13.6.3 _Released 1/16/2024_ diff --git a/packages/driver/cypress/e2e/commands/navigation.cy.js b/packages/driver/cypress/e2e/commands/navigation.cy.js index ba1252da74..51c0ae9e34 100644 --- a/packages/driver/cypress/e2e/commands/navigation.cy.js +++ b/packages/driver/cypress/e2e/commands/navigation.cy.js @@ -2779,37 +2779,15 @@ describe('src/cy/commands/navigation', () => { }) context('resets state', () => { - context('test isolation on', { testIsolation: true }, () => { - it('resets the server state', () => { - cy.stub(Cypress, 'backend').log(false).callThrough() + it('resets the server state', () => { + cy.stub(Cypress, 'backend').log(false).callThrough() - Cypress.emitThen('test:before:run:async', { - id: 'r1', - currentRetry: 1, - }) - .then(() => { - expect(Cypress.backend).to.be.calledWith( - 'reset:server:state', - { testIsolation: true }, - ) - }) + Cypress.emitThen('test:before:run:async', { + id: 'r1', + currentRetry: 1, }) - }) - - context('test isolation off', { testIsolation: false }, () => { - it('resets the server state', () => { - cy.stub(Cypress, 'backend').log(false).callThrough() - - Cypress.emitThen('test:before:run:async', { - id: 'r1', - currentRetry: 1, - }) - .then(() => { - expect(Cypress.backend).to.be.calledWith( - 'reset:server:state', - { testIsolation: false }, - ) - }) + .then(() => { + expect(Cypress.backend).to.be.calledWith('reset:server:state') }) }) }) diff --git a/packages/driver/cypress/e2e/issues/28545.cy.ts b/packages/driver/cypress/e2e/issues/28545.cy.ts deleted file mode 100644 index 0d0ec7bc7f..0000000000 --- a/packages/driver/cypress/e2e/issues/28545.cy.ts +++ /dev/null @@ -1,38 +0,0 @@ -// https://github.com/cypress-io/cypress/issues/28545 - -describe('lots of requests', () => { - beforeEach(() => { - cy.intercept('/lots-of-requests', (req) => { - req.reply( - ` - - Lots of Requests - - - - - `, - ) - }) - }) - - describe('test isolation off', { testIsolation: false }, () => { - it('test 1', () => { - cy.visit('http://localhost:3500/lots-of-requests') - }) - - it('test 2', () => { - cy.get('#done').should('contain', 'Done') - }) - }) -}) diff --git a/packages/driver/src/cy/commands/navigation.ts b/packages/driver/src/cy/commands/navigation.ts index 85ad1fb322..7028f49175 100644 --- a/packages/driver/src/cy/commands/navigation.ts +++ b/packages/driver/src/cy/commands/navigation.ts @@ -452,7 +452,7 @@ export default (Commands, Cypress, cy, state, config) => { // reset any state on the backend // TODO: this is a bug in e2e it needs to be returned - return Cypress.backend('reset:server:state', { testIsolation: Cypress.config('testIsolation') }) + return Cypress.backend('reset:server:state') }) Cypress.on('test:before:run', reset) diff --git a/packages/proxy/lib/http/index.ts b/packages/proxy/lib/http/index.ts index 216b668247..54161495c7 100644 --- a/packages/proxy/lib/http/index.ts +++ b/packages/proxy/lib/http/index.ts @@ -371,10 +371,14 @@ export class Http { } ctx.error = error - if (ctx.req.browserPreRequest && !ctx.req.browserPreRequest.errorHandled) { + + // if there is a pre-request and the error has not been handled and the response has not been destroyed + // (which implies the request was canceled by the browser), try to re-use the pre-request for the next retry + // + // browsers will retry requests in the event of network errors, but they will not send pre-requests, + // so try to re-use the current browserPreRequest for the next retry after incrementing the ID. + if (ctx.req.browserPreRequest && !ctx.req.browserPreRequest.errorHandled && !ctx.res.destroyed) { ctx.req.browserPreRequest.errorHandled = true - // browsers will retry requests in the event of network errors, but they will not send pre-requests, - // so try to re-use the current browserPreRequest for the next retry after incrementing the ID. const preRequest = { ...ctx.req.browserPreRequest, requestId: getUniqueRequestId(ctx.req.browserPreRequest.requestId), @@ -448,15 +452,12 @@ export class Http { } } - reset (options: { resetPreRequests: boolean, resetBetweenSpecs: boolean }) { + reset (options: { resetBetweenSpecs: boolean }) { this.buffers.reset() this.setAUTUrl(undefined) - if (options.resetPreRequests) { - this.preRequests.reset() - } - if (options.resetBetweenSpecs) { + this.preRequests.reset() this.serviceWorkerManager = new ServiceWorkerManager() } } @@ -477,6 +478,10 @@ export class Http { this.preRequests.removePendingPreRequest(requestId) } + getPendingBrowserPreRequests () { + return this.preRequests.pendingPreRequests + } + addPendingUrlWithoutPreRequest (url: string) { this.preRequests.addPendingUrlWithoutPreRequest(url) } diff --git a/packages/proxy/lib/http/request-middleware.ts b/packages/proxy/lib/http/request-middleware.ts index 8f8689aed8..3477db503c 100644 --- a/packages/proxy/lib/http/request-middleware.ts +++ b/packages/proxy/lib/http/request-middleware.ts @@ -100,13 +100,25 @@ const CorrelateBrowserPreRequest: RequestMiddleware = async function () { shouldCorrelatePreRequest: shouldCorrelatePreRequests, }) - if (!this.shouldCorrelatePreRequests()) { + if (!shouldCorrelatePreRequests) { span?.end() return this.next() } + const onClose = () => { + // if we haven't matched a browser pre-request and the request has been destroyed, raise an error + if (this.req.destroyed) { + span?.end() + this.reqMiddlewareSpan?.end() + + this.onError(new Error('request destroyed before browser pre-request was received')) + } + } + const copyResourceTypeAndNext = () => { + this.res.off('close', onClose) + this.req.resourceType = this.req.browserPreRequest?.resourceType span?.setAttributes({ @@ -144,6 +156,8 @@ const CorrelateBrowserPreRequest: RequestMiddleware = async function () { return copyResourceTypeAndNext() } + this.res.once('close', onClose) + this.debug('waiting for prerequest') this.pendingRequest = this.getPreRequest((({ browserPreRequest, noPreRequestExpected }) => { this.req.browserPreRequest = browserPreRequest diff --git a/packages/proxy/lib/http/util/prerequests.ts b/packages/proxy/lib/http/util/prerequests.ts index 443e810915..2016dd71a7 100644 --- a/packages/proxy/lib/http/util/prerequests.ts +++ b/packages/proxy/lib/http/util/prerequests.ts @@ -275,6 +275,7 @@ export class PreRequests { proxyRequestReceivedTimestamp: performance.now() + performance.timeOrigin, timeout: setTimeout(() => { ctxDebug('Never received pre-request or url without pre-request for request %s after waiting %sms. Continuing without one.', key, this.requestTimeout) + debug('Never received pre-request or url without pre-request for request %s after waiting %sms. Continuing without one.', key, this.requestTimeout) metrics.unmatchedRequests++ pendingRequest.timedOut = true callback({ @@ -308,7 +309,10 @@ export class PreRequests { this.pendingPreRequests = new QueueMap() // Clear out the pending requests timeout callbacks first then clear the queue - this.pendingRequests.forEach(({ callback, timeout }) => { + this.pendingRequests.forEach(({ callback, timeout, timedOut }) => { + // If the request has already timed out, just return + if (timedOut) return + clearTimeout(timeout) metrics.unmatchedRequests++ callback?.({ diff --git a/packages/proxy/lib/network-proxy.ts b/packages/proxy/lib/network-proxy.ts index 22306930cd..2498d2efa1 100644 --- a/packages/proxy/lib/network-proxy.ts +++ b/packages/proxy/lib/network-proxy.ts @@ -18,6 +18,10 @@ export class NetworkProxy { this.http.removePendingBrowserPreRequest(requestId) } + getPendingBrowserPreRequests () { + return this.http.getPendingBrowserPreRequests() + } + addPendingUrlWithoutPreRequest (url: string) { this.http.addPendingUrlWithoutPreRequest(url) } @@ -59,7 +63,7 @@ export class NetworkProxy { this.http.setBuffer(buffer) } - reset (options: { resetPreRequests: boolean, resetBetweenSpecs: boolean } = { resetPreRequests: true, resetBetweenSpecs: false }) { + reset (options: { resetBetweenSpecs: boolean } = { resetBetweenSpecs: false }) { this.http.reset(options) } diff --git a/packages/proxy/test/unit/http/index.spec.ts b/packages/proxy/test/unit/http/index.spec.ts index 08c25230ae..f5b6edc9bf 100644 --- a/packages/proxy/test/unit/http/index.spec.ts +++ b/packages/proxy/test/unit/http/index.spec.ts @@ -80,6 +80,44 @@ describe('http', function () { }) }) + it('creates fake pending browser pre request', function () { + incomingRequest.callsFake(function () { + this.req.browserPreRequest = { + requestId: '1234', + errorHandled: false, + } + + this.res.destroyed = false + + throw new Error('oops') + }) + + error.callsFake(function () { + expect(this.error.message).to.eq('Internal error while proxying "GET url" in 0:\noops') + this.end() + }) + + const http = new Http(httpOpts) + + http.addPendingBrowserPreRequest = sinon.stub() + + return http + // @ts-expect-error + .handleHttpRequest({ method: 'GET', proxiedUrl: 'url' }, { on, off }) + .then(function () { + expect(incomingRequest).to.be.calledOnce + expect(incomingResponse).to.not.be.called + expect(error).to.be.calledOnce + expect(http.addPendingBrowserPreRequest).to.be.calledOnceWith({ + requestId: '1234-retry-1', + errorHandled: false, + }) + + expect(on).to.not.be.called + expect(off).to.be.calledThrice + }) + }) + it('ensures not to create fake pending browser pre requests on multiple errors', function () { incomingRequest.callsFake(function () { this.req.browserPreRequest = { @@ -111,6 +149,39 @@ describe('http', function () { }) }) + it('does not create fake pending browser pre request when the response is destroyed', function () { + incomingRequest.callsFake(function () { + this.req.browserPreRequest = { + errorHandled: false, + } + + this.res.destroyed = true + + throw new Error('oops') + }) + + error.callsFake(function () { + expect(this.error.message).to.eq('Internal error while proxying "GET url" in 0:\noops') + this.end() + }) + + const http = new Http(httpOpts) + + http.addPendingBrowserPreRequest = sinon.stub() + + return http + // @ts-expect-error + .handleHttpRequest({ method: 'GET', proxiedUrl: 'url' }, { on, off }) + .then(function () { + expect(incomingRequest).to.be.calledOnce + expect(incomingResponse).to.not.be.called + expect(error).to.be.calledOnce + expect(http.addPendingBrowserPreRequest).to.not.be.called + expect(on).to.not.be.called + expect(off).to.be.calledThrice + }) + }) + it('moves to Error stack if err in IncomingResponse', function () { incomingRequest.callsFake(function () { this.incomingRes = {} @@ -214,22 +285,22 @@ describe('http', function () { httpOpts = { config: {}, middleware: {} } }) - it('resets preRequests when resetPreRequests is true', function () { + it('resets preRequests when resetBetweenSpecs is true', function () { const http = new Http(httpOpts) http.preRequests.reset = sinon.stub() - http.reset({ resetPreRequests: true, resetBetweenSpecs: false }) + http.reset({ resetBetweenSpecs: true }) expect(http.preRequests.reset).to.be.calledOnce }) - it('does not reset preRequests when resetPreRequests is false', function () { + it('does not reset preRequests when resetBetweenSpecs is false', function () { const http = new Http(httpOpts) http.preRequests.reset = sinon.stub() - http.reset({ resetPreRequests: false, resetBetweenSpecs: false }) + http.reset({ resetBetweenSpecs: false }) expect(http.preRequests.reset).to.not.be.called }) diff --git a/packages/proxy/test/unit/http/request-middleware.spec.ts b/packages/proxy/test/unit/http/request-middleware.spec.ts index 25df005bbc..604dbcffd6 100644 --- a/packages/proxy/test/unit/http/request-middleware.spec.ts +++ b/packages/proxy/test/unit/http/request-middleware.spec.ts @@ -738,6 +738,108 @@ describe('http/request-middleware', () => { }) }) + describe('CorrelateBrowserPreRequest', () => { + const { CorrelateBrowserPreRequest } = RequestMiddleware + + it('skips if shouldCorrelatePreRequests returns false', async () => { + const ctx = { + res: { + off: sinon.stub(), + }, + shouldCorrelatePreRequests: () => false, + getPreRequest: sinon.stub(), + } + + await testMiddleware([CorrelateBrowserPreRequest], ctx) + .then(() => { + expect(ctx.getPreRequest).not.to.be.called + }) + }) + + it('sets browserPreRequest on the request', async () => { + const browserPreRequest = sinon.stub() + + const ctx = { + req: { + proxiedUrl: 'https://www.cypress.io/', + browserPreRequest: undefined, + headers: [], + }, + res: { + off: sinon.stub(), + once: sinon.stub(), + }, + shouldCorrelatePreRequests: () => true, + getPreRequest: sinon.stub().yields({ + browserPreRequest, + }), + } + + await testMiddleware([CorrelateBrowserPreRequest], ctx) + .then(() => { + expect(ctx.getPreRequest).to.be.calledOnce + expect(ctx.req.browserPreRequest).to.equal(browserPreRequest) + expect(ctx.res.once).to.be.calledWith('close') + expect(ctx.res.off).to.be.calledWith('close') + }) + }) + + it('sets noPreRequestExpected on the request', async () => { + const ctx = { + req: { + proxiedUrl: 'https://www.cypress.io/', + browserPreRequest: undefined, + noPreRequestExpected: undefined, + headers: [], + }, + res: { + off: sinon.stub(), + once: sinon.stub(), + }, + shouldCorrelatePreRequests: () => true, + getPreRequest: sinon.stub().yields({ + noPreRequestExpected: true, + }), + } + + await testMiddleware([CorrelateBrowserPreRequest], ctx) + .then(() => { + expect(ctx.getPreRequest).to.be.calledOnce + expect(ctx.req.noPreRequestExpected).to.be.true + expect(ctx.res.once).to.be.calledWith('close') + expect(ctx.res.off).to.be.calledWith('close') + }) + }) + + it('errors when the request is destroyed prior to receiving a pre-request', () => { + const ctx = { + req: { + proxiedUrl: 'https://www.cypress.io/', + destroyed: true, + browserPreRequest: undefined, + noPreRequestExpected: undefined, + headers: [], + }, + res: { + off: sinon.stub(), + once: sinon.stub(), + }, + shouldCorrelatePreRequests: () => true, + getPreRequest: sinon.stub(), + onError: sinon.stub(), + } + + testMiddleware([CorrelateBrowserPreRequest], ctx) + ctx.res.once.callArg(1) + + expect(ctx.getPreRequest).to.be.calledOnce + expect(ctx.req.noPreRequestExpected).to.be.undefined + expect(ctx.req.browserPreRequest).to.be.undefined + expect(ctx.res.once).to.be.calledWith('close') + expect(ctx.onError).to.be.calledOnce + }) + }) + describe('SendRequestOutgoing', () => { const { SendRequestOutgoing } = RequestMiddleware diff --git a/packages/server/lib/project-base.ts b/packages/server/lib/project-base.ts index 74499c0c1b..38042a032b 100644 --- a/packages/server/lib/project-base.ts +++ b/packages/server/lib/project-base.ts @@ -50,6 +50,7 @@ export interface Cfg extends ReceivedCypressOptions { const localCwd = process.cwd() const debug = Debug('cypress:server:project') +const debugVerbose = Debug('cypress-verbose:server:project') type StartWebsocketOptions = Pick @@ -415,6 +416,8 @@ export class ProjectBase extends EE { reporterInstance.emit(event, runnable) if (event === 'test:before:run') { + debugVerbose('browserPreRequests prior to running %s: %O', runnable.title, this.server.getBrowserPreRequests()) + this.emit('test:before:run', { runnable, previousResults: reporterInstance?.results() || {}, diff --git a/packages/server/lib/server-base.ts b/packages/server/lib/server-base.ts index 0b00f96e37..494847ac5e 100644 --- a/packages/server/lib/server-base.ts +++ b/packages/server/lib/server-base.ts @@ -472,8 +472,8 @@ export class ServerBase { options.getRenderedHTMLOrigins = this._networkProxy?.http.getRenderedHTMLOrigins options.getCurrentBrowser = () => this.getCurrentBrowser?.() - options.onResetServerState = (options: { testIsolation: boolean }) => { - this.networkProxy.reset({ resetPreRequests: !!options.testIsolation, resetBetweenSpecs: false }) + options.onResetServerState = () => { + this.networkProxy.reset({ resetBetweenSpecs: false }) this.netStubbingState.reset() this._remoteStates.reset() this.resourceTypeAndCredentialManager.clear() @@ -500,6 +500,10 @@ export class ServerBase { this.networkProxy.removePendingBrowserPreRequest(requestId) } + getBrowserPreRequests () { + return this._networkProxy?.getPendingBrowserPreRequests() + } + emitRequestEvent (eventName, data) { this.socket.toDriver('request:event', eventName, data) } @@ -630,7 +634,7 @@ export class ServerBase { } reset () { - this._networkProxy?.reset({ resetPreRequests: true, resetBetweenSpecs: true }) + this._networkProxy?.reset({ resetBetweenSpecs: true }) this.resourceTypeAndCredentialManager.clear() const baseUrl = this._baseUrl ?? '' diff --git a/packages/server/lib/socket-base.ts b/packages/server/lib/socket-base.ts index c47a401f47..4b165f445a 100644 --- a/packages/server/lib/socket-base.ts +++ b/packages/server/lib/socket-base.ts @@ -412,7 +412,7 @@ export class SocketBase { case 'http:request': return options.onRequest(userAgent, automationRequest, args[0]) case 'reset:server:state': - return options.onResetServerState(args[0]) + return options.onResetServerState() case 'log:memory:pressure': return firefoxUtil.log() case 'firefox:force:gc': diff --git a/system-tests/lib/system-tests.ts b/system-tests/lib/system-tests.ts index 530b388ff3..e5566d2708 100644 --- a/system-tests/lib/system-tests.ts +++ b/system-tests/lib/system-tests.ts @@ -143,13 +143,17 @@ type ExecOptions = { */ snapshot?: boolean /** - * By default strip ansi codes from stdout. Pass false to turn off. + * By default strip ansi codes from stdout/stderr. Pass false to turn off. */ stripAnsi?: boolean /** * Pass a function to assert on and/or modify the stdout before snapshotting. */ onStdout?: (stdout: string) => string | void + /** + * Pass a function to assert on and/or modify the stderr. + */ + onStderr?: (stderr: string) => string | void /** * Pass a function to receive the spawned process as an argument. */ @@ -857,24 +861,30 @@ const systemTests = { }) if (options.stripAnsi) { - // always strip ansi from stdout before yielding + // always strip ansi from stdout/stderr before yielding // it to any callback functions stdout = stripAnsi(stdout) + stderr = stripAnsi(stderr) + } + + if (options.onStdout) { + const newStdout = options.onStdout(stdout) + + if (newStdout && _.isString(newStdout)) { + stdout = newStdout + } + } + + if (options.onStderr) { + const newStderr = options.onStderr(stderr) + + if (newStderr && _.isString(newStderr)) { + stderr = newStderr + } } // snapshot the stdout! if (options.snapshot) { - // enable callback to modify stdout - const ostd = options.onStdout - - if (ostd) { - const newStdout = ostd(stdout) - - if (newStdout && _.isString(newStdout)) { - stdout = newStdout - } - } - // if we have browser in the stdout make // sure its legit const matches = browserNameVersionRe.exec(stdout) diff --git a/system-tests/projects/e2e/cypress/e2e/proxy_correlation.cy.js b/system-tests/projects/e2e/cypress/e2e/proxy_correlation.cy.js new file mode 100644 index 0000000000..29839b3789 --- /dev/null +++ b/system-tests/projects/e2e/cypress/e2e/proxy_correlation.cy.js @@ -0,0 +1,62 @@ +describe('lots of requests', () => { + describe('test isolation', () => { + describe('test isolation on', { testIsolation: true }, () => { + it('test 1', () => { + cy.visit('/lots-of-requests?test=1&i=1') + }) + + it('test 2', () => { + cy.visit('/lots-of-requests?test=2&i=1') + cy.get('#done').should('contain', 'Done') + }) + }) + + describe('test isolation off', { testIsolation: false }, () => { + it('test 3', () => { + cy.visit('/lots-of-requests?test=3&i=1') + }) + + it('test 4', () => { + cy.get('#done').should('contain', 'Done') + }) + }) + + describe('test isolation back on', { testIsolation: true }, () => { + it('test 5', () => { + cy.visit('/lots-of-requests?test=5&i=1') + }) + + it('test 6', () => { + cy.visit('/lots-of-requests?test=6&i=1') + cy.get('#done').should('contain', 'Done') + }) + }) + }) + + describe('multiple visits in one test', { testIsolation: true }, () => { + it('test 7', () => { + cy.visit('/lots-of-requests?test=7&i=1') + cy.visit('/lots-of-requests?test=7&i=2') + cy.get('#done').should('contain', 'Done') + }) + }) + + describe('navigation in one test', { testIsolation: true }, () => { + it('test 8', () => { + cy.visit('/lots-of-requests?test=8&i=1') + cy.get('a').click() + cy.get('#done').should('contain', 'Done') + }) + }) + + describe('network error', { testIsolation: true, browser: '!webkit' }, () => { + beforeEach(() => { + cy.intercept('GET', '/1mb?test=9&i=1&j=8', { forceNetworkError: true }) + }) + + it('test 9', () => { + cy.visit('/lots-of-requests?test=9&i=1') + cy.get('#done').should('contain', 'Done') + }) + }) +}) diff --git a/system-tests/test/proxy_correlation_spec.js b/system-tests/test/proxy_correlation_spec.js new file mode 100644 index 0000000000..c4fbd59292 --- /dev/null +++ b/system-tests/test/proxy_correlation_spec.js @@ -0,0 +1,76 @@ +const systemTests = require('../lib/system-tests').default + +const onServer = (app) => { + app.get('/lots-of-requests', (req, res) => { + const test = req.query.test + const i = req.query.i + + res.send( + ` + + Lots of Requests + + + + Visit + + + `, + ) + }) + + app.get('/1mb', (req, res) => { + return res.type('text').send('x'.repeat(1024 * 1024)) + }) +} + +describe('e2e proxy correlation spec', () => { + const timedOutRequests = (stderr) => { + const matches = stderr.matchAll(/Never received pre-request or url without pre-request for request (.*) after waiting/g) + + // filter out all non-localhost requests since we only care about ones that came from the app, + // browsers make requests that don't have pre-requests for various reasons + // e.g. https://clientservices.googleapis.com/* and https://accounts.google.com/* in chrome + // https://firefox.settings.services.mozilla.com/v1/ and https://tracking-protection.cdn.mozilla.net/* in firefox + return [...matches].filter((match) => match[1].includes('localhost')).map((match) => match[1]) + } + + systemTests.setup({ + servers: { + port: 3500, + onServer, + }, + settings: { + e2e: { + baseUrl: 'http://localhost:3500', + }, + }, + }) + + systemTests.it('correctly correlates requests', { + spec: 'proxy_correlation.cy.js', + processEnv: { + DEBUG: 'cypress:proxy:http:util:prerequests', + }, + config: { + experimentalWebKitSupport: true, + defaultCommandTimeout: 10000, + }, + onStderr (stderr) { + const requests = timedOutRequests(stderr) + + expect(requests).to.be.empty + }, + }) +})