From 6bb151b7355be7c03be72a0941149d306ce3981a Mon Sep 17 00:00:00 2001 From: Matthew Schile Date: Fri, 6 Dec 2024 13:30:43 -0700 Subject: [PATCH 1/5] add tests --- .../server/lib/browsers/browser-cri-client.ts | 5 ++--- packages/server/lib/browsers/cri-client.ts | 16 ++++++++------ .../test/unit/browsers/cri-client_spec.ts | 21 +++++++++++++++++++ 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/packages/server/lib/browsers/browser-cri-client.ts b/packages/server/lib/browsers/browser-cri-client.ts index c4f53cb8785d..9af893a31a9b 100644 --- a/packages/server/lib/browsers/browser-cri-client.ts +++ b/packages/server/lib/browsers/browser-cri-client.ts @@ -342,9 +342,8 @@ export class BrowserCriClient { try { await browserClient.send('Runtime.runIfWaitingForDebugger', undefined, 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 running Runtime.runIfWaitingForDebugger:', error) + // it's possible that the target was closed before we could tell it to run, in that case, just ignore + debug('error running Runtime.runIfWaitingForDebugger: %o', error) } } diff --git a/packages/server/lib/browsers/cri-client.ts b/packages/server/lib/browsers/cri-client.ts index 36e123eb3f12..47cd110e5e65 100644 --- a/packages/server/lib/browsers/cri-client.ts +++ b/packages/server/lib/browsers/cri-client.ts @@ -161,8 +161,7 @@ export class CriClient implements ICriClient { this._isChildTarget = !!this.host if (this._isChildTarget) { - // If crash listeners are added at the browser level, tabs/page connections do not - // emit them. + // If crash listeners are added at the browser level, tabs/page connections do not emit them. this.cdpConnection.on('Target.targetCrashed', async (event) => { debug('crash event detected', event) if (event.targetId !== this.targetId) { @@ -368,13 +367,18 @@ export class CriClient implements ICriClient { if (event.targetInfo.type !== 'service_worker' && event.targetInfo.type !== 'page' && event.targetInfo.type !== 'other') { await this.cdpConnection.send('Network.enable', this.protocolManager?.networkEnableOptions ?? DEFAULT_NETWORK_ENABLE_OPTIONS, event.sessionId) } + } catch (error) { + // it's possible that the target was closed before we could enable network, in that case, just ignore + debug('error attaching to target cri: %o', { error, event }) + } - if (event.waitingForDebugger) { + if (event.waitingForDebugger) { + try { await this.cdpConnection.send('Runtime.runIfWaitingForDebugger', undefined, event.sessionId) + } catch (error) { + // it's possible that the target was closed before we could tell it to run, in that case, just ignore + debug('error running Runtime.runIfWaitingForDebugger: %o', { error, event }) } - } 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/cri-client_spec.ts b/packages/server/test/unit/browsers/cri-client_spec.ts index 608cfcf2e55b..8b98c821fb37 100644 --- a/packages/server/test/unit/browsers/cri-client_spec.ts +++ b/packages/server/test/unit/browsers/cri-client_spec.ts @@ -147,6 +147,27 @@ describe('lib/browsers/cri-client', function () { expect(criStub.send).to.have.been.calledWith('Runtime.runIfWaitingForDebugger', undefined, sessionId) }) + + it('does not send Runtime.runIfWaitingForDebugger if not waiting for debugger', async () => { + await fireCDPEvent('Target.attachedToTarget', { + waitingForDebugger: false, + // @ts-ignore + targetInfo: { type: 'service_worker' }, + }) + + expect(criStub.send).not.to.have.been.calledWith('Runtime.runIfWaitingForDebugger') + }) + + it('sends Runtime.runIfWaitingForDebugger even if Network.enable throws', async () => { + criStub.send.withArgs('Network.enable').throws(new Error('ProtocolError: Inspected target closed')) + + await fireCDPEvent('Target.attachedToTarget', { + // @ts-ignore + targetInfo: { type: 'service_worker' }, + }) + + expect(criStub.send).to.have.been.calledWith('Runtime.runIfWaitingForDebugger') + }) }) }) From 00ee46d88bd5e0991efcda671ecfa4ba4541e21b Mon Sep 17 00:00:00 2001 From: Matthew Schile Date: Fri, 6 Dec 2024 15:42:38 -0700 Subject: [PATCH 2/5] updating spec --- .../test/unit/browsers/cri-client_spec.ts | 42 ++++++++++++------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/packages/server/test/unit/browsers/cri-client_spec.ts b/packages/server/test/unit/browsers/cri-client_spec.ts index 8b98c821fb37..e6300e3de778 100644 --- a/packages/server/test/unit/browsers/cri-client_spec.ts +++ b/packages/server/test/unit/browsers/cri-client_spec.ts @@ -3,6 +3,7 @@ import EventEmitter from 'events' import { ProtocolManagerShape } from '@packages/types' import type { CriClient } from '../../../lib/browsers/cri-client' import pDefer from 'p-defer' +import type Protocol from 'devtools-protocol' const { expect, proxyquire, sinon } = require('../../spec_helper') const DEBUGGER_URL = 'http://foo' @@ -108,11 +109,9 @@ describe('lib/browsers/cri-client', function () { it('does not enable network', async () => { await Promise.all(['service_worker', 'page', 'other'].map((type) => { return fireCDPEvent('Target.attachedToTarget', { - // do not need entire event payload for this test - // @ts-ignore targetInfo: { type, - }, + } as Protocol.Target.TargetInfo, }) })) @@ -123,11 +122,9 @@ describe('lib/browsers/cri-client', function () { describe('target type is something other than service worker, page, or other', () => { it('enables network', async () => { await fireCDPEvent('Target.attachedToTarget', { - // do not need entire event payload for this test - // @ts-ignore targetInfo: { - type: 'somethin else', - }, + type: 'iframe', + } as Protocol.Target.TargetInfo, }) expect(criStub.send).to.have.been.calledWith('Network.enable') @@ -135,14 +132,13 @@ describe('lib/browsers/cri-client', function () { }) describe('target is waiting for debugger', () => { - it('sends Runtime.runIfWaitingForDebugger', async () => { - const sessionId = 'abc123' + const sessionId = 'abc123' + it('sends Runtime.runIfWaitingForDebugger', async () => { await fireCDPEvent('Target.attachedToTarget', { waitingForDebugger: true, sessionId, - // @ts-ignore - targetInfo: { type: 'service_worker' }, + targetInfo: { type: 'service_worker' } as Protocol.Target.TargetInfo, }) expect(criStub.send).to.have.been.calledWith('Runtime.runIfWaitingForDebugger', undefined, sessionId) @@ -151,8 +147,8 @@ describe('lib/browsers/cri-client', function () { it('does not send Runtime.runIfWaitingForDebugger if not waiting for debugger', async () => { await fireCDPEvent('Target.attachedToTarget', { waitingForDebugger: false, - // @ts-ignore - targetInfo: { type: 'service_worker' }, + sessionId, + targetInfo: { type: 'service_worker' } as Protocol.Target.TargetInfo, }) expect(criStub.send).not.to.have.been.calledWith('Runtime.runIfWaitingForDebugger') @@ -162,11 +158,25 @@ describe('lib/browsers/cri-client', function () { criStub.send.withArgs('Network.enable').throws(new Error('ProtocolError: Inspected target closed')) await fireCDPEvent('Target.attachedToTarget', { - // @ts-ignore - targetInfo: { type: 'service_worker' }, + waitingForDebugger: true, + sessionId, + targetInfo: { type: 'iframe' } as Protocol.Target.TargetInfo, + }) + + expect(criStub.send).to.have.been.calledWith('Network.enable').and.to.have.thrown() + expect(criStub.send).to.have.been.calledWith('Runtime.runIfWaitingForDebugger', undefined, sessionId) + }) + + it('continues even if Runtime.runIfWaitingForDebugger throws', async () => { + criStub.send.withArgs('Runtime.runIfWaitingForDebugger').throws(new Error('ProtocolError: Inspected target closed')) + + await fireCDPEvent('Target.attachedToTarget', { + waitingForDebugger: true, + sessionId, + targetInfo: { type: 'service_worker' } as Protocol.Target.TargetInfo, }) - expect(criStub.send).to.have.been.calledWith('Runtime.runIfWaitingForDebugger') + expect(criStub.send).to.have.been.calledWith('Runtime.runIfWaitingForDebugger', undefined, sessionId).and.to.have.thrown() }) }) }) From f43173c33859a8b15846e5b326f01462a975d0a3 Mon Sep 17 00:00:00 2001 From: Matthew Schile Date: Fri, 6 Dec 2024 15:46:38 -0700 Subject: [PATCH 3/5] updating changelog --- cli/CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index b12efca21790..3308c8cb8cf5 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -1,4 +1,12 @@ +## 13.16.2 + +_Released 12/17/2024_ + +**Bugfixes:** + +- Fixed and issue where targets may hang if `Network.enable` is not implemented for the target. Addresses [#29876](https://github.com/cypress-io/cypress/issues/29876). + ## 13.16.1 _Released 12/03/2024_ From 431648cdef5c133a5037c09800d93d004faf233a Mon Sep 17 00:00:00 2001 From: Matthew Schile Date: Fri, 6 Dec 2024 16:06:20 -0700 Subject: [PATCH 4/5] update changelog --- cli/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 3308c8cb8cf5..7f743c26132e 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -1,7 +1,7 @@ ## 13.16.2 -_Released 12/17/2024_ +_Released 12/17/2024 (PENDING)_ **Bugfixes:** From 36aa3977233fcdab84898a1734268dd831d4c804 Mon Sep 17 00:00:00 2001 From: Matt Schile Date: Mon, 9 Dec 2024 08:30:50 -0700 Subject: [PATCH 5/5] Update cli/CHANGELOG.md --- cli/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 7f743c26132e..1d0971682710 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -5,7 +5,7 @@ _Released 12/17/2024 (PENDING)_ **Bugfixes:** -- Fixed and issue where targets may hang if `Network.enable` is not implemented for the target. Addresses [#29876](https://github.com/cypress-io/cypress/issues/29876). +- Fixed an issue where targets may hang if `Network.enable` is not implemented for the target. Addresses [#29876](https://github.com/cypress-io/cypress/issues/29876). ## 13.16.1