From 74a06c53b886e73f4943f381aefef3697fc1b551 Mon Sep 17 00:00:00 2001 From: janlengyel Date: Mon, 23 Oct 2023 21:32:39 +0200 Subject: [PATCH 1/5] fix: ignore dash, underscore and space in search (#28012) Co-authored-by: Matt Schile Co-authored-by: Matt Schile --- cli/CHANGELOG.md | 8 +++ packages/app/cypress/e2e/specs_list_e2e.cy.ts | 61 +++++++++++++++++++ packages/app/src/specs/spec-utils.ts | 15 ++++- .../app/src/specs/tree/useCollapsibleTree.ts | 2 + 4 files changed, 84 insertions(+), 2 deletions(-) diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 83704f784c3c..79cfe638b208 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -1,4 +1,12 @@ +## 13.3.3 + +_Released 10/24/2023 (PENDING)_ + +**Bugfixes:** + +- Fixed a regression in [10.0.0](#10.0.0), where search would not find a spec if the file name contains "-" or "\_", but search prompt contains " " instead (e.g. search file "spec-file.cy.ts" with prompt "spec file"). Fixes [#25303](https://github.com/cypress-io/cypress/issues/25303). + ## 13.3.2 _Released 10/18/2023_ diff --git a/packages/app/cypress/e2e/specs_list_e2e.cy.ts b/packages/app/cypress/e2e/specs_list_e2e.cy.ts index 06a1ab33123d..38db09b577f0 100644 --- a/packages/app/cypress/e2e/specs_list_e2e.cy.ts +++ b/packages/app/cypress/e2e/specs_list_e2e.cy.ts @@ -1,4 +1,5 @@ import { getPathForPlatform } from '../../src/paths' +import path from 'path' describe('App: Spec List (E2E)', () => { const launchApp = (specFilter?: string) => { @@ -253,6 +254,66 @@ describe('App: Spec List (E2E)', () => { cy.findByText('No specs matched your search:').should('not.be.visible') }) + it('searches specs with "-" or "_" when search contains space', function () { + clearSearchAndType('accounts list') + + cy.findAllByTestId('spec-item') + .should('have.length', 1) + .and('contain', 'accounts_list.spec.js') + + cy.findByText('No specs matched your search:').should('not.be.visible') + }) + + it('searches specs with "-" or "_" when search contains "-"', function () { + clearSearchAndType('accounts-list') + + cy.findAllByTestId('spec-item') + .should('have.length', 1) + .and('contain', 'accounts_list.spec.js') + + cy.findByText('No specs matched your search:').should('not.be.visible') + }) + + it('searches specs with "-" or "_" when search contains "_"', function () { + clearSearchAndType('accounts_list') + + cy.findAllByTestId('spec-item') + .should('have.length', 1) + .and('contain', 'accounts_list.spec.js') + + cy.findByText('No specs matched your search:').should('not.be.visible') + }) + + it('searches folders with "-" or "_" when search contains space', function () { + clearSearchAndType('a b c') + + cy.findAllByTestId('spec-list-directory') + .should('have.length', 1) + .and('contain', path.join('cypress', 'e2e', 'a-b_c')) + + cy.findByText('No specs matched your search:').should('not.be.visible') + }) + + it('searches folders with "-" or "_" when search contains "-"', function () { + clearSearchAndType('a-b-c') + + cy.findAllByTestId('spec-list-directory') + .should('have.length', 1) + .and('contain', path.join('cypress', 'e2e', 'a-b_c')) + + cy.findByText('No specs matched your search:').should('not.be.visible') + }) + + it('searches folders with "-" or "_" when search contains "_"', function () { + clearSearchAndType('a_b_c') + + cy.findAllByTestId('spec-list-directory') + .should('have.length', 1) + .and('contain', path.join('cypress', 'e2e', 'a-b_c')) + + cy.findByText('No specs matched your search:').should('not.be.visible') + }) + it('saves the filter when navigating to a spec and back', function () { const targetSpecFile = 'accounts_list.spec.js' diff --git a/packages/app/src/specs/spec-utils.ts b/packages/app/src/specs/spec-utils.ts index 5e725ac13a6c..87364a9f1aaf 100644 --- a/packages/app/src/specs/spec-utils.ts +++ b/packages/app/src/specs/spec-utils.ts @@ -5,10 +5,10 @@ import _ from 'lodash' import { FuzzyFoundSpec, getPlatform } from './tree/useCollapsibleTree' export function fuzzySortSpecs (specs: T[], searchValue: string) { - const normalizedSearchValue = getPlatform() === 'win32' ? searchValue.replaceAll('/', '\\') : searchValue + const normalizedSearchValue = normalizeSpecValue(searchValue) const fuzzySortResult = fuzzySort - .go(normalizedSearchValue, specs, { keys: ['relative', 'baseName'], allowTypo: false, threshold: -3000 }) + .go(normalizedSearchValue, specs, { keys: ['normalizedRelative', 'normalizedBaseName'], allowTypo: false, threshold: -3000 }) .map((result) => { const [relative, baseName] = result @@ -24,9 +24,20 @@ export function fuzzySortSpecs (specs: T[], searchVal return fuzzySortResult } +function normalizeSpecValue (name: string) { + const escapedPath = getPlatform() === 'win32' ? name.replaceAll('/', '\\') : name + // replace dash, underscore and space with common character (in this case dash) + // they are replaced and not removed to preserve string length (so highlighting works correctly) + const normalizedSymbols = escapedPath.replace(/[-_\s]/g, '-') + + return normalizedSymbols +} + export function makeFuzzyFoundSpec (spec: FoundSpec): FuzzyFoundSpec { return { ...spec, + normalizedBaseName: normalizeSpecValue(spec.baseName), + normalizedRelative: normalizeSpecValue(spec.relative), fuzzyIndexes: { relative: [], baseName: [], diff --git a/packages/app/src/specs/tree/useCollapsibleTree.ts b/packages/app/src/specs/tree/useCollapsibleTree.ts index d15215753660..7c9062ec01c7 100644 --- a/packages/app/src/specs/tree/useCollapsibleTree.ts +++ b/packages/app/src/specs/tree/useCollapsibleTree.ts @@ -11,6 +11,8 @@ export type RawNode = { } export type FuzzyFoundSpec = T & { + normalizedBaseName: string + normalizedRelative: string fuzzyIndexes: { relative: number[] baseName: number[] From d9606868c5201fc7dde5645094b756602b65f4f7 Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Mon, 23 Oct 2023 16:08:05 -0500 Subject: [PATCH 2/5] fix: fix failures and correlation in proxy (#28094) --- cli/CHANGELOG.md | 1 + packages/proxy/lib/http/index.ts | 20 +++- packages/proxy/lib/http/request-middleware.ts | 9 +- packages/proxy/lib/http/util/prerequests.ts | 22 +++- .../test/unit/http/util/prerequests.spec.ts | 25 +++- .../test/integration/http_requests_spec.js | 107 +++++++++++++++++- 6 files changed, 171 insertions(+), 13 deletions(-) diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 79cfe638b208..0b7dfbe0a425 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -5,6 +5,7 @@ _Released 10/24/2023 (PENDING)_ **Bugfixes:** +- Fixed a performance problem with proxy correlation when requests get aborted and then get miscorrelated with follow up requests. Fixed in [#28094](https://github.com/cypress-io/cypress/pull/28094). - Fixed a regression in [10.0.0](#10.0.0), where search would not find a spec if the file name contains "-" or "\_", but search prompt contains " " instead (e.g. search file "spec-file.cy.ts" with prompt "spec file"). Fixes [#25303](https://github.com/cypress-io/cypress/issues/25303). ## 13.3.2 diff --git a/packages/proxy/lib/http/index.ts b/packages/proxy/lib/http/index.ts index 3ccb9e80afbf..7549e508e67e 100644 --- a/packages/proxy/lib/http/index.ts +++ b/packages/proxy/lib/http/index.ts @@ -9,7 +9,7 @@ import ErrorMiddleware from './error-middleware' import RequestMiddleware from './request-middleware' import ResponseMiddleware from './response-middleware' import { HttpBuffers } from './util/buffers' -import { GetPreRequestCb, PreRequests } from './util/prerequests' +import { GetPreRequestCb, PendingRequest, PreRequests } from './util/prerequests' import type EventEmitter from 'events' import type CyServer from '@packages/server' @@ -63,10 +63,12 @@ type HttpMiddlewareCtx = { stage: HttpStages debug: Debug.Debugger middleware: HttpMiddlewareStacks + pendingRequest: PendingRequest | undefined getCookieJar: () => CookieJar deferSourceMapRewrite: (opts: { js: string, url: string }) => string - getPreRequest: (cb: GetPreRequestCb) => void + getPreRequest: (cb: GetPreRequestCb) => PendingRequest | undefined addPendingUrlWithoutPreRequest: (url: string) => void + removePendingRequest: (pendingRequest: PendingRequest) => void getAUTUrl: Http['getAUTUrl'] setAUTUrl: Http['setAUTUrl'] simulatedCookies: SerializableAutomationCookie[] @@ -325,15 +327,25 @@ export class Http { getAUTUrl: this.getAUTUrl, setAUTUrl: this.setAUTUrl, getPreRequest: (cb) => { - this.preRequests.get(ctx.req, ctx.debug, cb) + return this.preRequests.get(ctx.req, ctx.debug, cb) }, addPendingUrlWithoutPreRequest: (url) => { this.preRequests.addPendingUrlWithoutPreRequest(url) }, + removePendingRequest: (pendingRequest: PendingRequest) => { + this.preRequests.removePendingRequest(pendingRequest) + }, protocolManager: this.protocolManager, } const onError = (error: Error): Promise => { + const pendingRequest = ctx.pendingRequest as PendingRequest | undefined + + if (pendingRequest) { + delete ctx.pendingRequest + ctx.removePendingRequest(pendingRequest) + } + ctx.error = error if (ctx.req.browserPreRequest && !ctx.req.browserPreRequest.errorHandled) { ctx.req.browserPreRequest.errorHandled = true @@ -427,7 +439,7 @@ export class Http { } removePendingBrowserPreRequest (requestId: string) { - this.preRequests.removePending(requestId) + this.preRequests.removePendingPreRequest(requestId) } addPendingUrlWithoutPreRequest (url: string) { diff --git a/packages/proxy/lib/http/request-middleware.ts b/packages/proxy/lib/http/request-middleware.ts index 04391b5fadfe..08d4490a1c33 100644 --- a/packages/proxy/lib/http/request-middleware.ts +++ b/packages/proxy/lib/http/request-middleware.ts @@ -128,7 +128,7 @@ const CorrelateBrowserPreRequest: RequestMiddleware = async function () { } this.debug('waiting for prerequest') - this.getPreRequest((({ browserPreRequest, noPreRequestExpected }) => { + this.pendingRequest = this.getPreRequest((({ browserPreRequest, noPreRequestExpected }) => { this.req.browserPreRequest = browserPreRequest this.req.noPreRequestExpected = noPreRequestExpected copyResourceTypeAndNext() @@ -455,6 +455,13 @@ const SendRequestOutgoing: RequestMiddleware = function () { this.debug('request aborted') // if the request is aborted, close out the middleware span and http span. the response middleware did not run + const pendingRequest = this.pendingRequest + + if (pendingRequest) { + delete this.pendingRequest + this.removePendingRequest(pendingRequest) + } + this.reqMiddlewareSpan?.setAttributes({ requestAborted: true, }) diff --git a/packages/proxy/lib/http/util/prerequests.ts b/packages/proxy/lib/http/util/prerequests.ts index 9759828af211..cbdd41f9a13f 100644 --- a/packages/proxy/lib/http/util/prerequests.ts +++ b/packages/proxy/lib/http/util/prerequests.ts @@ -28,7 +28,8 @@ export type CorrelationInformation = { export type GetPreRequestCb = (correlationInformation: CorrelationInformation) => void -type PendingRequest = { +export type PendingRequest = { + key: string ctxDebug callback?: GetPreRequestCb timeout: NodeJS.Timeout @@ -74,10 +75,12 @@ class QueueMap { }) } removeExact (queueKey: string, value: T) { - const i = this.queues[queueKey].findIndex((v) => v === value) + const i = this.queues[queueKey]?.findIndex((v) => v === value) - this.queues[queueKey].splice(i, 1) - if (this.queues[queueKey].length === 0) delete this.queues[queueKey] + if (i > -1) { + this.queues[queueKey].splice(i, 1) + if (this.queues[queueKey].length === 0) delete this.queues[queueKey] + } } forEach (fn: (value: T) => void) { @@ -210,7 +213,7 @@ export class PreRequests { }) } - removePending (requestId: string) { + removePendingPreRequest (requestId: string) { this.pendingPreRequests.removeMatching(({ browserPreRequest }) => { return (browserPreRequest.requestId.includes('-retry-') && !browserPreRequest.requestId.startsWith(`${requestId}-`)) || (!browserPreRequest.requestId.includes('-retry-') && browserPreRequest.requestId !== requestId) }) @@ -267,6 +270,7 @@ export class PreRequests { } const pendingRequest: PendingRequest = { + key, ctxDebug, callback, proxyRequestReceivedTimestamp: performance.now() + performance.timeOrigin, @@ -283,6 +287,8 @@ export class PreRequests { } this.pendingRequests.push(key, pendingRequest) + + return pendingRequest } setProtocolManager (protocolManager: ProtocolManagerShape) { @@ -293,6 +299,12 @@ export class PreRequests { this.requestTimeout = requestTimeout } + removePendingRequest (pendingRequest: PendingRequest) { + this.pendingRequests.removeExact(pendingRequest.key, pendingRequest) + clearTimeout(pendingRequest.timeout) + delete pendingRequest.callback + } + reset () { this.pendingPreRequests = new QueueMap() diff --git a/packages/proxy/test/unit/http/util/prerequests.spec.ts b/packages/proxy/test/unit/http/util/prerequests.spec.ts index f607a823ee0b..391a73ef957d 100644 --- a/packages/proxy/test/unit/http/util/prerequests.spec.ts +++ b/packages/proxy/test/unit/http/util/prerequests.spec.ts @@ -207,7 +207,7 @@ describe('http/util/prerequests', () => { expectPendingCounts(0, 3) - preRequests.removePending('1235') + preRequests.removePendingPreRequest('1235') expectPendingCounts(0, 2) }) @@ -222,7 +222,7 @@ describe('http/util/prerequests', () => { expectPendingCounts(0, 6) - preRequests.removePending('1235') + preRequests.removePendingPreRequest('1235') expectPendingCounts(0, 2) }) @@ -236,6 +236,27 @@ describe('http/util/prerequests', () => { expect(cbServiceWorker).to.be.calledWith() }) + it('removes a pending request', () => { + const cb = sinon.stub() + + const firstPreRequest = preRequests.get({ proxiedUrl: 'foo', method: 'GET', headers: {} } as CypressIncomingRequest, () => {}, cb) + const secondPreRequest = preRequests.get({ proxiedUrl: 'foo', method: 'GET', headers: {} } as CypressIncomingRequest, () => {}, cb) + + expectPendingCounts(2, 0) + + preRequests.removePendingRequest(firstPreRequest!) + + expectPendingCounts(1, 0) + + preRequests.removePendingRequest(firstPreRequest!) + + expectPendingCounts(1, 0) + + preRequests.removePendingRequest(secondPreRequest!) + + expectPendingCounts(0, 0) + }) + it('resets the queues', () => { let callbackCalled = false diff --git a/packages/server/test/integration/http_requests_spec.js b/packages/server/test/integration/http_requests_spec.js index 921ed2097edc..5eed3127763b 100644 --- a/packages/server/test/integration/http_requests_spec.js +++ b/packages/server/test/integration/http_requests_spec.js @@ -94,7 +94,7 @@ describe('Routes', () => { Fixtures.scaffold() - this.setup = async (initialUrl, obj = {}, spec) => { + this.setup = async (initialUrl, obj = {}, spec, shouldCorrelatePreRequests = false) => { if (_.isObject(initialUrl)) { obj = initialUrl initialUrl = null @@ -170,6 +170,7 @@ describe('Routes', () => { createRoutes, testingType: 'e2e', exit: false, + shouldCorrelatePreRequests: () => shouldCorrelatePreRequests, }) .spread(async (port) => { const automationStub = { @@ -187,6 +188,8 @@ describe('Routes', () => { this.session = session(this.srv) this.proxy = `http://localhost:${port}` + + this.networkProxy = this.server._networkProxy }), ]) }) @@ -983,6 +986,108 @@ describe('Routes', () => { }) }) + context('basic request with correlation', () => { + beforeEach(function () { + return this.setup('http://www.github.com', undefined, undefined, true) + }) + + it('properly correlates when CDP failures come first', function () { + this.timeout(1500) + + nock(this.server.remoteStates.current().origin) + .get('/') + .reply(200, 'hello from bar!', { + 'Content-Type': 'text/html', + }) + + this.networkProxy.addPendingBrowserPreRequest({ + requestId: '1', + method: 'GET', + url: 'http://www.github.com/', + }) + + this.networkProxy.removePendingBrowserPreRequest({ + requestId: '1', + }) + + const requestPromise = this.rp({ + url: 'http://www.github.com/', + headers: { + 'Cookie': '__cypress.initial=true', + 'Accept-Encoding': 'identity', + }, + }) + + this.networkProxy.addPendingBrowserPreRequest({ + requestId: '1', + method: 'GET', + url: 'http://www.github.com/', + }) + + return requestPromise.then((res) => { + expect(res.statusCode).to.eq(200) + + expect(res.body).to.include('hello from bar!') + }) + }) + + it('properly correlates when proxy failure come first', function () { + this.networkProxy.setPreRequestTimeout(50) + // If this takes longer than the Promise.delay and the prerequest timeout then the second + // call has hit the prerequest timeout which is a problem + this.timeout(150) + + nock(this.server.remoteStates.current().origin) + .get('/') + .delay(50) + .once() + .reply(200, 'hello from bar!', { + 'Content-Type': 'text/html', + }) + + this.rp({ + url: 'http://www.github.com/', + headers: { + 'Cookie': '__cypress.initial=true', + 'Accept-Encoding': 'identity', + }, + // Timeout needs to be less than the prerequest timeout + the nock delay + timeout: 75, + }).catch(() => {}) + + // Wait 100 ms to make sure the request times out + return Promise.delay(100).then(() => { + nock(this.server.remoteStates.current().origin) + .get('/') + .once() + .reply(200, 'hello from baz!', { + 'Content-Type': 'text/html', + }) + + // This should not immediately correlate. If it does, then the next request will timeout + this.networkProxy.addPendingBrowserPreRequest({ + requestId: '1', + method: 'GET', + url: 'http://www.github.com/', + }) + + const followupRequestPromise = this.rp({ + url: 'http://www.github.com/', + headers: { + 'Cookie': '__cypress.initial=true', + 'Accept-Encoding': 'identity', + }, + }) + + return followupRequestPromise.then((res) => { + expect(res.statusCode).to.eq(200) + + expect(res.body).to.include('hello from baz!') + }) + }) + }) + }) + context('gzip', () => { beforeEach(function () { return this.setup('http://www.github.com') From dbd213926c0a6d116d7a392fe679d30519dd9f17 Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Mon, 23 Oct 2023 22:26:21 -0500 Subject: [PATCH 3/5] fix: issue with proxy correlations and web/shared workers (#28105) --- cli/CHANGELOG.md | 1 + .../server/lib/browsers/browser-cri-client.ts | 65 ++++++++++++- .../server/lib/browsers/cdp_automation.ts | 16 +--- packages/server/lib/browsers/chrome.ts | 10 +- packages/server/lib/browsers/cri-client.ts | 96 ++++++++++++------- packages/server/lib/browsers/electron.ts | 2 +- packages/server/lib/browsers/firefox-util.ts | 2 +- packages/server/lib/cloud/protocol.ts | 8 ++ .../unit/browsers/browser-cri-client_spec.ts | 41 +++++--- .../test/unit/browsers/cdp_automation_spec.ts | 4 +- .../test/unit/browsers/cri-client_spec.ts | 10 +- .../server/test/unit/browsers/firefox_spec.ts | 10 +- packages/types/src/protocol.ts | 1 + .../projects/e2e/cypress/e2e/web_worker.cy.js | 34 +++++++ system-tests/projects/e2e/shared-worker.js | 14 +++ system-tests/projects/e2e/web-worker.js | 10 ++ system-tests/projects/e2e/web_worker.html | 13 +++ system-tests/test/service_worker_spec.js | 2 +- system-tests/test/web_worker_spec.js | 53 ++++++++++ 19 files changed, 313 insertions(+), 79 deletions(-) create mode 100644 system-tests/projects/e2e/cypress/e2e/web_worker.cy.js create mode 100644 system-tests/projects/e2e/shared-worker.js create mode 100644 system-tests/projects/e2e/web-worker.js create mode 100644 system-tests/projects/e2e/web_worker.html create mode 100644 system-tests/test/web_worker_spec.js diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 0b7dfbe0a425..43880abc0b8d 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -5,6 +5,7 @@ _Released 10/24/2023 (PENDING)_ **Bugfixes:** +- Fixed a performance regression in `13.3.1` with proxy correlation timeouts and requests issued from web and shared workers. Fixes [#28104](https://github.com/cypress-io/cypress/issues/28104). - Fixed a performance problem with proxy correlation when requests get aborted and then get miscorrelated with follow up requests. Fixed in [#28094](https://github.com/cypress-io/cypress/pull/28094). - Fixed a regression in [10.0.0](#10.0.0), where search would not find a spec if the file name contains "-" or "\_", but search prompt contains " " instead (e.g. search file "spec-file.cy.ts" with prompt "spec file"). Fixes [#25303](https://github.com/cypress-io/cypress/issues/25303). diff --git a/packages/server/lib/browsers/browser-cri-client.ts b/packages/server/lib/browsers/browser-cri-client.ts index a084f2bd0cab..09a61720ffac 100644 --- a/packages/server/lib/browsers/browser-cri-client.ts +++ b/packages/server/lib/browsers/browser-cri-client.ts @@ -3,7 +3,7 @@ import CRI from 'chrome-remote-interface' import Debug from 'debug' import { _connectAsync, _getDelayMsForRetry } from './protocol' import * as errors from '../errors' -import { create, CriClient } from './cri-client' +import { create, CriClient, DEFAULT_NETWORK_ENABLE_OPTIONS } from './cri-client' import type { ProtocolManagerShape } from '@packages/types' const debug = Debug('cypress:server:browsers:browser-cri-client') @@ -13,6 +13,27 @@ interface Version { minor: number } +type BrowserCriClientOptions = { + browserClient: CriClient + versionInfo: CRI.VersionResult + host: string + port: number + browserName: string + onAsynchronousError: Function + protocolManager?: ProtocolManagerShape + fullyManageTabs?: boolean +} + +type BrowserCriClientCreateOptions = { + hosts: string[] + port: number + browserName: string + onAsynchronousError: Function + onReconnect?: (client: CriClient) => void + protocolManager?: ProtocolManagerShape + fullyManageTabs?: boolean +} + const isVersionGte = (a: Version, b: Version) => { return a.major > b.major || (a.major === b.major && a.minor >= b.minor) } @@ -114,6 +135,14 @@ const retryWithIncreasingDelay = async (retryable: () => Promise, browserN } export class BrowserCriClient { + private browserClient: CriClient + private versionInfo: CRI.VersionResult + private host: string + private port: number + private browserName: string + private onAsynchronousError: Function + private protocolManager?: ProtocolManagerShape + private fullyManageTabs?: boolean currentlyAttachedTarget: CriClient | undefined // whenever we instantiate the instance we're already connected bc // we receive an underlying CRI connection @@ -125,7 +154,16 @@ export class BrowserCriClient { gracefulShutdown?: Boolean onClose: Function | null = null - private constructor (private browserClient: CriClient, private versionInfo, public host: string, public port: number, private browserName: string, private onAsynchronousError: Function, private protocolManager?: ProtocolManagerShape) { } + private constructor ({ browserClient, versionInfo, host, port, browserName, onAsynchronousError, protocolManager, fullyManageTabs }: BrowserCriClientOptions) { + this.browserClient = browserClient + this.versionInfo = versionInfo + this.host = host + this.port = port + this.browserName = browserName + this.onAsynchronousError = onAsynchronousError + this.protocolManager = protocolManager + this.fullyManageTabs = fullyManageTabs + } /** * Factory method for the browser cri client. Connects to the browser and then returns a chrome remote interface wrapper around the @@ -140,7 +178,7 @@ export class BrowserCriClient { * @param fullyManageTabs whether or not to fully manage tabs. This is useful for firefox where some work is done with marionette and some with CDP. We don't want to handle disconnections in this class in those scenarios * @returns a wrapper around the chrome remote interface that is connected to the browser target */ - static async create (hosts: string[], port: number, browserName: string, onAsynchronousError: Function, onReconnect?: (client: CriClient) => void, protocolManager?: ProtocolManagerShape, { fullyManageTabs }: { fullyManageTabs?: boolean } = {}): Promise { + static async create ({ hosts, port, browserName, onAsynchronousError, onReconnect, protocolManager, fullyManageTabs }: BrowserCriClientCreateOptions): Promise { const host = await ensureLiveBrowser(hosts, port, browserName) return retryWithIncreasingDelay(async () => { @@ -151,11 +189,26 @@ export class BrowserCriClient { onAsynchronousError, onReconnect, protocolManager, + fullyManageTabs, }) - const browserCriClient = new BrowserCriClient(browserClient, versionInfo, host!, port, browserName, onAsynchronousError, protocolManager) + const browserCriClient = new BrowserCriClient({ browserClient, versionInfo, host, port, browserName, onAsynchronousError, protocolManager, fullyManageTabs }) if (fullyManageTabs) { + // The basic approach here is we attach to targets and enable network traffic + // We must attach in a paused state so that we can enable network traffic before the target starts running. + browserClient.on('Target.attachedToTarget', async (event) => { + if (event.targetInfo.type !== 'page') { + await browserClient.send('Network.enable', protocolManager?.networkEnableOptions ?? DEFAULT_NETWORK_ENABLE_OPTIONS, event.sessionId) + } + + if (event.waitingForDebugger) { + await browserClient.send('Runtime.runIfWaitingForDebugger', undefined, event.sessionId) + } + }) + + // Ideally we could use filter rather than checking the type above, but that was added relatively recently + await browserClient.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }) await browserClient.send('Target.setDiscoverTargets', { discover: true }) browserClient.on('Target.targetDestroyed', (event) => { debug('Target.targetDestroyed %o', { @@ -270,7 +323,8 @@ export class BrowserCriClient { throw new Error(`Could not find url target in browser ${url}. Targets were ${JSON.stringify(targets)}`) } - this.currentlyAttachedTarget = await create({ target: target.targetId, onAsynchronousError: this.onAsynchronousError, host: this.host, port: this.port, protocolManager: this.protocolManager }) + this.currentlyAttachedTarget = await create({ target: target.targetId, onAsynchronousError: this.onAsynchronousError, host: this.host, port: this.port, protocolManager: this.protocolManager, fullyManageTabs: this.fullyManageTabs, browserClient: this.browserClient }) + await this.protocolManager?.connectToBrowser(this.currentlyAttachedTarget) return this.currentlyAttachedTarget @@ -323,6 +377,7 @@ export class BrowserCriClient { host: this.host, port: this.port, protocolManager: this.protocolManager, + fullyManageTabs: this.fullyManageTabs, }) } else { this.currentlyAttachedTarget = undefined diff --git a/packages/server/lib/browsers/cdp_automation.ts b/packages/server/lib/browsers/cdp_automation.ts index 6255de775972..df65885c0a2d 100644 --- a/packages/server/lib/browsers/cdp_automation.ts +++ b/packages/server/lib/browsers/cdp_automation.ts @@ -13,7 +13,7 @@ import type { ResourceType, BrowserPreRequest, BrowserResponseReceived } from '@ import type { CDPClient, ProtocolManagerShape, WriteVideoFrame } from '@packages/types' import type { Automation } from '../automation' import { cookieMatches, CyCookie, CyCookieFilter } from '../automation/util' -import type { CriClient } from './cri-client' +import { DEFAULT_NETWORK_ENABLE_OPTIONS, CriClient } from './cri-client' export type CdpCommand = keyof ProtocolMapping.Commands @@ -140,7 +140,7 @@ export const normalizeResourceType = (resourceType: string | undefined): Resourc return ffToStandardResourceTypeMap[resourceType] || 'other' } -export type SendDebuggerCommand = (message: T, data?: ProtocolMapping.Commands[T]['paramsType'][0]) => Promise +export type SendDebuggerCommand = (message: T, data?: ProtocolMapping.Commands[T]['paramsType'][0], sessionId?: string) => Promise export type OnFn = (eventName: T, cb: (data: ProtocolMapping.Events[T][0]) => void) => void @@ -198,17 +198,7 @@ export class CdpAutomation implements CDPClient { static async create (sendDebuggerCommandFn: SendDebuggerCommand, onFn: OnFn, offFn: OffFn, sendCloseCommandFn: SendCloseCommand, automation: Automation, protocolManager?: ProtocolManagerShape): Promise { const cdpAutomation = new CdpAutomation(sendDebuggerCommandFn, onFn, offFn, sendCloseCommandFn, automation) - const networkEnabledOptions = protocolManager?.protocolEnabled ? { - maxTotalBufferSize: 0, - maxResourceBufferSize: 0, - maxPostDataSize: 64 * 1024, - } : { - maxTotalBufferSize: 0, - maxResourceBufferSize: 0, - maxPostDataSize: 0, - } - - await sendDebuggerCommandFn('Network.enable', networkEnabledOptions) + await sendDebuggerCommandFn('Network.enable', protocolManager?.networkEnableOptions ?? DEFAULT_NETWORK_ENABLE_OPTIONS) return cdpAutomation } diff --git a/packages/server/lib/browsers/chrome.ts b/packages/server/lib/browsers/chrome.ts index 4f09b9f7fc82..ea6b1e20d705 100644 --- a/packages/server/lib/browsers/chrome.ts +++ b/packages/server/lib/browsers/chrome.ts @@ -473,7 +473,7 @@ export = { debug('connecting to existing chrome instance with url and debugging port', { url: options.url, port }) if (!options.onError) throw new Error('Missing onError in connectToExisting') - const browserCriClient = await BrowserCriClient.create(['127.0.0.1'], port, browser.displayName, options.onError, onReconnect, undefined, { fullyManageTabs: false }) + const browserCriClient = await BrowserCriClient.create({ hosts: ['127.0.0.1'], port, browserName: browser.displayName, onAsynchronousError: options.onError, onReconnect, fullyManageTabs: false }) if (!options.url) throw new Error('Missing url in connectToExisting') @@ -488,7 +488,11 @@ export = { const browserCriClient = this._getBrowserCriClient() // Handle chrome tab crashes. - pageCriClient.on('Inspector.targetCrashed', async () => { + pageCriClient.on('Target.targetCrashed', async (event) => { + if (event.targetId !== browserCriClient?.currentlyAttachedTarget?.targetId) { + return + } + const err = errors.get('RENDERER_CRASHED', browser.displayName) await memory.endProfiling() @@ -597,7 +601,7 @@ export = { // navigate to the actual url if (!options.onError) throw new Error('Missing onError in chrome#open') - browserCriClient = await BrowserCriClient.create(['127.0.0.1'], port, browser.displayName, options.onError, onReconnect, options.protocolManager, { fullyManageTabs: true }) + browserCriClient = await BrowserCriClient.create({ hosts: ['127.0.0.1'], port, browserName: browser.displayName, onAsynchronousError: options.onError, onReconnect, protocolManager: options.protocolManager, fullyManageTabs: true }) la(browserCriClient, 'expected Chrome remote interface reference', browserCriClient) diff --git a/packages/server/lib/browsers/cri-client.ts b/packages/server/lib/browsers/cri-client.ts index 2b983b3b05fd..2db9a0ce6996 100644 --- a/packages/server/lib/browsers/cri-client.ts +++ b/packages/server/lib/browsers/cri-client.ts @@ -28,11 +28,13 @@ type EnqueuedCommand = { command: CdpCommand params?: object p: DeferredPromise + sessionId?: string } type EnableCommand = { command: CdpCommand params?: object + sessionId?: string } type Subscription = { @@ -45,6 +47,12 @@ interface CDPClient extends CDP.Client { _ws: WebSocket } +export const DEFAULT_NETWORK_ENABLE_OPTIONS = { + maxTotalBufferSize: 0, + maxResourceBufferSize: 0, + maxPostDataSize: 0, +} + export interface CriClient { /** * The target id attached to by this client @@ -138,6 +146,8 @@ type CreateParams = { port?: number onReconnect?: (client: CriClient) => void protocolManager?: ProtocolManagerShape + fullyManageTabs?: boolean + browserClient?: CriClient } export const create = async ({ @@ -147,6 +157,8 @@ export const create = async ({ port, onReconnect, protocolManager, + fullyManageTabs, + browserClient, }: CreateParams): Promise => { const subscriptions: Subscription[] = [] const enableCommands: EnableCommand[] = [] @@ -183,12 +195,12 @@ export const create = async ({ // '*.enable' commands need to be resent on reconnect or any events in // that namespace will no longer be received - await Promise.all(enableCommands.map(({ command, params }) => { - return cri.send(command, params) + await Promise.all(enableCommands.map(({ command, params, sessionId }) => { + return cri.send(command, params, sessionId) })) enqueuedCommands.forEach((cmd) => { - cri.send(cmd.command, cmd.params).then(cmd.p.resolve as any, cmd.p.reject as any) + cri.send(cmd.command, cmd.params, cmd.sessionId).then(cmd.p.resolve as any, cmd.p.reject as any) }) enqueuedCommands = [] @@ -258,35 +270,35 @@ export const create = async ({ cri.on('disconnect', retryReconnect) } - cri.on('Inspector.targetCrashed', 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. + // We only want to try and add child target traffic if we have a host set. This indicates that this is the child cri client. + // Browser cri traffic is handled in browser-cri-client.ts. The basic approach here is we attach to targets and enable network traffic + // We must attach in a paused state so that we can enable network traffic before the target starts running. 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) + cri.on('Target.targetCrashed', async (event) => { + if (event.targetId !== target) { + return } + + debug('crash detected') + crashed = true }) - await cri.send('Target.setDiscoverTargets', { discover: true }) + if (fullyManageTabs) { + cri.on('Target.attachedToTarget', async (event) => { + // Service workers get attached at the page and browser level. We only want to handle them at the browser level + if (event.targetInfo.type !== 'service_worker' && event.targetInfo.type !== 'page') { + await cri.send('Network.enable', protocolManager?.networkEnableOptions ?? DEFAULT_NETWORK_ENABLE_OPTIONS, event.sessionId) + } + + if (event.waitingForDebugger) { + await cri.send('Runtime.runIfWaitingForDebugger', undefined, event.sessionId) + } + }) + + // Ideally we could use filter rather than checking the type above, but that was added relatively recently + await cri.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }) + await cri.send('Target.setDiscoverTargets', { discover: true }) + } } } @@ -295,7 +307,7 @@ export const create = async ({ client = { targetId: target, - async send (command: CdpCommand, params?: object) { + async send (command: CdpCommand, params?: object, sessionId?: string) { if (crashed) { return Promise.reject(new Error(`${command} will not run as the target browser or tab CRI connection has crashed`)) } @@ -313,6 +325,10 @@ export const create = async ({ obj.params = params } + if (sessionId) { + obj.sessionId = sessionId + } + enqueuedCommands.push(obj) }) } @@ -328,12 +344,16 @@ export const create = async ({ obj.params = params } + if (sessionId) { + obj.sessionId = sessionId + } + enableCommands.push(obj) } if (connected) { try { - return await cri.send(command, params) + return await cri.send(command, params, sessionId) } catch (err) { // This error occurs when the browser has been left open for a long // time and/or the user's computer has been put to sleep. The @@ -343,7 +363,7 @@ export const create = async ({ throw err } - debug('encountered closed websocket on send %o', { command, params, err }) + debug('encountered closed websocket on send %o', { command, params, sessionId, err }) const p = enqueue() as Promise @@ -367,7 +387,12 @@ export const create = async ({ subscriptions.push({ eventName, cb }) debug('registering CDP on event %o', { eventName }) - return cri.on(eventName, cb) + cri.on(eventName, cb) + // This ensures that we are notified about the browser's network events that have been registered (e.g. service workers) + // Long term we should use flat mode entirely across all of chrome remote interface + if (eventName.startsWith('Network.')) { + browserClient?.on(eventName, cb) + } }, off (eventName, cb) { @@ -375,7 +400,12 @@ export const create = async ({ return sub.eventName === eventName && sub.cb === cb }), 1) - return cri.off(eventName, cb) + cri.off(eventName, cb) + // This ensures that we are notified about the browser's network events that have been registered (e.g. service workers) + // Long term we should use flat mode entirely across all of chrome remote interface + if (eventName.startsWith('Network.')) { + browserClient?.off(eventName, cb) + } }, get ws () { diff --git a/packages/server/lib/browsers/electron.ts b/packages/server/lib/browsers/electron.ts index 31583a06735b..52a7b1e33c73 100644 --- a/packages/server/lib/browsers/electron.ts +++ b/packages/server/lib/browsers/electron.ts @@ -54,7 +54,7 @@ const _getAutomation = async function (win, options: BrowserLaunchOpts, parent) const port = getRemoteDebuggingPort() if (!browserCriClient) { - browserCriClient = await BrowserCriClient.create(['127.0.0.1'], port, 'electron', options.onError, () => {}, undefined, { fullyManageTabs: true }) + browserCriClient = await BrowserCriClient.create({ hosts: ['127.0.0.1'], port, browserName: 'electron', onAsynchronousError: options.onError, onReconnect: () => {}, fullyManageTabs: true }) } const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank') diff --git a/packages/server/lib/browsers/firefox-util.ts b/packages/server/lib/browsers/firefox-util.ts index 34c6b5d640db..72b9c1e545b3 100644 --- a/packages/server/lib/browsers/firefox-util.ts +++ b/packages/server/lib/browsers/firefox-util.ts @@ -138,7 +138,7 @@ async function connectToNewSpec (options, automation: Automation, browserCriClie } async function setupRemote (remotePort, automation, onError): Promise { - const browserCriClient = await BrowserCriClient.create(['127.0.0.1', '::1'], remotePort, 'Firefox', onError) + const browserCriClient = await BrowserCriClient.create({ hosts: ['127.0.0.1', '::1'], port: remotePort, browserName: 'Firefox', onAsynchronousError: onError }) const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank') await CdpAutomation.create(pageCriClient.send, pageCriClient.on, pageCriClient.off, browserCriClient.resetBrowserTargets, automation) diff --git a/packages/server/lib/cloud/protocol.ts b/packages/server/lib/cloud/protocol.ts index 478a5d887207..79acbbedee92 100644 --- a/packages/server/lib/cloud/protocol.ts +++ b/packages/server/lib/cloud/protocol.ts @@ -72,6 +72,14 @@ export class ProtocolManager implements ProtocolManagerShape { return !!this._protocol } + get networkEnableOptions () { + return this.protocolEnabled ? { + maxTotalBufferSize: 0, + maxResourceBufferSize: 0, + maxPostDataSize: 64 * 1024, + } : undefined + } + async setupProtocol (script: string, options: ProtocolManagerOptions) { this._captureHash = base64url.fromBase64(crypto.createHash('SHA256').update(script).digest('base64')) 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 266b3dce36ed..fc4656f6f8bd 100644 --- a/packages/server/test/unit/browsers/browser-cri-client_spec.ts +++ b/packages/server/test/unit/browsers/browser-cri-client_spec.ts @@ -10,6 +10,11 @@ const HOST = '127.0.0.1' const PORT = 50505 const THROWS_PORT = 65535 +type GetClientParams = { + protocolManager?: ProtocolManagerShape + fullyManageTabs?: boolean +} + describe('lib/browsers/cri-client', function () { let browserCriClient: { BrowserCriClient: { @@ -17,13 +22,14 @@ describe('lib/browsers/cri-client', function () { } } let send: sinon.SinonStub + let on: sinon.SinonStub let close: sinon.SinonStub let criClientCreateStub: sinon.SinonStub let criImport: sinon.SinonStub & { Version: sinon.SinonStub } let onError: sinon.SinonStub - let getClient: (protocolManager?: ProtocolManagerShape) => ReturnType + let getClient: (options?: GetClientParams) => ReturnType beforeEach(function () { sinon.stub(protocol, '_connectAsync') @@ -37,10 +43,12 @@ describe('lib/browsers/cri-client', function () { .onSecondCall().throws() .onThirdCall().resolves({ webSocketDebuggerUrl: 'http://web/socket/url' }) + on = sinon.stub() send = sinon.stub() close = sinon.stub() - criClientCreateStub = sinon.stub(CriClient, 'create').withArgs({ target: 'http://web/socket/url', onAsynchronousError: onError, onReconnect: undefined, protocolManager: undefined }).resolves({ + criClientCreateStub = sinon.stub(CriClient, 'create').withArgs({ target: 'http://web/socket/url', onAsynchronousError: onError, onReconnect: undefined, protocolManager: undefined, fullyManageTabs: undefined }).resolves({ send, + on, close, }) @@ -48,13 +56,14 @@ describe('lib/browsers/cri-client', function () { 'chrome-remote-interface': criImport, }) - getClient = (protocolManager) => { - criClientCreateStub = criClientCreateStub.withArgs({ target: 'http://web/socket/url', onAsynchronousError: onError, onReconnect: undefined, protocolManager }).resolves({ + getClient = ({ protocolManager, fullyManageTabs } = {}) => { + criClientCreateStub = criClientCreateStub.withArgs({ target: 'http://web/socket/url', onAsynchronousError: onError, onReconnect: undefined, protocolManager, fullyManageTabs }).resolves({ send, + on, close, }) - return browserCriClient.BrowserCriClient.create(['127.0.0.1'], PORT, 'Chrome', onError, undefined, protocolManager) + return browserCriClient.BrowserCriClient.create({ hosts: ['127.0.0.1'], port: PORT, browserName: 'Chrome', onAsynchronousError: onError, protocolManager, fullyManageTabs }) } }) @@ -91,7 +100,7 @@ describe('lib/browsers/cri-client', function () { criImport.Version.withArgs({ host: '::1', port: THROWS_PORT, useHostName: true }).resolves({ webSocketDebuggerUrl: 'http://web/socket/url' }) - await browserCriClient.BrowserCriClient.create(['127.0.0.1', '::1'], THROWS_PORT, 'Chrome', onError) + await browserCriClient.BrowserCriClient.create({ hosts: ['127.0.0.1', '::1'], port: THROWS_PORT, browserName: 'Chrome', onAsynchronousError: onError }) expect(criImport.Version).to.be.calledOnce }) @@ -102,7 +111,7 @@ describe('lib/browsers/cri-client', function () { .onSecondCall().returns(100) .onThirdCall().returns(100) - const client = await browserCriClient.BrowserCriClient.create(['127.0.0.1'], THROWS_PORT, 'Chrome', onError) + const client = await browserCriClient.BrowserCriClient.create({ hosts: ['127.0.0.1'], port: THROWS_PORT, browserName: 'Chrome', onAsynchronousError: onError }) expect(client.attachToTargetUrl).to.be.instanceOf(Function) @@ -114,7 +123,7 @@ describe('lib/browsers/cri-client', function () { .onFirstCall().returns(100) .onSecondCall().returns(undefined) - await expect(browserCriClient.BrowserCriClient.create(['127.0.0.1'], THROWS_PORT, 'Chrome', onError)).to.be.rejected + await expect(browserCriClient.BrowserCriClient.create({ hosts: ['127.0.0.1'], port: THROWS_PORT, browserName: 'Chrome', onAsynchronousError: onError })).to.be.rejected expect(criImport.Version).to.be.calledTwice }) @@ -150,7 +159,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({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined }).resolves(mockPageClient) + criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined, fullyManageTabs: undefined, browserClient: { on, send, close } }).resolves(mockPageClient) const browserClient = await getClient() @@ -159,16 +168,18 @@ describe('lib/browsers/cri-client', function () { expect(client).to.be.equal(mockPageClient) }) - it('creates a page client when the passed in url is found and notifies the protocol manager', async function () { + it('creates a page client when the passed in url is found and notifies the protocol manager and fully managed tabs', async function () { const mockPageClient = {} const protocolManager: any = { connectToBrowser: sinon.stub().resolves(), } send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) - criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager }).resolves(mockPageClient) + send.withArgs('Target.setDiscoverTargets', { discover: true }) + on.withArgs('Target.targetDestroyed', sinon.match.func) + criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager, fullyManageTabs: true, browserClient: { on, send, close } }).resolves(mockPageClient) - const browserClient = await getClient(protocolManager) + const browserClient = await getClient({ protocolManager, fullyManageTabs: true }) const client = await browserClient.attachToTargetUrl('http://foo.com') @@ -187,7 +198,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({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined }).resolves(mockPageClient) + criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined, fullyManageTabs: undefined, browserClient: { on, send, close } }).resolves(mockPageClient) const browserClient = await getClient() @@ -205,7 +216,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({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined }).resolves(mockPageClient) + criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined, fullyManageTabs: undefined, browserClient: { on, send, close } }).resolves(mockPageClient) const browserClient = await getClient() @@ -226,7 +237,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({ target: '101', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined }).resolves(mockUpdatedCurrentlyAttachedTarget) + criClientCreateStub.withArgs({ target: '101', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined, fullyManageTabs: undefined }).resolves(mockUpdatedCurrentlyAttachedTarget) const browserClient = await getClient() as any diff --git a/packages/server/test/unit/browsers/cdp_automation_spec.ts b/packages/server/test/unit/browsers/cdp_automation_spec.ts index d645c489a96d..b02d6449ec6f 100644 --- a/packages/server/test/unit/browsers/cdp_automation_spec.ts +++ b/packages/server/test/unit/browsers/cdp_automation_spec.ts @@ -24,6 +24,7 @@ context('lib/browsers/cdp_automation', () => { } const localManager = { protocolEnabled: true, + networkEnableOptions: enabledObject, } as ProtocolManagerShape const localCommandStub = localCommand.withArgs('Network.enable', enabledObject).resolves() @@ -49,6 +50,7 @@ context('lib/browsers/cdp_automation', () => { } const localManager = { protocolEnabled: false, + networkEnableOptions: disabledObject, } as ProtocolManagerShape const localCommandStub = localCommand.withArgs('Network.enable', disabledObject).resolves() @@ -91,7 +93,7 @@ context('lib/browsers/cdp_automation', () => { const startScreencast = this.sendDebuggerCommand.withArgs('Page.startScreencast').resolves() const screencastFrameAck = this.sendDebuggerCommand.withArgs('Page.screencastFrameAck').resolves() - await cdpAutomation.startVideoRecording(writeVideoFrame) + await cdpAutomation.startVideoRecording(writeVideoFrame, {}) expect(startScreencast).to.have.been.calledWith('Page.startScreencast') expect(writeVideoFrame).to.have.been.calledWithMatch((arg) => Buffer.isBuffer(arg) && arg.length > 0) diff --git a/packages/server/test/unit/browsers/cri-client_spec.ts b/packages/server/test/unit/browsers/cri-client_spec.ts index 7b5d90dea71d..61391e890f71 100644 --- a/packages/server/test/unit/browsers/cri-client_spec.ts +++ b/packages/server/test/unit/browsers/cri-client_spec.ts @@ -23,7 +23,7 @@ describe('lib/browsers/cri-client', function () { _notifier: EventEmitter } let onError: sinon.SinonStub - let getClient: () => ReturnType + let getClient: (options?: { host?: string, fullyManageTabs?: boolean }) => ReturnType beforeEach(function () { send = sinon.stub() @@ -49,8 +49,8 @@ describe('lib/browsers/cri-client', function () { 'chrome-remote-interface': criImport, }) - getClient = () => { - return criClient.create({ target: DEBUGGER_URL, onAsynchronousError: onError }) + getClient = ({ host, fullyManageTabs } = {}) => { + return criClient.create({ target: DEBUGGER_URL, host, onAsynchronousError: onError, fullyManageTabs }) } }) @@ -82,9 +82,9 @@ describe('lib/browsers/cri-client', function () { it('rejects if target has crashed', async function () { const command = 'DOM.getDocument' - const client = await getClient() + const client = await getClient({ host: '127.0.0.1', fullyManageTabs: true }) - await criStub.on.withArgs('Inspector.targetCrashed').args[0][1]() + await criStub.on.withArgs('Target.targetCrashed').args[0][1]({ targetId: DEBUGGER_URL }) await expect(client.send(command, { depth: -1 })).to.be.rejectedWith(`${command} will not run as the target browser or tab CRI connection has crashed`) }) diff --git a/packages/server/test/unit/browsers/firefox_spec.ts b/packages/server/test/unit/browsers/firefox_spec.ts index 069573ea399a..b0ab02ffacd3 100644 --- a/packages/server/test/unit/browsers/firefox_spec.ts +++ b/packages/server/test/unit/browsers/firefox_spec.ts @@ -532,6 +532,14 @@ describe('lib/browsers/firefox', () => { on: sinon.stub(), off: sinon.stub(), close: sinon.stub(), + ws: sinon.stub() as any, + queue: { + enableCommands: [], + enqueuedCommands: [], + subscriptions: [], + }, + closed: false, + connected: false, } const browserCriClient: BrowserCriClient = sinon.createStubInstance(BrowserCriClient) @@ -545,7 +553,7 @@ describe('lib/browsers/firefox', () => { expect(actual).to.equal(browserCriClient) expect(browserCriClient.attachToTargetUrl).to.be.calledWith('about:blank') - expect(BrowserCriClient.create).to.be.calledWith(['127.0.0.1', '::1'], port, 'Firefox', null) + expect(BrowserCriClient.create).to.be.calledWith({ hosts: ['127.0.0.1', '::1'], port, browserName: 'Firefox', onAsynchronousError: null }) expect(CdpAutomation.create).to.be.calledWith( criClientStub.send, criClientStub.on, diff --git a/packages/types/src/protocol.ts b/packages/types/src/protocol.ts index e1c8e425201c..e6402ab51f28 100644 --- a/packages/types/src/protocol.ts +++ b/packages/types/src/protocol.ts @@ -85,6 +85,7 @@ export type ProtocolManagerOptions = { export interface ProtocolManagerShape extends AppCaptureProtocolCommon { protocolEnabled: boolean + networkEnableOptions?: { maxTotalBufferSize: number, maxResourceBufferSize: number, maxPostDataSize: number } setupProtocol(script: string, options: ProtocolManagerOptions): Promise beforeSpec (spec: { instanceId: string }): void reportNonFatalErrors (clientMetadata: any): Promise diff --git a/system-tests/projects/e2e/cypress/e2e/web_worker.cy.js b/system-tests/projects/e2e/cypress/e2e/web_worker.cy.js new file mode 100644 index 000000000000..997341952fd0 --- /dev/null +++ b/system-tests/projects/e2e/cypress/e2e/web_worker.cy.js @@ -0,0 +1,34 @@ +const webWorker = (win) => { + return new Promise((resolve) => { + win.worker.onmessage = (e) => { + if (e.data.foo === 'bar2') { + resolve(win) + } + } + + win.worker.postMessage({ + foo: 'bar', + }) + }) +} + +const sharedWorker = (win) => { + return new Promise((resolve) => { + win.sharedWorker.port.onmessage = (e) => { + if (e.data.foo === 'baz2') { + resolve(win) + } + } + + win.sharedWorker.port.postMessage({ + foo: 'baz', + }) + }) +} + +// Timeout of 1900 will ensure that the proxy correlation timeout is not hit +it('loads web workers', { defaultCommandTimeout: 1900 }, () => { + cy.visit('https://localhost:1515/web_worker.html') + .then(webWorker) + .then(sharedWorker) +}) diff --git a/system-tests/projects/e2e/shared-worker.js b/system-tests/projects/e2e/shared-worker.js new file mode 100644 index 000000000000..8db062f6ecd9 --- /dev/null +++ b/system-tests/projects/e2e/shared-worker.js @@ -0,0 +1,14 @@ +// eslint-disable-next-line no-undef +importScripts('/sw.js') + +self.addEventListener('connect', (event) => { + const port = event.ports[0] + + port.onmessage = (e) => { + if (e.data.foo === 'baz') { + port.postMessage({ + foo: 'baz2', + }) + } + } +}) diff --git a/system-tests/projects/e2e/web-worker.js b/system-tests/projects/e2e/web-worker.js new file mode 100644 index 000000000000..31781f7a4bb9 --- /dev/null +++ b/system-tests/projects/e2e/web-worker.js @@ -0,0 +1,10 @@ +// eslint-disable-next-line no-undef +importScripts('/ww.js') + +onmessage = (e) => { + if (e.data.foo === 'bar') { + postMessage({ + foo: 'bar2', + }) + } +} diff --git a/system-tests/projects/e2e/web_worker.html b/system-tests/projects/e2e/web_worker.html new file mode 100644 index 000000000000..04dd74fa682f --- /dev/null +++ b/system-tests/projects/e2e/web_worker.html @@ -0,0 +1,13 @@ + + + + + + + +

hi

+ + diff --git a/system-tests/test/service_worker_spec.js b/system-tests/test/service_worker_spec.js index 5969288d433f..5d4906926f9f 100644 --- a/system-tests/test/service_worker_spec.js +++ b/system-tests/test/service_worker_spec.js @@ -31,7 +31,7 @@ const onServer = function (app) { }) } -describe('e2e browser reset', () => { +describe('e2e service worker', () => { systemTests.setup({ servers: [{ https: true, diff --git a/system-tests/test/web_worker_spec.js b/system-tests/test/web_worker_spec.js new file mode 100644 index 000000000000..f2c519de65e5 --- /dev/null +++ b/system-tests/test/web_worker_spec.js @@ -0,0 +1,53 @@ +const express = require('express') +const Fixtures = require('../lib/fixtures') +const systemTests = require('../lib/system-tests').default + +const e2ePath = Fixtures.projectPath('e2e') + +let requestsForWebWorker = 0 +let requestsForSharedWorker = 0 + +const onServer = function (app) { + app.use(express.static(e2ePath, { + // force caching to happen + maxAge: 3600000, + })) + + app.get('/ww.js', (req, res) => { + requestsForWebWorker += 1 + + res.set('Content-Type', 'application/javascript') + res.set('Mime-Type', 'application/javascript') + + return res.send('const x = 1') + }) + + app.get('/sw.js', (req, res) => { + requestsForSharedWorker += 1 + + res.set('Content-Type', 'application/javascript') + res.set('Mime-Type', 'application/javascript') + + return res.send('const x = 1') + }) +} + +describe('e2e web worker', () => { + systemTests.setup({ + servers: [{ + https: true, + port: 1515, + onServer, + }], + }) + + systemTests.it('executes one spec with a web and shared worker', { + project: 'e2e', + spec: 'web_worker.cy.js', + onRun: async (exec, browser) => { + await exec() + expect(requestsForWebWorker).to.eq(1) + expect(requestsForSharedWorker).to.eq(1) + }, + }) +}) From 7f01e8dffcc4c4310890b374f989d155e8c35720 Mon Sep 17 00:00:00 2001 From: Matt Schile Date: Tue, 24 Oct 2023 10:32:54 -0600 Subject: [PATCH 4/5] chore: 13.3.3 release updates (#28129) --- cli/CHANGELOG.md | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 43880abc0b8d..a84836124cf5 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -1,12 +1,12 @@ ## 13.3.3 -_Released 10/24/2023 (PENDING)_ +_Released 10/24/2023_ **Bugfixes:** - Fixed a performance regression in `13.3.1` with proxy correlation timeouts and requests issued from web and shared workers. Fixes [#28104](https://github.com/cypress-io/cypress/issues/28104). -- Fixed a performance problem with proxy correlation when requests get aborted and then get miscorrelated with follow up requests. Fixed in [#28094](https://github.com/cypress-io/cypress/pull/28094). +- Fixed a performance problem with proxy correlation when requests get aborted and then get miscorrelated with follow up requests. Addressed in [#28094](https://github.com/cypress-io/cypress/pull/28094). - Fixed a regression in [10.0.0](#10.0.0), where search would not find a spec if the file name contains "-" or "\_", but search prompt contains " " instead (e.g. search file "spec-file.cy.ts" with prompt "spec file"). Fixes [#25303](https://github.com/cypress-io/cypress/issues/25303). ## 13.3.2 diff --git a/package.json b/package.json index f1177034550a..4a6e6e9b2ccd 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "cypress", - "version": "13.3.2", + "version": "13.3.3", "description": "Cypress is a next generation front end testing tool built for the modern web", "private": true, "scripts": { From 058f3a80b033f16bf57f50ee78f63d8b82e75851 Mon Sep 17 00:00:00 2001 From: Matt Schile Date: Tue, 24 Oct 2023 11:54:22 -0600 Subject: [PATCH 5/5] test: fix spec search in windows (#28130) --- packages/app/cypress/e2e/specs_list_e2e.cy.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/app/cypress/e2e/specs_list_e2e.cy.ts b/packages/app/cypress/e2e/specs_list_e2e.cy.ts index 38db09b577f0..6d05d6c3e909 100644 --- a/packages/app/cypress/e2e/specs_list_e2e.cy.ts +++ b/packages/app/cypress/e2e/specs_list_e2e.cy.ts @@ -1,5 +1,4 @@ import { getPathForPlatform } from '../../src/paths' -import path from 'path' describe('App: Spec List (E2E)', () => { const launchApp = (specFilter?: string) => { @@ -289,7 +288,7 @@ describe('App: Spec List (E2E)', () => { cy.findAllByTestId('spec-list-directory') .should('have.length', 1) - .and('contain', path.join('cypress', 'e2e', 'a-b_c')) + .and('contain', 'a-b_c') cy.findByText('No specs matched your search:').should('not.be.visible') }) @@ -299,7 +298,7 @@ describe('App: Spec List (E2E)', () => { cy.findAllByTestId('spec-list-directory') .should('have.length', 1) - .and('contain', path.join('cypress', 'e2e', 'a-b_c')) + .and('contain', 'a-b_c') cy.findByText('No specs matched your search:').should('not.be.visible') }) @@ -309,7 +308,7 @@ describe('App: Spec List (E2E)', () => { cy.findAllByTestId('spec-list-directory') .should('have.length', 1) - .and('contain', path.join('cypress', 'e2e', 'a-b_c')) + .and('contain', 'a-b_c') cy.findByText('No specs matched your search:').should('not.be.visible') })