From 70248ab9c0bb90e0dd5987d64e2199d46f3dc600 Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Sun, 10 Sep 2023 21:41:40 -0500 Subject: [PATCH] fix: prerequest correlation for various retried and cached requests (#27771) --- cli/CHANGELOG.md | 1 + .../proxy/lib/http/response-middleware.ts | 20 ++++++++- packages/proxy/lib/http/util/prerequests.ts | 2 +- .../unit/http/response-middleware.spec.ts | 45 ++++++++++++++++++- .../test/unit/http/util/prerequests.spec.ts | 15 +++++++ .../server/lib/browsers/cdp_automation.ts | 6 +++ .../test/unit/browsers/cdp_automation_spec.ts | 17 +++++++ 7 files changed, 101 insertions(+), 5 deletions(-) diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index d4ad63cde915..d7e90061321c 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -9,6 +9,7 @@ _Released 09/12/2023 (PENDING)_ **Bugfixes:** +- Edge cases where `cy.intercept()` would not properly intercept and asset response bodies would not properly be captured for test replay have been addressed. Addressed in [#27771](https://github.com/cypress-io/cypress/issues/27771). - Fixed an issue where `enter`, `keyup`, and `space` events where not triggering `click` events properly in some versions of Firefox. Addressed in [#27715](https://github.com/cypress-io/cypress/pull/27715). **Dependency Updates:** diff --git a/packages/proxy/lib/http/response-middleware.ts b/packages/proxy/lib/http/response-middleware.ts index a83f92890a34..641c76d846a9 100644 --- a/packages/proxy/lib/http/response-middleware.ts +++ b/packages/proxy/lib/http/response-middleware.ts @@ -145,6 +145,18 @@ const stringifyFeaturePolicy = (policy: any): string => { return pairs.map((directive) => directive.join(' ')).join('; ') } +const requestIdRegEx = /^(.*)-retry-([\d]+)$/ +const getOriginalRequestId = (requestId: string) => { + let originalRequestId = requestId + const match = requestIdRegEx.exec(requestId) + + if (match) { + [, originalRequestId] = match + } + + return originalRequestId +} + const LogResponse: ResponseMiddleware = function () { this.debug('received response %o', { browserPreRequest: _.pick(this.req.browserPreRequest, 'requestId'), @@ -674,8 +686,10 @@ const ClearCyInitialCookie: ResponseMiddleware = function () { const MaybeEndWithEmptyBody: ResponseMiddleware = function () { if (httpUtils.responseMustHaveEmptyBody(this.req, this.incomingRes)) { if (this.protocolManager && this.req.browserPreRequest?.requestId) { + const requestId = getOriginalRequestId(this.req.browserPreRequest.requestId) + this.protocolManager.responseEndedWithEmptyBody({ - requestId: this.req.browserPreRequest.requestId, + requestId, isCached: this.incomingRes.statusCode === 304, }) } @@ -783,9 +797,11 @@ const MaybeRemoveSecurity: ResponseMiddleware = function () { const GzipBody: ResponseMiddleware = async function () { if (this.protocolManager && this.req.browserPreRequest?.requestId) { + const requestId = getOriginalRequestId(this.req.browserPreRequest.requestId) + const span = telemetry.startSpan({ name: 'gzip:body:protocol-notification', parentSpan: this.resMiddlewareSpan, isVerbose }) const resultingStream = this.protocolManager.responseStreamReceived({ - requestId: this.req.browserPreRequest.requestId, + requestId, responseHeaders: this.incomingRes.headers, isAlreadyGunzipped: this.isGunzipped, responseStream: this.incomingResStream, diff --git a/packages/proxy/lib/http/util/prerequests.ts b/packages/proxy/lib/http/util/prerequests.ts index dcf538c9fd23..d598f9397bb6 100644 --- a/packages/proxy/lib/http/util/prerequests.ts +++ b/packages/proxy/lib/http/util/prerequests.ts @@ -138,7 +138,7 @@ export class PreRequests { removePending (requestId: string) { this.pendingPreRequests.removeMatching(({ browserPreRequest }) => { - return browserPreRequest.requestId !== requestId + return (browserPreRequest.requestId.includes('-retry-') && !browserPreRequest.requestId.startsWith(`${requestId}-`)) || (!browserPreRequest.requestId.includes('-retry-') && browserPreRequest.requestId !== requestId) }) } diff --git a/packages/proxy/test/unit/http/response-middleware.spec.ts b/packages/proxy/test/unit/http/response-middleware.spec.ts index b405b8de9abd..d0f9dc075b09 100644 --- a/packages/proxy/test/unit/http/response-middleware.spec.ts +++ b/packages/proxy/test/unit/http/response-middleware.spec.ts @@ -1847,14 +1847,14 @@ describe('http/response-middleware', function () { }) }) - it('calls responseEndedWithEmptyBody on protocolManager if protocolManager present and request is correlated and response must have empty body and response is not cached', function () { + it('calls responseEndedWithEmptyBody on protocolManager if protocolManager present and retried request is correlated and response must have empty body and response is not cached', function () { prepareContext({ protocolManager: { responseEndedWithEmptyBody: responseEndedWithEmptyBodyStub, }, req: { browserPreRequest: { - requestId: '123', + requestId: '123-retry-1', }, }, incomingRes: { @@ -2285,6 +2285,47 @@ describe('http/response-middleware', function () { }) }) + it('calls responseStreamReceived on protocolManager if protocolManager present and retried request is correlated', function () { + const stream = Readable.from(['foo']) + const headers = { 'content-encoding': 'gzip' } + const res = { + on: (event, listener) => {}, + off: (event, listener) => {}, + } + + prepareContext({ + protocolManager: { + responseStreamReceived: responseStreamReceivedStub, + }, + req: { + browserPreRequest: { + requestId: '123-retry-1', + }, + }, + res, + incomingRes: { + headers, + }, + isGunzipped: true, + incomingResStream: stream, + }) + + return testMiddleware([GzipBody], ctx) + .then(() => { + expect(responseStreamReceivedStub).to.be.calledWith( + sinon.match(function (actual) { + expect(actual.requestId).to.equal('123') + expect(actual.responseHeaders).to.equal(headers) + expect(actual.isAlreadyGunzipped).to.equal(true) + expect(actual.responseStream).to.equal(stream) + expect(actual.res).to.equal(res) + + return true + }), + ) + }) + }) + it('does not call responseStreamReceived on protocolManager if protocolManager present and request is not correlated', function () { const stream = Readable.from(['foo']) const headers = { 'content-encoding': 'gzip' } diff --git a/packages/proxy/test/unit/http/util/prerequests.spec.ts b/packages/proxy/test/unit/http/util/prerequests.spec.ts index bfbe9f0e0e32..08b1fd6a9a4a 100644 --- a/packages/proxy/test/unit/http/util/prerequests.spec.ts +++ b/packages/proxy/test/unit/http/util/prerequests.spec.ts @@ -90,4 +90,19 @@ describe('http/util/prerequests', () => { expectPendingCounts(0, 2) }) + + it('removes a pre-request with a matching requestId with retries', () => { + preRequests.addPending({ requestId: '1234', url: 'foo', method: 'GET' } as BrowserPreRequest) + preRequests.addPending({ requestId: '1235', url: 'foo', method: 'GET' } as BrowserPreRequest) + preRequests.addPending({ requestId: '1235-retry-1', url: 'foo', method: 'GET' } as BrowserPreRequest) + preRequests.addPending({ requestId: '1235-retry-2', url: 'foo', method: 'GET' } as BrowserPreRequest) + preRequests.addPending({ requestId: '1235-retry-3', url: 'foo', method: 'GET' } as BrowserPreRequest) + preRequests.addPending({ requestId: '1236', url: 'foo', method: 'GET' } as BrowserPreRequest) + + expectPendingCounts(0, 6) + + preRequests.removePending('1235') + + expectPendingCounts(0, 2) + }) }) diff --git a/packages/server/lib/browsers/cdp_automation.ts b/packages/server/lib/browsers/cdp_automation.ts index 6f37150298b9..818e81942140 100644 --- a/packages/server/lib/browsers/cdp_automation.ts +++ b/packages/server/lib/browsers/cdp_automation.ts @@ -251,6 +251,12 @@ export class CdpAutomation implements CDPClient { } private onResponseReceived = (params: Protocol.Network.ResponseReceivedEvent) => { + if (params.response.fromDiskCache) { + this.automation.onRequestServedFromCache?.(params.requestId) + + return + } + const browserResponseReceived: BrowserResponseReceived = { requestId: params.requestId, status: params.response.status, diff --git a/packages/server/test/unit/browsers/cdp_automation_spec.ts b/packages/server/test/unit/browsers/cdp_automation_spec.ts index ade10b8513b0..9021b9b63d92 100644 --- a/packages/server/test/unit/browsers/cdp_automation_spec.ts +++ b/packages/server/test/unit/browsers/cdp_automation_spec.ts @@ -181,6 +181,23 @@ context('lib/browsers/cdp_automation', () => { }, ) }) + + it('cleans up prerequests when response is cached from disk', function () { + const browserResponseReceived = { + requestId: '0', + response: { + status: 200, + headers: {}, + fromDiskCache: true, + }, + } + + this.onFn + .withArgs('Network.responseReceived') + .yield(browserResponseReceived) + + expect(this.automation.onRequestEvent).not.to.have.been.called + }) }) describe('.onRequestServedFromCache', function () {