diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index c4d6625b3d02..ea500ae914d3 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -8,6 +8,7 @@ _Released 10/03/2023 (PENDING)_ - Fixed an issue where requests were correlated in the wrong order in the proxy. This could cause an issue where the wrong request is used for `cy.intercept` or assets (e.g. stylesheets or images) may not properly be available in Test Replay. Addressed in [#27892](https://github.com/cypress-io/cypress/pull/27892). - Fixed an issue where a crashed Chrome renderer can cause the Test Replay recorder to hang. Addressed in [#27909](https://github.com/cypress-io/cypress/pull/27909). - Fixed an issue where multiple responses yielded from calls to `cy.wait()` would sometimes be out of order. Fixes [#27337](https://github.com/cypress-io/cypress/issues/27337). +- Fixed an issue where requests were timing out in the proxy. This could cause an issue where the wrong request is used for `cy.intercept` or assets (e.g. stylesheets or images) may not properly be available in Test Replay. Addressed in [#27976](https://github.com/cypress-io/cypress/pull/27976) - Fixed an issue where Test Replay couldn't record tests due to issues involving `GLIBC`. Fixed deprecation warnings during the rebuild of better-sqlite3. Fixes [#27891](https://github.com/cypress-io/cypress/issues/27891) and [#27902](https://github.com/cypress-io/cypress/issues/27902). - Enables test replay for executed specs in runs that have a spec that causes a browser crash. Addressed in [#27786](https://github.com/cypress-io/cypress/pull/27786) diff --git a/packages/proxy/lib/http/index.ts b/packages/proxy/lib/http/index.ts index f6436f9391b9..433881759c05 100644 --- a/packages/proxy/lib/http/index.ts +++ b/packages/proxy/lib/http/index.ts @@ -425,7 +425,12 @@ export class Http { this.preRequests.removePending(requestId) } + addPendingUrlWithoutPreRequest (url: string) { + this.preRequests.addPendingUrlWithoutPreRequest(url) + } + setProtocolManager (protocolManager: ProtocolManagerShape) { this.protocolManager = protocolManager + this.preRequests.setProtocolManager(protocolManager) } } diff --git a/packages/proxy/lib/http/response-middleware.ts b/packages/proxy/lib/http/response-middleware.ts index 641c76d846a9..c1e534ceaa14 100644 --- a/packages/proxy/lib/http/response-middleware.ts +++ b/packages/proxy/lib/http/response-middleware.ts @@ -691,6 +691,13 @@ const MaybeEndWithEmptyBody: ResponseMiddleware = function () { this.protocolManager.responseEndedWithEmptyBody({ requestId, isCached: this.incomingRes.statusCode === 304, + timings: { + cdpRequestWillBeSentTimestamp: this.req.browserPreRequest.cdpRequestWillBeSentTimestamp, + cdpRequestWillBeSentReceivedTimestamp: this.req.browserPreRequest.cdpRequestWillBeSentReceivedTimestamp, + proxyRequestReceivedTimestamp: this.req.browserPreRequest.proxyRequestReceivedTimestamp, + cdpLagDuration: this.req.browserPreRequest.cdpLagDuration, + proxyRequestCorrelationDuration: this.req.browserPreRequest.proxyRequestCorrelationDuration, + }, }) } @@ -797,15 +804,24 @@ const MaybeRemoveSecurity: ResponseMiddleware = function () { const GzipBody: ResponseMiddleware = async function () { if (this.protocolManager && this.req.browserPreRequest?.requestId) { - const requestId = getOriginalRequestId(this.req.browserPreRequest.requestId) + const preRequest = this.req.browserPreRequest + const requestId = getOriginalRequestId(preRequest.requestId) const span = telemetry.startSpan({ name: 'gzip:body:protocol-notification', parentSpan: this.resMiddlewareSpan, isVerbose }) + const resultingStream = this.protocolManager.responseStreamReceived({ requestId, responseHeaders: this.incomingRes.headers, isAlreadyGunzipped: this.isGunzipped, responseStream: this.incomingResStream, res: this.res, + timings: { + cdpRequestWillBeSentTimestamp: preRequest.cdpRequestWillBeSentTimestamp, + cdpRequestWillBeSentReceivedTimestamp: preRequest.cdpRequestWillBeSentReceivedTimestamp, + proxyRequestReceivedTimestamp: preRequest.proxyRequestReceivedTimestamp, + cdpLagDuration: preRequest.cdpLagDuration, + proxyRequestCorrelationDuration: preRequest.proxyRequestCorrelationDuration, + }, }) if (resultingStream) { diff --git a/packages/proxy/lib/http/util/prerequests.ts b/packages/proxy/lib/http/util/prerequests.ts index 2062d66967be..ee2dc2cf3e1a 100644 --- a/packages/proxy/lib/http/util/prerequests.ts +++ b/packages/proxy/lib/http/util/prerequests.ts @@ -1,7 +1,9 @@ import type { CypressIncomingRequest, BrowserPreRequest, + BrowserPreRequestWithTimings, } from '@packages/proxy' +import type { ProtocolManagerShape } from '@packages/types' import Debug from 'debug' const debug = Debug('cypress:proxy:http:util:prerequests') @@ -19,16 +21,23 @@ process.once('exit', () => { debug('metrics: %o', metrics) }) -export type GetPreRequestCb = (browserPreRequest?: BrowserPreRequest) => void +export type GetPreRequestCb = (browserPreRequest?: BrowserPreRequestWithTimings) => void type PendingRequest = { ctxDebug callback: GetPreRequestCb timeout: NodeJS.Timeout + timedOut?: boolean + proxyRequestReceivedTimestamp: number } type PendingPreRequest = { browserPreRequest: BrowserPreRequest + cdpRequestWillBeSentTimestamp: number + cdpRequestWillBeSentReceivedTimestamp: number +} + +type PendingUrlWithoutPreRequest = { timestamp: number } @@ -81,12 +90,14 @@ export class PreRequests { sweepInterval: number pendingPreRequests = new QueueMap() pendingRequests = new QueueMap() + pendingUrlsWithoutPreRequests = new QueueMap() sweepIntervalTimer: NodeJS.Timeout + protocolManager?: ProtocolManagerShape constructor ( - requestTimeout = 500, + requestTimeout = 2000, // 10 seconds - sweepInterval = 1000 * 10, + sweepInterval = 10000, ) { // If a request comes in and we don't have a matching pre-request after this timeout, // we invoke the request callback to tell the server to proceed (we don't want to block @@ -103,8 +114,8 @@ export class PreRequests { this.sweepIntervalTimer = setInterval(() => { const now = Date.now() - this.pendingPreRequests.removeMatching(({ timestamp, browserPreRequest }) => { - if (timestamp + this.sweepInterval < now) { + this.pendingPreRequests.removeMatching(({ cdpRequestWillBeSentReceivedTimestamp, browserPreRequest }) => { + if (cdpRequestWillBeSentReceivedTimestamp + this.sweepInterval < now) { debugVerbose('timed out unmatched pre-request: %o', browserPreRequest) metrics.unmatchedPreRequests++ @@ -113,6 +124,10 @@ export class PreRequests { return true }) + + this.pendingUrlsWithoutPreRequests.removeMatching(({ timestamp }) => { + return timestamp + this.sweepInterval >= now + }) }, this.sweepInterval) } @@ -122,9 +137,29 @@ export class PreRequests { const pendingRequest = this.pendingRequests.shift(key) if (pendingRequest) { + const timings = { + cdpRequestWillBeSentTimestamp: browserPreRequest.cdpRequestWillBeSentTimestamp, + cdpRequestWillBeSentReceivedTimestamp: browserPreRequest.cdpRequestWillBeSentReceivedTimestamp, + proxyRequestReceivedTimestamp: pendingRequest.proxyRequestReceivedTimestamp, + cdpLagDuration: browserPreRequest.cdpRequestWillBeSentReceivedTimestamp - browserPreRequest.cdpRequestWillBeSentTimestamp, + proxyRequestCorrelationDuration: Math.max(browserPreRequest.cdpRequestWillBeSentReceivedTimestamp - pendingRequest.proxyRequestReceivedTimestamp, 0), + } + debugVerbose('Incoming pre-request %s matches pending request. %o', key, browserPreRequest) - clearTimeout(pendingRequest.timeout) - pendingRequest.callback(browserPreRequest) + if (!pendingRequest.timedOut) { + clearTimeout(pendingRequest.timeout) + pendingRequest.callback({ + ...browserPreRequest, + ...timings, + }) + + return + } + + this.protocolManager?.responseStreamTimedOut({ + requestId: browserPreRequest.requestId, + timings, + }) return } @@ -132,6 +167,24 @@ export class PreRequests { debugVerbose('Caching pre-request %s to be matched later. %o', key, browserPreRequest) this.pendingPreRequests.push(key, { browserPreRequest, + cdpRequestWillBeSentTimestamp: browserPreRequest.cdpRequestWillBeSentTimestamp, + cdpRequestWillBeSentReceivedTimestamp: browserPreRequest.cdpRequestWillBeSentReceivedTimestamp, + }) + } + + addPendingUrlWithoutPreRequest (url: string) { + const key = `GET-${url}` + const pendingRequest = this.pendingRequests.shift(key) + + if (pendingRequest) { + debugVerbose('Handling %s without a CDP prerequest', key) + clearTimeout(pendingRequest.timeout) + pendingRequest.callback() + + return + } + + this.pendingUrlsWithoutPreRequests.push(key, { timestamp: Date.now(), }) } @@ -143,6 +196,8 @@ export class PreRequests { } get (req: CypressIncomingRequest, ctxDebug, callback: GetPreRequestCb) { + const proxyRequestReceivedTimestamp = performance.now() + performance.timeOrigin + metrics.proxyRequestsReceived++ const key = `${req.method}-${req.proxiedUrl}` const pendingPreRequest = this.pendingPreRequests.shift(key) @@ -150,7 +205,24 @@ export class PreRequests { if (pendingPreRequest) { metrics.immediatelyMatchedRequests++ ctxDebug('Incoming request %s matches known pre-request: %o', key, pendingPreRequest) - callback(pendingPreRequest.browserPreRequest) + callback({ + ...pendingPreRequest.browserPreRequest, + cdpRequestWillBeSentTimestamp: pendingPreRequest.cdpRequestWillBeSentTimestamp, + cdpRequestWillBeSentReceivedTimestamp: pendingPreRequest.cdpRequestWillBeSentReceivedTimestamp, + proxyRequestReceivedTimestamp, + cdpLagDuration: pendingPreRequest.cdpRequestWillBeSentReceivedTimestamp - pendingPreRequest.cdpRequestWillBeSentTimestamp, + proxyRequestCorrelationDuration: Math.max(pendingPreRequest.cdpRequestWillBeSentReceivedTimestamp - proxyRequestReceivedTimestamp, 0), + }) + + return + } + + const pendingUrlWithoutPreRequests = this.pendingUrlsWithoutPreRequests.shift(key) + + if (pendingUrlWithoutPreRequests) { + metrics.immediatelyMatchedRequests++ + ctxDebug('Incoming request %s matches known pending url without pre request', key) + callback() return } @@ -158,14 +230,19 @@ export class PreRequests { const pendingRequest: PendingRequest = { ctxDebug, callback, + proxyRequestReceivedTimestamp: performance.now() + performance.timeOrigin, timeout: setTimeout(() => { - ctxDebug('Never received pre-request for request %s after waiting %sms. Continuing without one.', key, this.requestTimeout) + ctxDebug('Never received pre-request or url without pre-request for request %s after waiting %sms. Continuing without one.', key, this.requestTimeout) metrics.unmatchedRequests++ - this.pendingRequests.removeExact(key, pendingRequest) + pendingRequest.timedOut = true callback() }, this.requestTimeout), } this.pendingRequests.push(key, pendingRequest) } + + setProtocolManager (protocolManager: ProtocolManagerShape) { + this.protocolManager = protocolManager + } } diff --git a/packages/proxy/lib/network-proxy.ts b/packages/proxy/lib/network-proxy.ts index a604614739a9..2702b059a1f7 100644 --- a/packages/proxy/lib/network-proxy.ts +++ b/packages/proxy/lib/network-proxy.ts @@ -17,6 +17,10 @@ export class NetworkProxy { this.http.removePendingBrowserPreRequest(requestId) } + addPendingUrlWithoutPreRequest (url: string) { + this.http.addPendingUrlWithoutPreRequest(url) + } + handleHttpRequest (req, res) { const span = telemetry.startSpan({ name: 'network:proxy:handleHttpRequest', diff --git a/packages/proxy/lib/types.ts b/packages/proxy/lib/types.ts index 20384ee211e6..94f73165441a 100644 --- a/packages/proxy/lib/types.ts +++ b/packages/proxy/lib/types.ts @@ -1,5 +1,6 @@ import type { Readable } from 'stream' import type { Request, Response } from 'express' +import type { ProxyTimings } from '@packages/types' import type { ResourceType } from '@packages/net-stubbing' import type { BackendRoute } from '@packages/net-stubbing/lib/server/types' @@ -10,7 +11,7 @@ export type CypressIncomingRequest = Request & { proxiedUrl: string abort: () => void requestId: string - browserPreRequest?: BrowserPreRequest + browserPreRequest?: BrowserPreRequestWithTimings body?: string responseTimeout?: number followRedirect?: boolean @@ -60,8 +61,12 @@ export type BrowserPreRequest = { resourceType: ResourceType originalResourceType: string | undefined errorHandled?: boolean + cdpRequestWillBeSentTimestamp: number + cdpRequestWillBeSentReceivedTimestamp: number } +export type BrowserPreRequestWithTimings = BrowserPreRequest & ProxyTimings + /** * Notification that the browser has received a response for a request for which a pre-request may have been emitted. */ diff --git a/packages/proxy/test/unit/http/response-middleware.spec.ts b/packages/proxy/test/unit/http/response-middleware.spec.ts index d0f9dc075b09..6adfa1ba472c 100644 --- a/packages/proxy/test/unit/http/response-middleware.spec.ts +++ b/packages/proxy/test/unit/http/response-middleware.spec.ts @@ -1827,6 +1827,11 @@ describe('http/response-middleware', function () { req: { browserPreRequest: { requestId: '123', + cdpRequestWillBeSentTimestamp: 1, + cdpRequestWillBeSentReceivedTimestamp: 2, + proxyRequestReceivedTimestamp: 3, + cdpLagDuration: 4, + proxyRequestCorrelationDuration: 5, }, }, incomingRes: { @@ -1840,6 +1845,11 @@ describe('http/response-middleware', function () { sinon.match(function (actual) { expect(actual.requestId).to.equal('123') expect(actual.isCached).to.equal(true) + expect(actual.timings.cdpRequestWillBeSentTimestamp).to.equal(1) + expect(actual.timings.cdpRequestWillBeSentReceivedTimestamp).to.equal(2) + expect(actual.timings.proxyRequestReceivedTimestamp).to.equal(3) + expect(actual.timings.cdpLagDuration).to.equal(4) + expect(actual.timings.proxyRequestCorrelationDuration).to.equal(5) return true }), @@ -2259,6 +2269,11 @@ describe('http/response-middleware', function () { req: { browserPreRequest: { requestId: '123', + cdpRequestWillBeSentTimestamp: 1, + cdpRequestWillBeSentReceivedTimestamp: 2, + proxyRequestReceivedTimestamp: 3, + cdpLagDuration: 4, + proxyRequestCorrelationDuration: 5, }, }, res, @@ -2278,6 +2293,11 @@ describe('http/response-middleware', function () { expect(actual.isAlreadyGunzipped).to.equal(true) expect(actual.responseStream).to.equal(stream) expect(actual.res).to.equal(res) + expect(actual.timings.cdpRequestWillBeSentTimestamp).to.equal(1) + expect(actual.timings.cdpRequestWillBeSentReceivedTimestamp).to.equal(2) + expect(actual.timings.proxyRequestReceivedTimestamp).to.equal(3) + expect(actual.timings.cdpLagDuration).to.equal(4) + expect(actual.timings.proxyRequestCorrelationDuration).to.equal(5) return true }), diff --git a/packages/proxy/test/unit/http/util/prerequests.spec.ts b/packages/proxy/test/unit/http/util/prerequests.spec.ts index f325e539ecf8..309727358fe4 100644 --- a/packages/proxy/test/unit/http/util/prerequests.spec.ts +++ b/packages/proxy/test/unit/http/util/prerequests.spec.ts @@ -2,17 +2,26 @@ import { PreRequests } from '@packages/proxy/lib/http/util/prerequests' import { BrowserPreRequest, CypressIncomingRequest } from '@packages/proxy' import { expect } from 'chai' import sinon from 'sinon' +import { performance } from 'perf_hooks' +import { ProtocolManagerShape } from '@packages/types' describe('http/util/prerequests', () => { let preRequests: PreRequests + let protocolManager: ProtocolManagerShape - function expectPendingCounts (pendingRequests: number, pendingPreRequests: number) { + function expectPendingCounts (pendingRequests: number, pendingPreRequests: number, pendingWithoutPreRequests = 0) { expect(preRequests.pendingRequests.length).to.eq(pendingRequests, 'wrong number of pending requests') expect(preRequests.pendingPreRequests.length).to.eq(pendingPreRequests, 'wrong number of pending prerequests') + expect(preRequests.pendingUrlsWithoutPreRequests.length).to.eq(pendingWithoutPreRequests, 'wrong number of pending without prerequests') } beforeEach(() => { preRequests = new PreRequests(10) + protocolManager = { + responseStreamTimedOut: sinon.stub(), + } as any + + preRequests.setProtocolManager(protocolManager) }) afterEach(() => { @@ -21,11 +30,40 @@ describe('http/util/prerequests', () => { it('synchronously matches a pre-request that existed at the time of the request', () => { // should match in order - preRequests.addPending({ requestId: '1234', url: 'foo', method: 'WRONGMETHOD' } as BrowserPreRequest) - const secondPreRequest = { requestId: '1234', url: 'foo', method: 'GET' } as BrowserPreRequest + preRequests.addPending({ + requestId: '1234', + url: 'foo', + method: 'WRONGMETHOD', + headers: {}, + resourceType: 'xhr', + originalResourceType: undefined, + cdpRequestWillBeSentTimestamp: 1, + cdpRequestWillBeSentReceivedTimestamp: 2, + }) + + const secondPreRequest: BrowserPreRequest = { + requestId: '1234', + url: 'foo', + method: 'GET', + headers: {}, + resourceType: 'xhr', + originalResourceType: undefined, + cdpRequestWillBeSentTimestamp: 1, + cdpRequestWillBeSentReceivedTimestamp: performance.now() + performance.timeOrigin + 10000, + } preRequests.addPending(secondPreRequest) - preRequests.addPending({ requestId: '1234', url: 'foo', method: 'GET' } as BrowserPreRequest) + preRequests.addPending({ + requestId: '1234', + url: 'foo', + method: 'GET', + headers: {}, + resourceType: 'xhr', + originalResourceType: undefined, + cdpRequestWillBeSentTimestamp: 1, + cdpRequestWillBeSentReceivedTimestamp: 2, + }) + expectPendingCounts(0, 3) const cb = sinon.stub() @@ -33,12 +71,43 @@ describe('http/util/prerequests', () => { preRequests.get({ proxiedUrl: 'foo', method: 'GET' } as CypressIncomingRequest, () => {}, cb) const { args } = cb.getCall(0) - - expect(args[0]).to.eq(secondPreRequest) + const arg = args[0] + + expect(arg.requestId).to.eq(secondPreRequest.requestId) + expect(arg.url).to.eq(secondPreRequest.url) + expect(arg.method).to.eq(secondPreRequest.method) + expect(arg.headers).to.deep.eq(secondPreRequest.headers) + expect(arg.resourceType).to.eq(secondPreRequest.resourceType) + expect(arg.originalResourceType).to.eq(secondPreRequest.originalResourceType) + expect(arg.cdpRequestWillBeSentTimestamp).to.eq(secondPreRequest.cdpRequestWillBeSentTimestamp) + expect(arg.cdpRequestWillBeSentReceivedTimestamp).to.eq(secondPreRequest.cdpRequestWillBeSentReceivedTimestamp) + expect(arg.proxyRequestReceivedTimestamp).to.be.a('number') + expect(arg.cdpLagDuration).to.eq(secondPreRequest.cdpRequestWillBeSentReceivedTimestamp - secondPreRequest.cdpRequestWillBeSentTimestamp) + expect(arg.proxyRequestCorrelationDuration).to.eq(secondPreRequest.cdpRequestWillBeSentReceivedTimestamp - arg.proxyRequestReceivedTimestamp) expectPendingCounts(0, 2) }) + it('synchronously matches a request without a pre-request that existed at the time of the request', () => { + // should match in order + preRequests.addPendingUrlWithoutPreRequest('foo') + preRequests.addPendingUrlWithoutPreRequest('foo') + preRequests.addPendingUrlWithoutPreRequest('foo') + + expectPendingCounts(0, 0, 3) + + const cb = sinon.stub() + + preRequests.get({ proxiedUrl: 'foo', method: 'GET' } as CypressIncomingRequest, () => {}, cb) + + const { args } = cb.getCall(0) + const arg = args[0] + + expect(arg).to.be.undefined + + expectPendingCounts(0, 0, 2) + }) + it('synchronously matches a pre-request added after the request', (done) => { const cb = (preRequest) => { expect(preRequest).to.include({ requestId: '1234', url: 'foo', method: 'GET' }) @@ -50,7 +119,7 @@ describe('http/util/prerequests', () => { preRequests.addPending({ requestId: '1234', url: 'foo', method: 'GET' } as BrowserPreRequest) }) - it('invokes a request callback after a timeout if no pre-request occurs', (done) => { + it('synchronously matches a request without a pre-request added after the request', (done) => { const cb = (preRequest) => { expect(preRequest).to.be.undefined expectPendingCounts(0, 0) @@ -58,12 +127,56 @@ describe('http/util/prerequests', () => { } preRequests.get({ proxiedUrl: 'foo', method: 'GET' } as CypressIncomingRequest, () => {}, cb) + preRequests.addPendingUrlWithoutPreRequest('foo') + }) + + it('invokes a request callback after a timeout if no pre-request occurs', async () => { + let cb + const cbPromise = new Promise((resolve) => { + cb = (preRequest) => { + expect(preRequest).to.be.undefined + + // we should have keep the pending request to eventually be correlated later, but don't block the body in the meantime + expectPendingCounts(1, 0) + + resolve() + } + }) + + preRequests.get({ proxiedUrl: 'foo', method: 'GET' } as CypressIncomingRequest, () => {}, cb) + + await cbPromise + + const browserPreRequest: BrowserPreRequest = { + requestId: '1234', + url: 'foo', + method: 'GET', + headers: {}, + resourceType: 'xhr', + originalResourceType: undefined, + cdpRequestWillBeSentTimestamp: 1, + cdpRequestWillBeSentReceivedTimestamp: performance.now() + performance.timeOrigin + 10000, + } + + preRequests.addPending(browserPreRequest) + + expectPendingCounts(0, 0) + + const arg = (protocolManager.responseStreamTimedOut as any).getCall(0).args[0] + + expect(arg.requestId).to.eq(browserPreRequest.requestId) + expect(arg.timings.cdpRequestWillBeSentTimestamp).to.eq(browserPreRequest.cdpRequestWillBeSentTimestamp) + expect(arg.timings.cdpRequestWillBeSentReceivedTimestamp).to.eq(browserPreRequest.cdpRequestWillBeSentReceivedTimestamp) + expect(arg.timings.proxyRequestReceivedTimestamp).to.be.a('number') + expect(arg.timings.cdpLagDuration).to.eq(browserPreRequest.cdpRequestWillBeSentReceivedTimestamp - browserPreRequest.cdpRequestWillBeSentTimestamp) + expect(arg.timings.proxyRequestCorrelationDuration).to.eq(browserPreRequest.cdpRequestWillBeSentReceivedTimestamp - arg.timings.proxyRequestReceivedTimestamp) }) // https://github.com/cypress-io/cypress/issues/17853 it('eventually discards pre-requests that don\'t match requests', (done) => { preRequests = new PreRequests(10, 200) - preRequests.addPending({ requestId: '1234', url: 'foo', method: 'GET' } as BrowserPreRequest) + preRequests.addPending({ requestId: '1234', url: 'foo', method: 'GET', cdpRequestWillBeSentReceivedTimestamp: performance.now() + performance.timeOrigin } as BrowserPreRequest) + preRequests.addPendingUrlWithoutPreRequest('bar') // preRequests garbage collects pre-requests that never matched up with an incoming request after around // 2 * requestTimeout. We verify that it's gone (and therefore not leaking memory) by sending in a request @@ -71,7 +184,7 @@ describe('http/util/prerequests', () => { setTimeout(() => { const cb = (preRequest) => { expect(preRequest).to.be.undefined - expectPendingCounts(0, 0) + expectPendingCounts(1, 0, 0) done() } diff --git a/packages/server/lib/automation/automation.ts b/packages/server/lib/automation/automation.ts index b1101a505ca4..53d4bbf0f94f 100644 --- a/packages/server/lib/automation/automation.ts +++ b/packages/server/lib/automation/automation.ts @@ -14,7 +14,7 @@ export class Automation { private cookies: Cookies private screenshot: { capture: (data: any, automate: any) => any } - constructor (cyNamespace?: string, cookieNamespace?: string, screenshotsFolder?: string | false, public onBrowserPreRequest?: OnBrowserPreRequest, public onRequestEvent?: OnRequestEvent, public onRequestServedFromCache?: (requestId: string) => void, public onRequestFailed?: (requestId: string) => void) { + constructor (cyNamespace?: string, cookieNamespace?: string, screenshotsFolder?: string | false, public onBrowserPreRequest?: OnBrowserPreRequest, public onRequestEvent?: OnRequestEvent, public onRequestServedFromCache?: (requestId: string) => void, public onRequestFailed?: (requestId: string) => void, public onDownloadLinkClicked?: (downloadUrl: string) => void) { this.requests = {} // set the middleware diff --git a/packages/server/lib/browsers/cdp_automation.ts b/packages/server/lib/browsers/cdp_automation.ts index 818e81942140..6255de775972 100644 --- a/packages/server/lib/browsers/cdp_automation.ts +++ b/packages/server/lib/browsers/cdp_automation.ts @@ -7,6 +7,7 @@ import type ProtocolMapping from 'devtools-protocol/types/protocol-mapping' import { cors, uri } from '@packages/network' import debugModule from 'debug' import { URL } from 'url' +import { performance } from 'perf_hooks' import type { ResourceType, BrowserPreRequest, BrowserResponseReceived } from '@packages/proxy' import type { CDPClient, ProtocolManagerShape, WriteVideoFrame } from '@packages/types' @@ -237,6 +238,10 @@ export class CdpAutomation implements CDPClient { headers: params.request.headers, resourceType: normalizeResourceType(params.type), originalResourceType: params.type, + // wallTime is in seconds: https://vanilla.aslushnikov.com/?Network.TimeSinceEpoch + // normalize to milliseconds to be comparable to everything else we're gathering + cdpRequestWillBeSentTimestamp: params.wallTime * 1000, + cdpRequestWillBeSentReceivedTimestamp: performance.now() + performance.timeOrigin, } this.automation.onBrowserPreRequest?.(browserPreRequest) diff --git a/packages/server/lib/browsers/chrome.ts b/packages/server/lib/browsers/chrome.ts index e3a11b10067c..f5a11c449b19 100644 --- a/packages/server/lib/browsers/chrome.ts +++ b/packages/server/lib/browsers/chrome.ts @@ -517,6 +517,8 @@ export = { await pageCriClient.send('Page.enable') + await utils.handleDownloadLinksViaCDP(pageCriClient, automation) + await options['onInitializeNewBrowserTab']?.() await Promise.all([ diff --git a/packages/server/lib/browsers/electron.ts b/packages/server/lib/browsers/electron.ts index b105e6e20954..33aed67a3866 100644 --- a/packages/server/lib/browsers/electron.ts +++ b/packages/server/lib/browsers/electron.ts @@ -307,6 +307,7 @@ export = { cdpSocketServer?.attachCDPClient(cdpAutomation), videoApi && recordVideo(cdpAutomation, videoApi), this._handleDownloads(win, options.downloadsFolder, automation), + await utils.handleDownloadLinksViaCDP(pageCriClient, automation), ]) } diff --git a/packages/server/lib/browsers/utils.ts b/packages/server/lib/browsers/utils.ts index 3353b881973c..b6f246b28ff9 100644 --- a/packages/server/lib/browsers/utils.ts +++ b/packages/server/lib/browsers/utils.ts @@ -6,6 +6,17 @@ import * as errors from '../errors' import * as plugins from '../plugins' import { getError } from '@packages/errors' import * as launcher from '@packages/launcher' +import type { Automation } from '../automation' +import type { CriClient } from './cri-client' + +declare global { + interface Window { + navigation?: { + addEventListener: (event: string, listener: (event: any) => void) => void + } + cypressDownloadLinkClicked: (url: string) => void + } +} const path = require('path') const debug = require('debug')('cypress:server:browsers:utils') @@ -362,6 +373,52 @@ const throwBrowserNotFound = function (browserName, browsers: FoundBrowser[] = [ return errors.throwErr('BROWSER_NOT_FOUND_BY_NAME', browserName, formatBrowsersToOptions(browsers)) } +// Chromium browsers and webkit do not give us pre requests for download links but they still go through the proxy. +// We need to notify the proxy when they are clicked so that we can resolve the pending request waiting to be +// correlated in the proxy. +const handleDownloadLinksViaCDP = async (criClient: CriClient, automation: Automation) => { + await criClient.send('Runtime.enable') + await criClient.send('Runtime.addBinding', { + name: 'cypressDownloadLinkClicked', + }) + + await criClient.on('Runtime.bindingCalled', async (data) => { + if (data.name === 'cypressDownloadLinkClicked') { + const url = data.payload + + await automation.onDownloadLinkClicked?.(url) + } + }) + + await criClient.send('Page.addScriptToEvaluateOnNewDocument', { + source: `(${listenForDownload.toString()})()`, + }) +} + +// The most efficient way to do this is to listen for the navigate event. However, this is only available in chromium browsers (after 102). +// For older versions and for webkit, we need to listen for click events on anchor tags with the download attribute. +const listenForDownload = () => { + if (window.navigation) { + window.navigation.addEventListener('navigate', (event) => { + if (typeof event.downloadRequest === 'string') { + window.cypressDownloadLinkClicked(event.destination.url) + } + }) + } else { + document.addEventListener('click', (event) => { + if (event.target instanceof HTMLAnchorElement && typeof event.target.download === 'string') { + window.cypressDownloadLinkClicked(event.target.href) + } + }) + + document.addEventListener('keydown', (event) => { + if (event.target instanceof HTMLAnchorElement && event.key === 'Enter' && typeof event.target.download === 'string') { + window.cypressDownloadLinkClicked(event.target.href) + } + }) + } +} + export = { extendLaunchOptionsFromPlugins, @@ -396,6 +453,10 @@ export = { throwBrowserNotFound, + handleDownloadLinksViaCDP, + + listenForDownload, + writeExtension (browser, isTextTerminal, proxyUrl, socketIoRoute) { debug('writing extension') diff --git a/packages/server/lib/browsers/webkit-automation.ts b/packages/server/lib/browsers/webkit-automation.ts index daa9e95e13e1..d06ea86311b3 100644 --- a/packages/server/lib/browsers/webkit-automation.ts +++ b/packages/server/lib/browsers/webkit-automation.ts @@ -7,6 +7,7 @@ import type { RunModeVideoApi } from '@packages/types' import path from 'path' import mime from 'mime' import { cookieMatches, CyCookieFilter } from '../automation/util' +import utils from './utils' const debug = Debug('cypress:server:browsers:webkit-automation') @@ -108,6 +109,14 @@ export class WebKitAutomation { this.page = await newContext.newPage() this.context = this.page.context() + await this.page.addInitScript({ + content: `(${utils.listenForDownload.toString()})()`, + }) + + await this.context.exposeBinding('cypressDownloadLinkClicked', (source, downloadUrl) => { + this.automation.onDownloadLinkClicked?.(downloadUrl) + }) + this.handleRequestEvents() if (options.downloadsFolder) this.handleDownloadEvents(options.downloadsFolder) @@ -222,6 +231,8 @@ export class WebKitAutomation { headers: request.headers(), resourceType: normalizeResourceType(request.resourceType()), originalResourceType: request.resourceType(), + cdpRequestWillBeSentTimestamp: request.timing().requestStart, + cdpRequestWillBeSentReceivedTimestamp: performance.now() + performance.timeOrigin, } debug('received request %o', { browserPreRequest }) diff --git a/packages/server/lib/cloud/protocol.ts b/packages/server/lib/cloud/protocol.ts index e7bf72d3b0e5..478a5d887207 100644 --- a/packages/server/lib/cloud/protocol.ts +++ b/packages/server/lib/cloud/protocol.ts @@ -13,7 +13,7 @@ import pkg from '@packages/root' import env from '../util/env' import type { Readable } from 'stream' -import type { ProtocolManagerShape, AppCaptureProtocolInterface, CDPClient, ProtocolError, CaptureArtifact, ProtocolErrorReport, ProtocolCaptureMethod, ProtocolManagerOptions, ResponseStreamOptions, ResponseEndedWithEmptyBodyOptions } from '@packages/types' +import type { ProtocolManagerShape, AppCaptureProtocolInterface, CDPClient, ProtocolError, CaptureArtifact, ProtocolErrorReport, ProtocolCaptureMethod, ProtocolManagerOptions, ResponseStreamOptions, ResponseEndedWithEmptyBodyOptions, ResponseStreamTimedOutOptions } from '@packages/types' const routes = require('./routes') @@ -223,6 +223,10 @@ export class ProtocolManager implements ProtocolManagerShape { return this.invokeSync('responseStreamReceived', { isEssential: false }, options) } + responseStreamTimedOut (options: ResponseStreamTimedOutOptions): void { + this.invokeSync('responseStreamTimedOut', { isEssential: false }, options) + } + canUpload (): boolean { return !!this._protocol && !!this._archivePath && !!this._db } diff --git a/packages/server/lib/project-base.ts b/packages/server/lib/project-base.ts index e8172918c40c..129a38908316 100644 --- a/packages/server/lib/project-base.ts +++ b/packages/server/lib/project-base.ts @@ -338,7 +338,11 @@ export class ProjectBase extends EE { this.server.removeBrowserPreRequest(requestId) } - this._automation = new Automation(namespace, socketIoCookie, screenshotsFolder, onBrowserPreRequest, onRequestEvent, onRequestServedFromCache, onRequestFailed) + const onDownloadLinkClicked = (downloadUrl: string) => { + this.server.addPendingUrlWithoutPreRequest(downloadUrl) + } + + this._automation = new Automation(namespace, socketIoCookie, screenshotsFolder, onBrowserPreRequest, onRequestEvent, onRequestServedFromCache, onRequestFailed, onDownloadLinkClicked) const ios = this.server.startWebsockets(this.automation, this.cfg, { onReloadBrowser: options.onReloadBrowser, diff --git a/packages/server/lib/server-base.ts b/packages/server/lib/server-base.ts index 78cfbd7a9d6d..cf23c711ddec 100644 --- a/packages/server/lib/server-base.ts +++ b/packages/server/lib/server-base.ts @@ -499,6 +499,10 @@ export class ServerBase { this.socket.toDriver('request:event', eventName, data) } + addPendingUrlWithoutPreRequest (downloadUrl: string) { + this.networkProxy.addPendingUrlWithoutPreRequest(downloadUrl) + } + _createHttpServer (app): DestroyableHttpServer { const svr = http.createServer(httpUtils.lenientOptions, app) diff --git a/packages/server/test/unit/browsers/cdp_automation_spec.ts b/packages/server/test/unit/browsers/cdp_automation_spec.ts index 9021b9b63d92..e239eea1442c 100644 --- a/packages/server/test/unit/browsers/cdp_automation_spec.ts +++ b/packages/server/test/unit/browsers/cdp_automation_spec.ts @@ -109,20 +109,23 @@ context('lib/browsers/cdp_automation', () => { url: 'https://www.google.com', headers: {}, }, + wallTime: 100.100100, } this.onFn .withArgs('Network.requestWillBeSent') .yield(browserPreRequest) - expect(this.automation.onBrowserPreRequest).to.have.been.calledWith({ - requestId: browserPreRequest.requestId, - method: browserPreRequest.request.method, - url: browserPreRequest.request.url, - headers: browserPreRequest.request.headers, - resourceType: browserPreRequest.type, - originalResourceType: browserPreRequest.type, - }) + const arg = this.automation.onBrowserPreRequest.getCall(0).args[0] + + expect(arg.requestId).to.eq(browserPreRequest.requestId) + expect(arg.method).to.eq(browserPreRequest.request.method) + expect(arg.url).to.eq(browserPreRequest.request.url) + expect(arg.headers).to.eq(browserPreRequest.request.headers) + expect(arg.resourceType).to.eq(browserPreRequest.type) + expect(arg.originalResourceType).to.eq(browserPreRequest.type) + expect(arg.cdpRequestWillBeSentTimestamp).to.be.closeTo(100100.100, 0.001) + expect(arg.cdpRequestWillBeSentReceivedTimestamp).to.be.a('number') }) it('removes # from a url', function () { @@ -134,20 +137,23 @@ context('lib/browsers/cdp_automation', () => { url: 'https://www.google.com/foo#', headers: {}, }, + wallTime: 100.100100, } this.onFn .withArgs('Network.requestWillBeSent') .yield(browserPreRequest) - expect(this.automation.onBrowserPreRequest).to.have.been.calledWith({ - requestId: browserPreRequest.requestId, - method: browserPreRequest.request.method, - url: 'https://www.google.com/foo', // we only care about the url - headers: browserPreRequest.request.headers, - resourceType: browserPreRequest.type, - originalResourceType: browserPreRequest.type, - }) + const arg = this.automation.onBrowserPreRequest.getCall(0).args[0] + + expect(arg.requestId).to.eq(browserPreRequest.requestId) + expect(arg.method).to.eq(browserPreRequest.request.method) + expect(arg.url).to.eq('https://www.google.com/foo') + expect(arg.headers).to.eq(browserPreRequest.request.headers) + expect(arg.resourceType).to.eq(browserPreRequest.type) + expect(arg.originalResourceType).to.eq(browserPreRequest.type) + expect(arg.cdpRequestWillBeSentTimestamp).to.be.closeTo(100100.100, 0.001) + expect(arg.cdpRequestWillBeSentReceivedTimestamp).to.be.a('number') }) it('ignore events with data urls', function () { diff --git a/packages/server/test/unit/browsers/chrome_spec.js b/packages/server/test/unit/browsers/chrome_spec.js index 4aa4aeef440b..987afc4c8313 100644 --- a/packages/server/test/unit/browsers/chrome_spec.js +++ b/packages/server/test/unit/browsers/chrome_spec.js @@ -10,6 +10,7 @@ const utils = require(`../../../lib/browsers/utils`) const chrome = require(`../../../lib/browsers/chrome`) const { fs } = require(`../../../lib/util/fs`) const { BrowserCriClient } = require('../../../lib/browsers/browser-cri-client') +const { expect } = require('chai') const openOpts = { onError: () => {}, @@ -59,6 +60,7 @@ describe('lib/browsers/chrome', () => { sinon.stub(launch, 'launch').resolves(this.launchedBrowser) sinon.stub(utils, 'getProfileDir').returns('/profile/dir') sinon.stub(utils, 'ensureCleanCache').resolves('/profile/dir/CypressCache') + sinon.stub(utils, 'handleDownloadLinksViaCDP').resolves() this.readJson = sinon.stub(fs, 'readJson') this.readJson.withArgs('/profile/dir/Default/Preferences').rejects({ code: 'ENOENT' }) @@ -86,6 +88,8 @@ describe('lib/browsers/chrome', () => { expect(this.pageCriClient.send).to.have.been.calledWith('Page.setDownloadBehavior') expect(this.pageCriClient.send).to.have.been.calledWith('Network.enable') expect(this.pageCriClient.send).to.have.been.calledWith('Fetch.enable') + + expect(utils.handleDownloadLinksViaCDP).to.be.calledOnce }) }) diff --git a/packages/server/test/unit/browsers/electron_spec.js b/packages/server/test/unit/browsers/electron_spec.js index 10a09998feed..4e7ec86c602f 100644 --- a/packages/server/test/unit/browsers/electron_spec.js +++ b/packages/server/test/unit/browsers/electron_spec.js @@ -13,6 +13,7 @@ const savedState = require(`../../../lib/saved_state`) const { Automation } = require(`../../../lib/automation`) const { BrowserCriClient } = require('../../../lib/browsers/browser-cri-client') const electronApp = require('../../../lib/util/electron-app') +const utils = require('../../../lib/browsers/utils') const ELECTRON_PID = 10001 @@ -67,6 +68,7 @@ describe('lib/browsers/electron', () => { sinon.stub(Windows, 'installExtension').returns() sinon.stub(Windows, 'removeAllExtensions').returns() sinon.stub(electronApp, 'getRemoteDebuggingPort').resolves(1234) + sinon.stub(utils, 'handleDownloadLinksViaCDP').resolves() // mock CRI client during testing this.pageCriClient = { @@ -347,6 +349,13 @@ describe('lib/browsers/electron', () => { }) }) + it('handles download links via cdp', function () { + return electron._launch(this.win, this.url, this.automation, this.options, undefined, undefined, { attachCDPClient: sinon.stub() }) + .then(() => { + expect(utils.handleDownloadLinksViaCDP).to.be.calledWith(this.pageCriClient, this.automation) + }) + }) + it('registers onRequest automation middleware and calls show when requesting to be focused', function () { sinon.spy(this.automation, 'use') diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 5ba54eda0e31..e3e5fb57c6f7 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -43,3 +43,5 @@ export * from './git' export * from './video' export * from './protocol' + +export * from './proxy' diff --git a/packages/types/src/protocol.ts b/packages/types/src/protocol.ts index d3247c82d637..f6737c7e1454 100644 --- a/packages/types/src/protocol.ts +++ b/packages/types/src/protocol.ts @@ -2,6 +2,7 @@ import type { Database } from 'better-sqlite3' import type ProtocolMapping from 'devtools-protocol/types/protocol-mapping' import type { IncomingHttpHeaders } from 'http' import type { Readable } from 'stream' +import type { ProxyTimings } from '@packages/types' type Commands = ProtocolMapping.Commands type Command = Commands[T] @@ -31,6 +32,7 @@ export interface AppCaptureProtocolCommon { resetTest (testId: string): void responseEndedWithEmptyBody: (options: ResponseEndedWithEmptyBodyOptions) => void responseStreamReceived (options: ResponseStreamOptions): Readable | undefined + responseStreamTimedOut (options: ResponseStreamTimedOutOptions): void } export interface AppCaptureProtocolInterface extends AppCaptureProtocolCommon { @@ -97,6 +99,7 @@ type Response = { export type ResponseEndedWithEmptyBodyOptions = { requestId: string isCached: boolean + timings: ProxyTimings } export type ResponseStreamOptions = { @@ -105,4 +108,10 @@ export type ResponseStreamOptions = { isAlreadyGunzipped: boolean responseStream: Readable res: Response + timings: ProxyTimings +} + +export type ResponseStreamTimedOutOptions = { + requestId: string + timings: ProxyTimings } diff --git a/packages/types/src/proxy.ts b/packages/types/src/proxy.ts new file mode 100644 index 000000000000..7100b59a7375 --- /dev/null +++ b/packages/types/src/proxy.ts @@ -0,0 +1,7 @@ +export type ProxyTimings = { + cdpRequestWillBeSentTimestamp: number + cdpRequestWillBeSentReceivedTimestamp: number + proxyRequestReceivedTimestamp: number + cdpLagDuration: number + proxyRequestCorrelationDuration: number +} diff --git a/system-tests/__snapshots__/protocol_spec.js b/system-tests/__snapshots__/protocol_spec.js index dd54a9389164..5d85bd7468db 100644 --- a/system-tests/__snapshots__/protocol_spec.js +++ b/system-tests/__snapshots__/protocol_spec.js @@ -4923,7 +4923,8 @@ exports['e2e events'] = ` "resetTest": [ "r3" ], - "responseEndedWithEmptyBody": [] + "responseEndedWithEmptyBody": [], + "responseStreamTimedOut": [] } ` @@ -6531,37 +6532,94 @@ exports['component events - experimentalSingleTabRunMode: true'] = ` "responseEndedWithEmptyBody": [ { "requestId": "Any.Number", - "isCached": true + "isCached": true, + "timings": { + "cdpRequestWillBeSentTimestamp": "Any.Number", + "cdpRequestWillBeSentReceivedTimestamp": "Any.Number", + "proxyRequestReceivedTimestamp": "Any.Number", + "cdpLagDuration": "Any.Number", + "proxyRequestCorrelationDuration": "Any.Number" + } }, { "requestId": "Any.Number", - "isCached": true + "isCached": true, + "timings": { + "cdpRequestWillBeSentTimestamp": "Any.Number", + "cdpRequestWillBeSentReceivedTimestamp": "Any.Number", + "proxyRequestReceivedTimestamp": "Any.Number", + "cdpLagDuration": "Any.Number", + "proxyRequestCorrelationDuration": "Any.Number" + } }, { "requestId": "Any.Number", - "isCached": true + "isCached": true, + "timings": { + "cdpRequestWillBeSentTimestamp": "Any.Number", + "cdpRequestWillBeSentReceivedTimestamp": "Any.Number", + "proxyRequestReceivedTimestamp": "Any.Number", + "cdpLagDuration": "Any.Number", + "proxyRequestCorrelationDuration": "Any.Number" + } }, { "requestId": "Any.Number", - "isCached": true + "isCached": true, + "timings": { + "cdpRequestWillBeSentTimestamp": "Any.Number", + "cdpRequestWillBeSentReceivedTimestamp": "Any.Number", + "proxyRequestReceivedTimestamp": "Any.Number", + "cdpLagDuration": "Any.Number", + "proxyRequestCorrelationDuration": "Any.Number" + } }, { "requestId": "Any.Number", - "isCached": true + "isCached": true, + "timings": { + "cdpRequestWillBeSentTimestamp": "Any.Number", + "cdpRequestWillBeSentReceivedTimestamp": "Any.Number", + "proxyRequestReceivedTimestamp": "Any.Number", + "cdpLagDuration": "Any.Number", + "proxyRequestCorrelationDuration": "Any.Number" + } }, { "requestId": "Any.Number", - "isCached": true + "isCached": true, + "timings": { + "cdpRequestWillBeSentTimestamp": "Any.Number", + "cdpRequestWillBeSentReceivedTimestamp": "Any.Number", + "proxyRequestReceivedTimestamp": "Any.Number", + "cdpLagDuration": "Any.Number", + "proxyRequestCorrelationDuration": "Any.Number" + } }, { "requestId": "Any.Number", - "isCached": true + "isCached": true, + "timings": { + "cdpRequestWillBeSentTimestamp": "Any.Number", + "cdpRequestWillBeSentReceivedTimestamp": "Any.Number", + "proxyRequestReceivedTimestamp": "Any.Number", + "cdpLagDuration": "Any.Number", + "proxyRequestCorrelationDuration": "Any.Number" + } }, { "requestId": "Any.Number", - "isCached": true + "isCached": true, + "timings": { + "cdpRequestWillBeSentTimestamp": "Any.Number", + "cdpRequestWillBeSentReceivedTimestamp": "Any.Number", + "proxyRequestReceivedTimestamp": "Any.Number", + "cdpLagDuration": "Any.Number", + "proxyRequestCorrelationDuration": "Any.Number" + } } - ] + ], + "responseStreamTimedOut": [] } ` @@ -8169,20 +8227,49 @@ exports['component events - experimentalSingleTabRunMode: false'] = ` "responseEndedWithEmptyBody": [ { "requestId": "Any.Number", - "isCached": true + "isCached": true, + "timings": { + "cdpRequestWillBeSentTimestamp": "Any.Number", + "cdpRequestWillBeSentReceivedTimestamp": "Any.Number", + "proxyRequestReceivedTimestamp": "Any.Number", + "cdpLagDuration": "Any.Number", + "proxyRequestCorrelationDuration": "Any.Number" + } }, { "requestId": "Any.Number", - "isCached": true + "isCached": true, + "timings": { + "cdpRequestWillBeSentTimestamp": "Any.Number", + "cdpRequestWillBeSentReceivedTimestamp": "Any.Number", + "proxyRequestReceivedTimestamp": "Any.Number", + "cdpLagDuration": "Any.Number", + "proxyRequestCorrelationDuration": "Any.Number" + } }, { "requestId": "Any.Number", - "isCached": true + "isCached": true, + "timings": { + "cdpRequestWillBeSentTimestamp": "Any.Number", + "cdpRequestWillBeSentReceivedTimestamp": "Any.Number", + "proxyRequestReceivedTimestamp": "Any.Number", + "cdpLagDuration": "Any.Number", + "proxyRequestCorrelationDuration": "Any.Number" + } }, { "requestId": "Any.Number", - "isCached": true + "isCached": true, + "timings": { + "cdpRequestWillBeSentTimestamp": "Any.Number", + "cdpRequestWillBeSentReceivedTimestamp": "Any.Number", + "proxyRequestReceivedTimestamp": "Any.Number", + "cdpLagDuration": "Any.Number", + "proxyRequestCorrelationDuration": "Any.Number" + } } - ] + ], + "responseStreamTimedOut": [] } ` diff --git a/system-tests/lib/protocol-stubs/protocolStub.ts b/system-tests/lib/protocol-stubs/protocolStub.ts index 3cdca28a2ac4..c0b1b3a95323 100644 --- a/system-tests/lib/protocol-stubs/protocolStub.ts +++ b/system-tests/lib/protocol-stubs/protocolStub.ts @@ -1,6 +1,6 @@ import path from 'path' import fs from 'fs-extra' -import type { AppCaptureProtocolInterface, ResponseEndedWithEmptyBodyOptions, ResponseStreamOptions } from '@packages/types' +import type { AppCaptureProtocolInterface, ResponseEndedWithEmptyBodyOptions, ResponseStreamOptions, ResponseStreamTimedOutOptions } from '@packages/types' import type { Readable } from 'stream' const getFilePath = (filename) => { @@ -29,6 +29,7 @@ export class AppCaptureProtocol implements AppCaptureProtocolInterface { pageLoading: [], resetTest: [], responseEndedWithEmptyBody: [], + responseStreamTimedOut: [], } private cdpClient: any private scriptToEvaluateId: any @@ -54,6 +55,7 @@ export class AppCaptureProtocol implements AppCaptureProtocolInterface { this.events.urlChanged = [] this.events.pageLoading = [] this.events.responseEndedWithEmptyBody = [] + this.events.responseStreamTimedOut = [] } connectToBrowser = async (cdpClient) => { @@ -159,6 +161,10 @@ export class AppCaptureProtocol implements AppCaptureProtocolInterface { this.events.responseEndedWithEmptyBody.push(options) } + responseStreamTimedOut (options: ResponseStreamTimedOutOptions): void { + this.events.responseStreamTimedOut.push(options) + } + resetTest (testId: string): void { this.resetEvents() diff --git a/system-tests/lib/protocol-stubs/protocolStubWithBeforeSpecError.ts b/system-tests/lib/protocol-stubs/protocolStubWithBeforeSpecError.ts index 4826cf33e0ea..3281e114cd0c 100644 --- a/system-tests/lib/protocol-stubs/protocolStubWithBeforeSpecError.ts +++ b/system-tests/lib/protocol-stubs/protocolStubWithBeforeSpecError.ts @@ -1,4 +1,4 @@ -import type { AppCaptureProtocolInterface, CDPClient, ResponseEndedWithEmptyBodyOptions, ResponseStreamOptions } from '@packages/types' +import type { AppCaptureProtocolInterface, CDPClient, ResponseEndedWithEmptyBodyOptions, ResponseStreamOptions, ResponseStreamTimedOutOptions } from '@packages/types' import type { Readable } from 'stream' export class AppCaptureProtocol implements AppCaptureProtocolInterface { @@ -37,4 +37,5 @@ export class AppCaptureProtocol implements AppCaptureProtocolInterface { pageLoading (input: any): void {} resetTest (testId: string): void {} responseEndedWithEmptyBody: (options: ResponseEndedWithEmptyBodyOptions) => {} + responseStreamTimedOut: (options: ResponseStreamTimedOutOptions) => {} } diff --git a/system-tests/lib/protocol-stubs/protocolStubWithBeforeTestError.ts b/system-tests/lib/protocol-stubs/protocolStubWithBeforeTestError.ts index d4b120eb0e5e..b9a36e943b76 100644 --- a/system-tests/lib/protocol-stubs/protocolStubWithBeforeTestError.ts +++ b/system-tests/lib/protocol-stubs/protocolStubWithBeforeTestError.ts @@ -1,4 +1,4 @@ -import type { AppCaptureProtocolInterface, CDPClient, ResponseEndedWithEmptyBodyOptions, ResponseStreamOptions } from '@packages/types' +import type { AppCaptureProtocolInterface, CDPClient, ResponseEndedWithEmptyBodyOptions, ResponseStreamOptions, ResponseStreamTimedOutOptions } from '@packages/types' import type { Readable } from 'stream' export class AppCaptureProtocol implements AppCaptureProtocolInterface { @@ -45,4 +45,5 @@ export class AppCaptureProtocol implements AppCaptureProtocolInterface { responseEndedWithEmptyBody (options: ResponseEndedWithEmptyBodyOptions) { //throw new Error('error in responseEndedWithEmptyBody') } + responseStreamTimedOut: (options: ResponseStreamTimedOutOptions) => {} } diff --git a/system-tests/lib/protocol-stubs/protocolStubWithNonFatalError.ts b/system-tests/lib/protocol-stubs/protocolStubWithNonFatalError.ts index b0c641c0f55a..b0c809100066 100644 --- a/system-tests/lib/protocol-stubs/protocolStubWithNonFatalError.ts +++ b/system-tests/lib/protocol-stubs/protocolStubWithNonFatalError.ts @@ -1,4 +1,4 @@ -import type { AppCaptureProtocolInterface, CDPClient, ResponseEndedWithEmptyBodyOptions, ResponseStreamOptions } from '@packages/types' +import type { AppCaptureProtocolInterface, CDPClient, ResponseEndedWithEmptyBodyOptions, ResponseStreamOptions, ResponseStreamTimedOutOptions } from '@packages/types' import type { Readable } from 'stream' export class AppCaptureProtocol implements AppCaptureProtocolInterface { @@ -37,4 +37,5 @@ export class AppCaptureProtocol implements AppCaptureProtocolInterface { pageLoading (input: any): void {} resetTest (testId: string): void {} responseEndedWithEmptyBody: (options: ResponseEndedWithEmptyBodyOptions) => {} + responseStreamTimedOut: (options: ResponseStreamTimedOutOptions) => {} } diff --git a/system-tests/lib/protocol-stubs/protocolStubWithRuntimeError.ts b/system-tests/lib/protocol-stubs/protocolStubWithRuntimeError.ts index 57be916d6800..2bbb432b2fb0 100644 --- a/system-tests/lib/protocol-stubs/protocolStubWithRuntimeError.ts +++ b/system-tests/lib/protocol-stubs/protocolStubWithRuntimeError.ts @@ -1,4 +1,4 @@ -import type { AppCaptureProtocolInterface, ResponseEndedWithEmptyBodyOptions, ResponseStreamOptions } from '@packages/types' +import type { AppCaptureProtocolInterface, ResponseEndedWithEmptyBodyOptions, ResponseStreamOptions, ResponseStreamTimedOutOptions } from '@packages/types' import type { Readable } from 'stream' export class AppCaptureProtocol implements AppCaptureProtocolInterface { @@ -39,4 +39,5 @@ export class AppCaptureProtocol implements AppCaptureProtocolInterface { return Promise.resolve() } responseEndedWithEmptyBody: (options: ResponseEndedWithEmptyBodyOptions) => {} + responseStreamTimedOut: (options: ResponseStreamTimedOutOptions) => {} } diff --git a/system-tests/projects/downloads/cypress/e2e/download_csv.cy.ts b/system-tests/projects/downloads/cypress/e2e/download_csv.cy.ts index 740cad2cfb51..df8486047cbd 100644 --- a/system-tests/projects/downloads/cypress/e2e/download_csv.cy.ts +++ b/system-tests/projects/downloads/cypress/e2e/download_csv.cy.ts @@ -6,7 +6,9 @@ describe('downloads', () => { }) it('downloads cvs file', () => { - // wait 600ms after the click to wait/finish the download of the file - cy.get('[data-cy=download-csv]').click().wait(600) + // Note that we don't wait for the download here. If we ever need to do a wait here to ensure that the download happens, + // we need to make sure that wait is smaller than the proxy correlation timeout. Otherwise, we may be papering over a bug + // in the proxy correlation logic. + cy.get('[data-cy=download-csv]').click() }) }) diff --git a/system-tests/projects/downloads/cypress/e2e/downloads.cy.ts b/system-tests/projects/downloads/cypress/e2e/downloads.cy.ts index aeba6baa966e..1f5ccdaeb626 100644 --- a/system-tests/projects/downloads/cypress/e2e/downloads.cy.ts +++ b/system-tests/projects/downloads/cypress/e2e/downloads.cy.ts @@ -8,19 +8,25 @@ describe('downloads', () => { it('handles csv file download', () => { cy.get('[data-cy=download-csv]').click() cy - .readFile(`${Cypress.config('downloadsFolder')}/records.csv`) + // Note that this timeout is less than the proxy correlation timeout. It needs to be less than that timeout + // to ensure that we don't paper over a bug in the proxy correlation logic. + .readFile(`${Cypress.config('downloadsFolder')}/records.csv`, { timeout: 1000 }) .should('contain', '"Joe","Smith"') }) it('handles zip file download', () => { cy.get('[data-cy=download-zip]').click() // not worth adding a dependency to read contents, just ensure it's there - cy.readFile(`${Cypress.config('downloadsFolder')}/files.zip`) + // Note that this timeout is less than the proxy correlation timeout. It needs to be less than that timeout + // to ensure that we don't paper over a bug in the proxy correlation logic. + cy.readFile(`${Cypress.config('downloadsFolder')}/files.zip`, { timeout: 1000 }) }) it('handles xlsx file download', () => { cy.get('[data-cy=download-xlsx]').click() // not worth adding a dependency to read contents, just ensure it's there - cy.readFile(`${Cypress.config('downloadsFolder')}/people.xlsx`) + // Note that this timeout is less than the proxy correlation timeout. It needs to be less than that timeout + // to ensure that we don't paper over a bug in the proxy correlation logic. + cy.readFile(`${Cypress.config('downloadsFolder')}/people.xlsx`, { timeout: 1000 }) }) }) diff --git a/system-tests/test/downloads_spec.ts b/system-tests/test/downloads_spec.ts index ec6c3bbd1200..ec95c1594acb 100644 --- a/system-tests/test/downloads_spec.ts +++ b/system-tests/test/downloads_spec.ts @@ -10,7 +10,6 @@ describe('e2e downloads', () => { systemTests.setup() systemTests.it('handles various file downloads', { - browser: '!webkit', // TODO(webkit): fix+unskip (implement downloads support) project: 'downloads', spec: 'downloads.cy.ts', }) diff --git a/system-tests/test/protocol_spec.js b/system-tests/test/protocol_spec.js index 9e89523dbc67..8ac73046544a 100644 --- a/system-tests/test/protocol_spec.js +++ b/system-tests/test/protocol_spec.js @@ -10,7 +10,7 @@ const { // source: https://www.myintervals.com/blog/2009/05/20/iso-8601-date-validation-that-doesnt-suck/ const isoDateRegex = /"([\+-]?\d{4}(?!\d{2}\b))((-?)((0[1-9]|1[0-2])(\3([12]\d|0[1-9]|3[01]))?|W([0-4]\d|5[0-2])(-?[1-7])?|(00[1-9]|0[1-9]\d|[12]\d{2}|3([0-5]\d|6[1-6])))([T\s]((([01]\d|2[0-3])((:?)[0-5]\d)?|24\:?00)([\.,]\d+(?!:))?)?(\17[0-5]\d([\.,]\d+)?)?([zZ]|([\+-])([01]\d|2[0-3]):?([0-5]\d)?)?)?)?"/g -const numberRegex = /"(wallClockDuration|fnDuration|afterFnDuration|lifecycle|duration|timestamp|createdAtTimestamp|updatedAtTimestamp|x|y|top|left|topCenter|leftCenter|requestId)": \"?(0|[1-9]\d*)(\.\d+)?\"?/g +const numberRegex = /"(wallClockDuration|fnDuration|afterFnDuration|lifecycle|duration|timestamp|createdAtTimestamp|updatedAtTimestamp|x|y|top|left|topCenter|leftCenter|requestId|cdpRequestWillBeSentTimestamp|cdpRequestWillBeSentReceivedTimestamp|proxyRequestReceivedTimestamp|cdpLagDuration|proxyRequestCorrelationDuration)": \"?(0|[1-9]\d*)(\.\d+)?\"?/g const pathRegex = /"(name|absoluteFile)": "\/[^"]+"/g const componentSpecPathRegex = /"(url|message)": "(http:\/\/localhost:2121\/__cypress\/iframes\/index.html\?specPath=)(.*)(\/protocol\/src\/components\/)(.*)"/g