diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index d18c9525562d..00577e909d58 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -5,6 +5,9 @@ _Released 10/25/2023 (PENDING)_ **Bugfixes:** +- Fixed a performance regression in `13.3.1` with proxy correlation timeouts and requests issued from service workers. Fixes [#28054](https://github.com/cypress-io/cypress/issues/28054) and [#28056](https://github.com/cypress-io/cypress/issues/28056). +- Fixed an issue where proxy correlation would leak over from a previous spec causing performance problems, `cy.intercept` problems, and Test Replay asset capturing issues. Addressed in [#28060](https://github.com/cypress-io/cypress/pull/28060). +- Fixed an issue where redirects of requests that knowingly don't have CDP traffic should also be assumed to not have CDP traffic. Addressed in [#28060](https://github.com/cypress-io/cypress/pull/28060). - Fixed an issue with Accept Encoding headers by forcing gzip when no accept encoding header is sent and using identity if gzip is not sent. Fixes [#28025](https://github.com/cypress-io/cypress/issues/28025). **Dependency Updates:** diff --git a/packages/proxy/lib/http/index.ts b/packages/proxy/lib/http/index.ts index 433881759c05..3ccb9e80afbf 100644 --- a/packages/proxy/lib/http/index.ts +++ b/packages/proxy/lib/http/index.ts @@ -66,6 +66,7 @@ type HttpMiddlewareCtx = { getCookieJar: () => CookieJar deferSourceMapRewrite: (opts: { js: string, url: string }) => string getPreRequest: (cb: GetPreRequestCb) => void + addPendingUrlWithoutPreRequest: (url: string) => void getAUTUrl: Http['getAUTUrl'] setAUTUrl: Http['setAUTUrl'] simulatedCookies: SerializableAutomationCookie[] @@ -326,6 +327,9 @@ export class Http { getPreRequest: (cb) => { this.preRequests.get(ctx.req, ctx.debug, cb) }, + addPendingUrlWithoutPreRequest: (url) => { + this.preRequests.addPendingUrlWithoutPreRequest(url) + }, protocolManager: this.protocolManager, } @@ -411,6 +415,7 @@ export class Http { reset () { this.buffers.reset() this.setAUTUrl(undefined) + this.preRequests.reset() } setBuffer (buffer) { @@ -433,4 +438,8 @@ export class Http { this.protocolManager = protocolManager this.preRequests.setProtocolManager(protocolManager) } + + setPreRequestTimeout (timeout: number) { + this.preRequests.setPreRequestTimeout(timeout) + } } diff --git a/packages/proxy/lib/http/request-middleware.ts b/packages/proxy/lib/http/request-middleware.ts index d8ccd58b2fc4..04391b5fadfe 100644 --- a/packages/proxy/lib/http/request-middleware.ts +++ b/packages/proxy/lib/http/request-middleware.ts @@ -128,8 +128,9 @@ const CorrelateBrowserPreRequest: RequestMiddleware = async function () { } this.debug('waiting for prerequest') - this.getPreRequest(((browserPreRequest) => { + this.getPreRequest((({ browserPreRequest, noPreRequestExpected }) => { this.req.browserPreRequest = browserPreRequest + this.req.noPreRequestExpected = noPreRequestExpected copyResourceTypeAndNext() })) } diff --git a/packages/proxy/lib/http/response-middleware.ts b/packages/proxy/lib/http/response-middleware.ts index c1e534ceaa14..ef6d57e44ef2 100644 --- a/packages/proxy/lib/http/response-middleware.ts +++ b/packages/proxy/lib/http/response-middleware.ts @@ -656,6 +656,11 @@ const MaybeSendRedirectToClient: ResponseMiddleware = function () { return this.next() } + // If we're redirecting from a request that doesn't expect to have a preRequest (e.g. download links), we need to treat the redirected url as such as well. + if (this.req.noPreRequestExpected) { + this.addPendingUrlWithoutPreRequest(newUrl) + } + setInitialCookie(this.res, this.remoteStates.current(), true) this.debug('redirecting to new url %o', { statusCode, newUrl }) diff --git a/packages/proxy/lib/http/util/prerequests.ts b/packages/proxy/lib/http/util/prerequests.ts index ee2dc2cf3e1a..9759828af211 100644 --- a/packages/proxy/lib/http/util/prerequests.ts +++ b/packages/proxy/lib/http/util/prerequests.ts @@ -21,11 +21,16 @@ process.once('exit', () => { debug('metrics: %o', metrics) }) -export type GetPreRequestCb = (browserPreRequest?: BrowserPreRequestWithTimings) => void +export type CorrelationInformation = { + browserPreRequest?: BrowserPreRequestWithTimings + noPreRequestExpected?: boolean +} + +export type GetPreRequestCb = (correlationInformation: CorrelationInformation) => void type PendingRequest = { ctxDebug - callback: GetPreRequestCb + callback?: GetPreRequestCb timeout: NodeJS.Timeout timedOut?: boolean proxyRequestReceivedTimestamp: number @@ -74,6 +79,13 @@ class QueueMap { this.queues[queueKey].splice(i, 1) if (this.queues[queueKey].length === 0) delete this.queues[queueKey] } + + forEach (fn: (value: T) => void) { + Object.values(this.queues).forEach((queue) => { + queue.forEach(fn) + }) + } + get length () { return Object.values(this.queues).reduce((prev, cur) => prev + cur.length, 0) } @@ -148,11 +160,16 @@ export class PreRequests { debugVerbose('Incoming pre-request %s matches pending request. %o', key, browserPreRequest) if (!pendingRequest.timedOut) { clearTimeout(pendingRequest.timeout) - pendingRequest.callback({ - ...browserPreRequest, - ...timings, + pendingRequest.callback?.({ + browserPreRequest: { + ...browserPreRequest, + ...timings, + }, + noPreRequestExpected: false, }) + delete pendingRequest.callback + return } @@ -179,7 +196,11 @@ export class PreRequests { if (pendingRequest) { debugVerbose('Handling %s without a CDP prerequest', key) clearTimeout(pendingRequest.timeout) - pendingRequest.callback() + pendingRequest.callback?.({ + noPreRequestExpected: true, + }) + + delete pendingRequest.callback return } @@ -196,6 +217,19 @@ export class PreRequests { } get (req: CypressIncomingRequest, ctxDebug, callback: GetPreRequestCb) { + // The initial request that loads the service worker does not get sent to CDP and it happens prior + // to the service worker target being added. Thus, we need to explicitly ignore it. We determine + // it's the service worker request via the `sec-fetch-dest` header + if (req.headers['sec-fetch-dest'] === 'serviceworker') { + ctxDebug('Ignoring request with sec-fetch-dest: serviceworker', req.proxiedUrl) + + callback({ + noPreRequestExpected: true, + }) + + return + } + const proxyRequestReceivedTimestamp = performance.now() + performance.timeOrigin metrics.proxyRequestsReceived++ @@ -206,12 +240,15 @@ export class PreRequests { metrics.immediatelyMatchedRequests++ ctxDebug('Incoming request %s matches known pre-request: %o', key, pendingPreRequest) callback({ - ...pendingPreRequest.browserPreRequest, - cdpRequestWillBeSentTimestamp: pendingPreRequest.cdpRequestWillBeSentTimestamp, - cdpRequestWillBeSentReceivedTimestamp: pendingPreRequest.cdpRequestWillBeSentReceivedTimestamp, - proxyRequestReceivedTimestamp, - cdpLagDuration: pendingPreRequest.cdpRequestWillBeSentReceivedTimestamp - pendingPreRequest.cdpRequestWillBeSentTimestamp, - proxyRequestCorrelationDuration: Math.max(pendingPreRequest.cdpRequestWillBeSentReceivedTimestamp - proxyRequestReceivedTimestamp, 0), + browserPreRequest: { + ...pendingPreRequest.browserPreRequest, + cdpRequestWillBeSentTimestamp: pendingPreRequest.cdpRequestWillBeSentTimestamp, + cdpRequestWillBeSentReceivedTimestamp: pendingPreRequest.cdpRequestWillBeSentReceivedTimestamp, + proxyRequestReceivedTimestamp, + cdpLagDuration: pendingPreRequest.cdpRequestWillBeSentReceivedTimestamp - pendingPreRequest.cdpRequestWillBeSentTimestamp, + proxyRequestCorrelationDuration: Math.max(pendingPreRequest.cdpRequestWillBeSentReceivedTimestamp - proxyRequestReceivedTimestamp, 0), + }, + noPreRequestExpected: false, }) return @@ -222,7 +259,9 @@ export class PreRequests { if (pendingUrlWithoutPreRequests) { metrics.immediatelyMatchedRequests++ ctxDebug('Incoming request %s matches known pending url without pre request', key) - callback() + callback({ + noPreRequestExpected: true, + }) return } @@ -235,7 +274,11 @@ export class PreRequests { ctxDebug('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() + callback({ + noPreRequestExpected: false, + }) + + delete pendingRequest.callback }, this.requestTimeout), } @@ -245,4 +288,24 @@ export class PreRequests { setProtocolManager (protocolManager: ProtocolManagerShape) { this.protocolManager = protocolManager } + + setPreRequestTimeout (requestTimeout: number) { + this.requestTimeout = requestTimeout + } + + reset () { + this.pendingPreRequests = new QueueMap() + + // Clear out the pending requests timeout callbacks first then clear the queue + this.pendingRequests.forEach(({ callback, timeout }) => { + clearTimeout(timeout) + metrics.unmatchedRequests++ + callback?.({ + noPreRequestExpected: false, + }) + }) + + this.pendingRequests = new QueueMap() + this.pendingUrlsWithoutPreRequests = new QueueMap() + } } diff --git a/packages/proxy/lib/network-proxy.ts b/packages/proxy/lib/network-proxy.ts index 2702b059a1f7..a3c4996450ad 100644 --- a/packages/proxy/lib/network-proxy.ts +++ b/packages/proxy/lib/network-proxy.ts @@ -53,4 +53,8 @@ export class NetworkProxy { setProtocolManager (protocolManager) { this.http.setProtocolManager(protocolManager) } + + setPreRequestTimeout (timeout) { + this.http.setPreRequestTimeout(timeout) + } } diff --git a/packages/proxy/lib/types.ts b/packages/proxy/lib/types.ts index 94f73165441a..d27fa2b7f798 100644 --- a/packages/proxy/lib/types.ts +++ b/packages/proxy/lib/types.ts @@ -12,6 +12,7 @@ export type CypressIncomingRequest = Request & { abort: () => void requestId: string browserPreRequest?: BrowserPreRequestWithTimings + noPreRequestExpected?: boolean body?: string responseTimeout?: number followRedirect?: boolean diff --git a/packages/proxy/test/unit/http/util/prerequests.spec.ts b/packages/proxy/test/unit/http/util/prerequests.spec.ts index 309727358fe4..f607a823ee0b 100644 --- a/packages/proxy/test/unit/http/util/prerequests.spec.ts +++ b/packages/proxy/test/unit/http/util/prerequests.spec.ts @@ -1,4 +1,4 @@ -import { PreRequests } from '@packages/proxy/lib/http/util/prerequests' +import { CorrelationInformation, PreRequests } from '@packages/proxy/lib/http/util/prerequests' import { BrowserPreRequest, CypressIncomingRequest } from '@packages/proxy' import { expect } from 'chai' import sinon from 'sinon' @@ -68,22 +68,25 @@ describe('http/util/prerequests', () => { const cb = sinon.stub() - preRequests.get({ proxiedUrl: 'foo', method: 'GET' } as CypressIncomingRequest, () => {}, cb) + preRequests.get({ proxiedUrl: 'foo', method: 'GET', headers: {} } as CypressIncomingRequest, () => {}, cb) const { args } = cb.getCall(0) - 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) + const browserPreRequest = args[0].browserPreRequest + const noPreRequestExpected = args[0].noPreRequestExpected + + expect(browserPreRequest.requestId).to.eq(secondPreRequest.requestId) + expect(browserPreRequest.url).to.eq(secondPreRequest.url) + expect(browserPreRequest.method).to.eq(secondPreRequest.method) + expect(browserPreRequest.headers).to.deep.eq(secondPreRequest.headers) + expect(browserPreRequest.resourceType).to.eq(secondPreRequest.resourceType) + expect(browserPreRequest.originalResourceType).to.eq(secondPreRequest.originalResourceType) + expect(browserPreRequest.cdpRequestWillBeSentTimestamp).to.eq(secondPreRequest.cdpRequestWillBeSentTimestamp) + expect(browserPreRequest.cdpRequestWillBeSentReceivedTimestamp).to.eq(secondPreRequest.cdpRequestWillBeSentReceivedTimestamp) + expect(browserPreRequest.proxyRequestReceivedTimestamp).to.be.a('number') + expect(browserPreRequest.cdpLagDuration).to.eq(secondPreRequest.cdpRequestWillBeSentReceivedTimestamp - secondPreRequest.cdpRequestWillBeSentTimestamp) + expect(browserPreRequest.proxyRequestCorrelationDuration).to.eq(secondPreRequest.cdpRequestWillBeSentReceivedTimestamp - browserPreRequest.proxyRequestReceivedTimestamp) + + expect(noPreRequestExpected).to.be.false expectPendingCounts(0, 2) }) @@ -98,43 +101,47 @@ describe('http/util/prerequests', () => { const cb = sinon.stub() - preRequests.get({ proxiedUrl: 'foo', method: 'GET' } as CypressIncomingRequest, () => {}, cb) + preRequests.get({ proxiedUrl: 'foo', method: 'GET', headers: {} } as CypressIncomingRequest, () => {}, cb) const { args } = cb.getCall(0) const arg = args[0] - expect(arg).to.be.undefined + expect(arg.preRequest).to.be.undefined + expect(arg.noPreRequestExpected).to.be.true 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' }) + const cb = ({ browserPreRequest, noPreRequestExpected }: CorrelationInformation) => { + expect(browserPreRequest).to.include({ requestId: '1234', url: 'foo', method: 'GET' }) + expect(noPreRequestExpected).to.be.false expectPendingCounts(0, 0) done() } - preRequests.get({ proxiedUrl: 'foo', method: 'GET' } as CypressIncomingRequest, () => {}, cb) + preRequests.get({ proxiedUrl: 'foo', method: 'GET', headers: {} } as CypressIncomingRequest, () => {}, cb) preRequests.addPending({ requestId: '1234', url: 'foo', method: 'GET' } as BrowserPreRequest) }) it('synchronously matches a request without a pre-request added after the request', (done) => { - const cb = (preRequest) => { - expect(preRequest).to.be.undefined + const cb = ({ browserPreRequest, noPreRequestExpected }: CorrelationInformation) => { + expect(browserPreRequest).to.be.undefined + expect(noPreRequestExpected).to.be.true expectPendingCounts(0, 0) done() } - preRequests.get({ proxiedUrl: 'foo', method: 'GET' } as CypressIncomingRequest, () => {}, cb) + preRequests.get({ proxiedUrl: 'foo', method: 'GET', headers: {} } 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 + cb = ({ browserPreRequest, noPreRequestExpected }: CorrelationInformation) => { + expect(browserPreRequest).to.be.undefined + expect(noPreRequestExpected).to.be.false // we should have keep the pending request to eventually be correlated later, but don't block the body in the meantime expectPendingCounts(1, 0) @@ -143,7 +150,7 @@ describe('http/util/prerequests', () => { } }) - preRequests.get({ proxiedUrl: 'foo', method: 'GET' } as CypressIncomingRequest, () => {}, cb) + preRequests.get({ proxiedUrl: 'foo', method: 'GET', headers: {} } as CypressIncomingRequest, () => {}, cb) await cbPromise @@ -182,13 +189,14 @@ describe('http/util/prerequests', () => { // 2 * requestTimeout. We verify that it's gone (and therefore not leaking memory) by sending in a request // and assuring that the pre-request wasn't there to be matched anymore. setTimeout(() => { - const cb = (preRequest) => { - expect(preRequest).to.be.undefined + const cb = ({ browserPreRequest, noPreRequestExpected }: CorrelationInformation) => { + expect(browserPreRequest).to.be.undefined + expect(noPreRequestExpected).to.be.false expectPendingCounts(1, 0, 0) done() } - preRequests.get({ proxiedUrl: 'foo', method: 'GET' } as CypressIncomingRequest, () => {}, cb) + preRequests.get({ proxiedUrl: 'foo', method: 'GET', headers: {} } as CypressIncomingRequest, () => {}, cb) }, 1200) }) @@ -218,4 +226,32 @@ describe('http/util/prerequests', () => { expectPendingCounts(0, 2) }) + + it('immediately handles a request from a service worker loading', () => { + const cbServiceWorker = sinon.stub() + + preRequests.get({ proxiedUrl: 'foo', method: 'GET', headers: { 'sec-fetch-dest': 'serviceworker' } } as any, () => {}, cbServiceWorker) + + expect(cbServiceWorker).to.be.calledOnce + expect(cbServiceWorker).to.be.calledWith() + }) + + it('resets the queues', () => { + let callbackCalled = false + + preRequests.addPending({ requestId: '1234', url: 'bar', method: 'GET' } as BrowserPreRequest) + preRequests.get({ proxiedUrl: 'foo', method: 'GET', headers: {} } as CypressIncomingRequest, () => {}, () => { + callbackCalled = true + }) + + preRequests.addPendingUrlWithoutPreRequest('baz') + + expectPendingCounts(1, 1, 1) + + preRequests.reset() + + expectPendingCounts(0, 0, 0) + + expect(callbackCalled).to.be.true + }) }) diff --git a/packages/server/lib/browsers/browser-cri-client.ts b/packages/server/lib/browsers/browser-cri-client.ts index 4d6d536731a6..a084f2bd0cab 100644 --- a/packages/server/lib/browsers/browser-cri-client.ts +++ b/packages/server/lib/browsers/browser-cri-client.ts @@ -145,13 +145,18 @@ export class BrowserCriClient { return retryWithIncreasingDelay(async () => { const versionInfo = await CRI.Version({ host, port, useHostName: true }) - const browserClient = await create(versionInfo.webSocketDebuggerUrl, onAsynchronousError, undefined, undefined, onReconnect) + + const browserClient = await create({ + target: versionInfo.webSocketDebuggerUrl, + onAsynchronousError, + onReconnect, + protocolManager, + }) const browserCriClient = new BrowserCriClient(browserClient, versionInfo, host!, port, browserName, onAsynchronousError, protocolManager) if (fullyManageTabs) { await browserClient.send('Target.setDiscoverTargets', { discover: true }) - browserClient.on('Target.targetDestroyed', (event) => { debug('Target.targetDestroyed %o', { event, @@ -265,7 +270,7 @@ export class BrowserCriClient { throw new Error(`Could not find url target in browser ${url}. Targets were ${JSON.stringify(targets)}`) } - this.currentlyAttachedTarget = await create(target.targetId, this.onAsynchronousError, this.host, this.port) + this.currentlyAttachedTarget = await create({ target: target.targetId, onAsynchronousError: this.onAsynchronousError, host: this.host, port: this.port, protocolManager: this.protocolManager }) await this.protocolManager?.connectToBrowser(this.currentlyAttachedTarget) return this.currentlyAttachedTarget @@ -312,7 +317,13 @@ export class BrowserCriClient { } if (target) { - this.currentlyAttachedTarget = await create(target.targetId, this.onAsynchronousError, this.host, this.port) + this.currentlyAttachedTarget = await create({ + target: target.targetId, + onAsynchronousError: this.onAsynchronousError, + host: this.host, + port: this.port, + protocolManager: this.protocolManager, + }) } else { this.currentlyAttachedTarget = undefined } diff --git a/packages/server/lib/browsers/cri-client.ts b/packages/server/lib/browsers/cri-client.ts index 8961b07e998d..2b983b3b05fd 100644 --- a/packages/server/lib/browsers/cri-client.ts +++ b/packages/server/lib/browsers/cri-client.ts @@ -8,6 +8,7 @@ import type WebSocket from 'ws' import type CDP from 'chrome-remote-interface' import type { SendDebuggerCommand, OnFn, CdpCommand, CdpEvent } from './cdp_automation' +import type { ProtocolManagerShape } from '@packages/types' const debug = debugModule('cypress:server:browsers:cri-client') // debug using cypress-verbose:server:browsers:cri-client:send:* @@ -130,14 +131,23 @@ const maybeDebugCdpMessages = (cri: CDPClient) => { } type DeferredPromise = { resolve: Function, reject: Function } +type CreateParams = { + target: string + onAsynchronousError: Function + host?: string + port?: number + onReconnect?: (client: CriClient) => void + protocolManager?: ProtocolManagerShape +} -export const create = async ( - target: string, - onAsynchronousError: Function, - host?: string, - port?: number, - onReconnect?: (client: CriClient) => void, -): Promise => { +export const create = async ({ + target, + onAsynchronousError, + host, + port, + onReconnect, + protocolManager, +}: CreateParams): Promise => { const subscriptions: Subscription[] = [] const enableCommands: EnableCommand[] = [] let enqueuedCommands: EnqueuedCommand[] = [] @@ -252,6 +262,32 @@ export const create = async ( debug('crash detected') crashed = true }) + + // We only want to try and add service worker traffic if we have a host set. This indicates that this is the child cri client. + if (host) { + cri.on('Target.targetCreated', async (event) => { + if (event.targetInfo.type === 'service_worker') { + const networkEnabledOptions = protocolManager?.protocolEnabled ? { + maxTotalBufferSize: 0, + maxResourceBufferSize: 0, + maxPostDataSize: 64 * 1024, + } : { + maxTotalBufferSize: 0, + maxResourceBufferSize: 0, + maxPostDataSize: 0, + } + + const { sessionId } = await cri.send('Target.attachToTarget', { + targetId: event.targetInfo.targetId, + flatten: true, + }) + + await cri.send('Network.enable', networkEnabledOptions, sessionId) + } + }) + + await cri.send('Target.setDiscoverTargets', { discover: true }) + } } await connect() diff --git a/packages/server/lib/browsers/electron.ts b/packages/server/lib/browsers/electron.ts index 33aed67a3866..31583a06735b 100644 --- a/packages/server/lib/browsers/electron.ts +++ b/packages/server/lib/browsers/electron.ts @@ -307,7 +307,10 @@ export = { cdpSocketServer?.attachCDPClient(cdpAutomation), videoApi && recordVideo(cdpAutomation, videoApi), this._handleDownloads(win, options.downloadsFolder, automation), - await utils.handleDownloadLinksViaCDP(pageCriClient, automation), + utils.handleDownloadLinksViaCDP(pageCriClient, automation), + // Ensure to clear browser state in between runs. This is handled differently in browsers when we launch new tabs, but we don't have that concept in electron + pageCriClient.send('Storage.clearDataForOrigin', { origin: '*', storageTypes: 'all' }), + pageCriClient.send('Network.clearBrowserCache'), ]) } diff --git a/packages/server/lib/project-base.ts b/packages/server/lib/project-base.ts index 129a38908316..5db44104b346 100644 --- a/packages/server/lib/project-base.ts +++ b/packages/server/lib/project-base.ts @@ -436,6 +436,12 @@ export class ProjectBase extends EE { setCurrentSpecAndBrowser (spec, browser: FoundBrowser) { this.spec = spec this.browser = browser + + if (this.browser.family !== 'chromium') { + // If we're not in chromium, our strategy for correlating service worker prerequests doesn't work in non-chromium browsers (https://github.com/cypress-io/cypress/issues/28079) + // in order to not hang for 2 seconds, we override the prerequest timeout to be 500 ms (which is what it has been historically) + this._server?.setPreRequestTimeout(500) + } } get protocolManager (): ProtocolManager | undefined { diff --git a/packages/server/lib/server-base.ts b/packages/server/lib/server-base.ts index cf6e15814382..6c36769ac8e6 100644 --- a/packages/server/lib/server-base.ts +++ b/packages/server/lib/server-base.ts @@ -218,6 +218,10 @@ export class ServerBase { this._networkProxy?.setProtocolManager(protocolManager) } + setPreRequestTimeout (timeout: number) { + this._networkProxy?.setPreRequestTimeout(timeout) + } + setupCrossOriginRequestHandling () { this._eventBus.on('cross:origin:cookies', (cookies: SerializableAutomationCookie[]) => { this.socket.localBus.once('cross:origin:cookies:received', () => { diff --git a/packages/server/test/integration/cdp_spec.ts b/packages/server/test/integration/cdp_spec.ts index fbf2f614f3c7..b38f4208a363 100644 --- a/packages/server/test/integration/cdp_spec.ts +++ b/packages/server/test/integration/cdp_spec.ts @@ -114,13 +114,11 @@ describe('CDP Clients', () => { const onAsynchronousError = reject const onReconnect = resolve - criClient = await CriClient.create( - `ws://127.0.0.1:${wsServerPort}`, + criClient = await CriClient.create({ + target: `ws://127.0.0.1:${wsServerPort}`, onAsynchronousError, - undefined, - undefined, onReconnect, - ) + }) criClient.onReconnectAttempt = stub @@ -146,13 +144,11 @@ describe('CDP Clients', () => { const onAsynchronousError = resolve const onReconnect = reject - criClient = await CriClient.create( - `ws://127.0.0.1:${wsServerPort}`, + criClient = await CriClient.create({ + target: `ws://127.0.0.1:${wsServerPort}`, onAsynchronousError, - undefined, - undefined, onReconnect, - ) + }) criClient.onReconnectAttempt = stub @@ -203,13 +199,11 @@ describe('CDP Clients', () => { return new Promise(async (resolve, reject) => { const onAsynchronousError = reject - criClient = await CriClient.create( - `ws://127.0.0.1:${wsServerPort}`, + criClient = await CriClient.create({ + target: `ws://127.0.0.1:${wsServerPort}`, onAsynchronousError, - undefined, - undefined, onReconnect, - ) + }) criClient.onReconnectAttempt = stub @@ -303,13 +297,11 @@ describe('CDP Clients', () => { const onAsynchronousError = reject const onReconnect = reject - criClient = await CriClient.create( - `ws://127.0.0.1:${wsServerPort}`, + criClient = await CriClient.create({ + target: `ws://127.0.0.1:${wsServerPort}`, onAsynchronousError, - undefined, - undefined, onReconnect, - ) + }) const stub = sinon.stub().onThirdCall().callsFake(async () => { criClient.close() diff --git a/packages/server/test/integration/cypress_spec.js b/packages/server/test/integration/cypress_spec.js index 1a0f9dada530..ca48af7aae03 100644 --- a/packages/server/test/integration/cypress_spec.js +++ b/packages/server/test/integration/cypress_spec.js @@ -1025,6 +1025,8 @@ describe('lib/cypress', () => { sinon.stub(chromeBrowser, '_setAutomation').returns(cdpAutomation) + sinon.stub(ProjectBase.prototype, 'resetBrowserState').resolves() + return cypress.start([ `--run-project=${this.pluginBrowser}`, '--browser=chrome', diff --git a/packages/server/test/unit/browsers/browser-cri-client_spec.ts b/packages/server/test/unit/browsers/browser-cri-client_spec.ts index abae2f836303..266b3dce36ed 100644 --- a/packages/server/test/unit/browsers/browser-cri-client_spec.ts +++ b/packages/server/test/unit/browsers/browser-cri-client_spec.ts @@ -39,7 +39,7 @@ describe('lib/browsers/cri-client', function () { send = sinon.stub() close = sinon.stub() - criClientCreateStub = sinon.stub(CriClient, 'create').withArgs('http://web/socket/url', onError).resolves({ + criClientCreateStub = sinon.stub(CriClient, 'create').withArgs({ target: 'http://web/socket/url', onAsynchronousError: onError, onReconnect: undefined, protocolManager: undefined }).resolves({ send, close, }) @@ -48,7 +48,14 @@ describe('lib/browsers/cri-client', function () { 'chrome-remote-interface': criImport, }) - getClient = (protocolManager) => browserCriClient.BrowserCriClient.create(['127.0.0.1'], PORT, 'Chrome', onError, undefined, protocolManager) + getClient = (protocolManager) => { + criClientCreateStub = criClientCreateStub.withArgs({ target: 'http://web/socket/url', onAsynchronousError: onError, onReconnect: undefined, protocolManager }).resolves({ + send, + close, + }) + + return browserCriClient.BrowserCriClient.create(['127.0.0.1'], PORT, 'Chrome', onError, undefined, protocolManager) + } }) context('.create', function () { @@ -143,7 +150,7 @@ describe('lib/browsers/cri-client', function () { const mockPageClient = {} send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) - criClientCreateStub.withArgs('1', onError, HOST, PORT).resolves(mockPageClient) + criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined }).resolves(mockPageClient) const browserClient = await getClient() @@ -159,7 +166,7 @@ describe('lib/browsers/cri-client', function () { } send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) - criClientCreateStub.withArgs('1', onError, HOST, PORT).resolves(mockPageClient) + criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager }).resolves(mockPageClient) const browserClient = await getClient(protocolManager) @@ -180,7 +187,7 @@ describe('lib/browsers/cri-client', function () { send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }, { targetId: '3', url: 'http://baz.com' }] }) - criClientCreateStub.withArgs('1', onError).resolves(mockPageClient) + criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined }).resolves(mockPageClient) const browserClient = await getClient() @@ -198,7 +205,7 @@ describe('lib/browsers/cri-client', function () { send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) - criClientCreateStub.withArgs('1', onError).resolves(mockPageClient) + criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined }).resolves(mockPageClient) const browserClient = await getClient() @@ -219,7 +226,7 @@ describe('lib/browsers/cri-client', function () { send.withArgs('Target.createTarget', { url: 'about:blank' }).resolves(mockUpdatedCurrentlyAttachedTarget) send.withArgs('Target.closeTarget', { targetId: '100' }).resolves() - criClientCreateStub.withArgs('101', onError, HOST, PORT).resolves(mockUpdatedCurrentlyAttachedTarget) + criClientCreateStub.withArgs({ target: '101', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined }).resolves(mockUpdatedCurrentlyAttachedTarget) const browserClient = await getClient() as any diff --git a/packages/server/test/unit/browsers/cri-client_spec.ts b/packages/server/test/unit/browsers/cri-client_spec.ts index 1eaf964401d4..7b5d90dea71d 100644 --- a/packages/server/test/unit/browsers/cri-client_spec.ts +++ b/packages/server/test/unit/browsers/cri-client_spec.ts @@ -50,7 +50,7 @@ describe('lib/browsers/cri-client', function () { }) getClient = () => { - return criClient.create(DEBUGGER_URL, onError) + return criClient.create({ target: DEBUGGER_URL, onAsynchronousError: onError }) } }) diff --git a/packages/server/test/unit/browsers/electron_spec.js b/packages/server/test/unit/browsers/electron_spec.js index 4e7ec86c602f..1fe7fc2d5c76 100644 --- a/packages/server/test/unit/browsers/electron_spec.js +++ b/packages/server/test/unit/browsers/electron_spec.js @@ -356,6 +356,14 @@ describe('lib/browsers/electron', () => { }) }) + it('expects the browser to be reset', function () { + return electron._launch(this.win, this.url, this.automation, this.options, undefined, undefined, { attachCDPClient: sinon.stub() }) + .then(() => { + expect(this.pageCriClient.send).to.be.calledWith('Storage.clearDataForOrigin', { origin: '*', storageTypes: 'all' }) + expect(this.pageCriClient.send).to.be.calledWith('Network.clearBrowserCache') + }) + }) + it('registers onRequest automation middleware and calls show when requesting to be focused', function () { sinon.spy(this.automation, 'use') diff --git a/system-tests/projects/e2e/browser_reset.html b/system-tests/projects/e2e/browser_reset.html new file mode 100644 index 000000000000..a54441e3dcf8 --- /dev/null +++ b/system-tests/projects/e2e/browser_reset.html @@ -0,0 +1,12 @@ + + + + + + + +

hi

+ + diff --git a/system-tests/projects/e2e/cypress/e2e/browser_reset_first_spec.cy.js b/system-tests/projects/e2e/cypress/e2e/browser_reset_first_spec.cy.js index 74351747c794..322e12669b75 100644 --- a/system-tests/projects/e2e/cypress/e2e/browser_reset_first_spec.cy.js +++ b/system-tests/projects/e2e/cypress/e2e/browser_reset_first_spec.cy.js @@ -17,7 +17,32 @@ const req = (win) => { }) } +const indexedDB = (win) => { + return new Promise((resolve, reject) => { + const DBOpenRequest = win.indexedDB.open('toDoList', 1) + + DBOpenRequest.onupgradeneeded = (e) => { + console.log('on upgrade needed') + const db = e.target.result + + try { + db.createObjectStore('toDoList') + } catch (error) { + console.log(error) + } + + resolve(win) + } + }) +} + +const swReq = (win) => { + return win.navigator?.serviceWorker?.ready.then(() => win) +} + it('makes cached request', () => { - cy.visit('http://localhost:1515') + cy.visit('http://localhost:1515/browser_reset.html') .then(req) // this creates the disk cache + .then(indexedDB) // this creates the indexedDB + .then(swReq) // this creates the service worker }) diff --git a/system-tests/projects/e2e/cypress/e2e/browser_reset_second_spec.cy.js b/system-tests/projects/e2e/cypress/e2e/browser_reset_second_spec.cy.js index 7facff182c3d..79d1257e4f17 100644 --- a/system-tests/projects/e2e/cypress/e2e/browser_reset_second_spec.cy.js +++ b/system-tests/projects/e2e/cypress/e2e/browser_reset_second_spec.cy.js @@ -17,7 +17,27 @@ const req = (win) => { }) } +const indexedDB = (win) => { + return new Promise((resolve, reject) => { + const DBOpenRequest = win.indexedDB.open('toDoList') + + DBOpenRequest.onsuccess = (e) => { + const db = e.target.result + + expect(db.objectStoreNames.contains('toDoList')).to.be.false + + resolve(win) + } + }) +} + +const swReq = (win) => { + return win.navigator?.serviceWorker?.ready.then(() => win) +} + it('makes cached request', () => { - cy.visit('http://localhost:1515') + cy.visit('http://localhost:1515/browser_reset.html') .then(req) // this should hit our server even though cached in the first spec + .then(indexedDB) // this ensures the indexedDB is empty + .then(swReq) // this ensures the service worker is not registered }) diff --git a/system-tests/projects/e2e/cypress/e2e/service_worker.cy.js b/system-tests/projects/e2e/cypress/e2e/service_worker.cy.js new file mode 100644 index 000000000000..75bb104cfd93 --- /dev/null +++ b/system-tests/projects/e2e/cypress/e2e/service_worker.cy.js @@ -0,0 +1,18 @@ +const swReq = (win) => { + return win.navigator?.serviceWorker?.ready.then(() => win) +} + +// Timeout of 1500 will ensure that the proxy correlation timeout is not hit +it('loads service worker', { defaultCommandTimeout: 1500 }, () => { + cy.visit('https://localhost:1515/service_worker.html') + .then(swReq) +}) + +// Load the service worker again to ensure that the service worker cache +// can be loaded properly. There are requests that are made with the +// cache that have different headers that need to be tested in the proxy. +// Timeout of 1500 will ensure that the proxy correlation timeout is not hit +it('loads service worker', { defaultCommandTimeout: 1500 }, () => { + cy.visit('https://localhost:1515/service_worker.html') + .then(swReq) +}) diff --git a/system-tests/projects/e2e/service-worker.js b/system-tests/projects/e2e/service-worker.js new file mode 100644 index 000000000000..19a562f0f494 --- /dev/null +++ b/system-tests/projects/e2e/service-worker.js @@ -0,0 +1,28 @@ +const activate = async () => { + self.clients.claim() + if (self.registration.navigationPreload) { + await self.registration.navigationPreload.enable() + } +} + +self.addEventListener('activate', (event) => { + event.waitUntil(activate()) +}) + +self.addEventListener('install', function (event) { + event.waitUntil( + caches.open('v1').then(function (cache) { + return cache.addAll([ + '/cached-sw', + ]) + }), + ) +}) + +self.addEventListener('fetch', function (event) { + event.respondWith( + caches.match(event.request).then(function (response) { + return response || fetch(event.request) + }), + ) +}) diff --git a/system-tests/projects/e2e/service_worker.html b/system-tests/projects/e2e/service_worker.html new file mode 100644 index 000000000000..7c9a6a19ba92 --- /dev/null +++ b/system-tests/projects/e2e/service_worker.html @@ -0,0 +1,13 @@ + + + + + + + + +

hi

+ + diff --git a/system-tests/test/browser_reset_spec.js b/system-tests/test/browser_reset_spec.js index 932e9d4c30d8..99ab4256922a 100644 --- a/system-tests/test/browser_reset_spec.js +++ b/system-tests/test/browser_reset_spec.js @@ -5,8 +5,12 @@ const systemTests = require('../lib/system-tests').default const e2ePath = Fixtures.projectPath('e2e') let requestsForCache = 0 +let requestsForServiceWorkerCache = 0 const onServer = function (app) { + requestsForCache = 0 + requestsForServiceWorkerCache = 0 + app.use(express.static(e2ePath, { // force caching to happen maxAge: 3600000, @@ -19,6 +23,13 @@ const onServer = function (app) { .set('cache-control', 'public, max-age=3600') .send('this response will be disk cached') }) + + app.get('/cached-sw', (req, res) => { + requestsForServiceWorkerCache += 1 + + return res + .send('this response will be disk cached by service worker') + }) } describe('e2e browser reset', () => { @@ -32,9 +43,13 @@ describe('e2e browser reset', () => { systemTests.it('executes two specs with a cached call', { project: 'e2e', spec: 'browser_reset_first_spec.cy.js,browser_reset_second_spec.cy.js', - onRun: async (exec) => { + onRun: async (exec, browser) => { await exec() + // Ensure that the cache was not used for either response expect(requestsForCache).to.eq(2) + if (browser.family === 'chromium') { + expect(requestsForServiceWorkerCache).to.eq(2) + } }, }) }) diff --git a/system-tests/test/service_worker_spec.js b/system-tests/test/service_worker_spec.js new file mode 100644 index 000000000000..5969288d433f --- /dev/null +++ b/system-tests/test/service_worker_spec.js @@ -0,0 +1,57 @@ +const express = require('express') +const Fixtures = require('../lib/fixtures') +const systemTests = require('../lib/system-tests').default + +const e2ePath = Fixtures.projectPath('e2e') + +let requestsForServiceWorkerCache = 0 + +const onServer = function (app) { + app.use(express.static(e2ePath, { + // force caching to happen + maxAge: 3600000, + })) + + app.get('/cached-sw', (req, res) => { + res.set({ + 'Access-Control-Allow-Origin': '*', + }) + + // redirect to cached-sw-redirect on a cross origin server + return res.redirect('https://localhost:1516/cached-sw-redirect') + }) + + app.get('/cached-sw-redirect', (req, res) => { + requestsForServiceWorkerCache += 1 + res.set({ + 'Access-Control-Allow-Origin': '*', + }) + + return res.send('this response will be used by service worker') + }) +} + +describe('e2e browser reset', () => { + systemTests.setup({ + servers: [{ + https: true, + port: 1515, + onServer, + }, { + https: true, + port: 1516, + onServer, + }], + }) + + systemTests.it('executes one spec with a cached call', { + project: 'e2e', + spec: 'service_worker.cy.js', + onRun: async (exec, browser) => { + await exec() + // Ensure that we only called this once even though we loaded the + // service worker twice + expect(requestsForServiceWorkerCache).to.eq(1) + }, + }) +})