diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index a84836124cf5..10d035262a2e 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -1,4 +1,12 @@ +## 13.3.4 + +_Released 11/7/2023 (PENDING)_ + +**Bugfixes:** + +- Fixed a regression in [`13.3.2`](https://docs.cypress.io/guides/references/changelog/13.3.2) where loading a service worker and immediately reloading the page can cause a crash. Fixes [#28141](https://github.com/cypress-io/cypress/issues/28141). + ## 13.3.3 _Released 10/24/2023_ diff --git a/packages/server/lib/browsers/browser-cri-client.ts b/packages/server/lib/browsers/browser-cri-client.ts index 09a61720ffac..b1b90363a72c 100644 --- a/packages/server/lib/browsers/browser-cri-client.ts +++ b/packages/server/lib/browsers/browser-cri-client.ts @@ -198,12 +198,17 @@ export class BrowserCriClient { // 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) - } + try { + 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) + if (event.waitingForDebugger) { + await browserClient.send('Runtime.runIfWaitingForDebugger', undefined, event.sessionId) + } + } catch (error) { + // it's possible that the target was closed before we could enable network and continue, in that case, just ignore + debug('error attaching to target browser', error) } }) diff --git a/packages/server/lib/browsers/cri-client.ts b/packages/server/lib/browsers/cri-client.ts index 2db9a0ce6996..0c61a887433d 100644 --- a/packages/server/lib/browsers/cri-client.ts +++ b/packages/server/lib/browsers/cri-client.ts @@ -285,13 +285,18 @@ export const create = async ({ 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) + try { + // 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) + } + } catch (error) { + // it's possible that the target was closed before we could enable network and continue, in that case, just ignore + debug('error attaching to target cri', error) } }) 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 fc4656f6f8bd..4beed34046f2 100644 --- a/packages/server/test/unit/browsers/browser-cri-client_spec.ts +++ b/packages/server/test/unit/browsers/browser-cri-client_spec.ts @@ -187,6 +187,31 @@ describe('lib/browsers/cri-client', function () { expect(protocolManager.connectToBrowser).to.be.calledWith(client) }) + it('creates a page client when the passed in url is found and notifies the protocol manager and fully managed tabs and attaching to target throws', 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' }] }) + send.withArgs('Target.setDiscoverTargets', { discover: true }) + on.withArgs('Target.targetDestroyed', sinon.match.func) + + send.withArgs('Network.enable').throws(new Error('ProtocolError: Inspected target navigated or closed')) + + criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager, fullyManageTabs: true, browserClient: { on, send, close } }).resolves(mockPageClient) + + const browserClient = await getClient({ protocolManager, fullyManageTabs: true }) + + const client = await browserClient.attachToTargetUrl('http://foo.com') + + expect(client).to.be.equal(mockPageClient) + expect(protocolManager.connectToBrowser).to.be.calledWith(client) + + // This would throw if the error was not caught + await on.withArgs('Target.attachedToTarget').args[0][1]({ targetInfo: { type: 'worker' } }) + }) + it('retries when the passed in url is not found', async function () { sinon.stub(protocol, '_getDelayMsForRetry') .onFirstCall().returns(100) diff --git a/packages/server/test/unit/browsers/cri-client_spec.ts b/packages/server/test/unit/browsers/cri-client_spec.ts index 61391e890f71..e76731e64b39 100644 --- a/packages/server/test/unit/browsers/cri-client_spec.ts +++ b/packages/server/test/unit/browsers/cri-client_spec.ts @@ -88,6 +88,14 @@ describe('lib/browsers/cri-client', function () { await expect(client.send(command, { depth: -1 })).to.be.rejectedWith(`${command} will not run as the target browser or tab CRI connection has crashed`) }) + it('does not reject if attachToTarget work throws', async function () { + criStub.send.withArgs('Network.enable').throws(new Error('ProtocolError: Inspected target navigated or closed')) + await getClient({ host: '127.0.0.1', fullyManageTabs: true }) + + // This would throw if the error was not caught + await criStub.on.withArgs('Target.attachedToTarget').args[0][1]({ targetInfo: { type: 'worker' } }) + }) + context('retries', () => { ([ 'WebSocket is not open',