Merge branch 'develop' into mschile/firefox_userChrome

cypress-io · Dec 9, 2024 · 7c817ea · 7c817ea
2 parents c20bb98 + e96593b
commit 7c817ea
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 20 deletions.
diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md
@@ -5,7 +5,8 @@ _Released 12/17/2024 (PENDING)_
 
 **Bugfixes:**
 
-- Updated Firefox `userChrome.css` file to correctly hide the toolbox during headless mode. Addresses [#30721](https://github.com/cypress-io/cypress/issues/30721).
+- 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).
+- Updated Firefox `userChrome.css` to correctly hide the toolbox during headless mode. Addresses [#30721](https://github.com/cypress-io/cypress/issues/30721).
 
 ## 13.16.1
 

diff --git 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
@@ -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
@@ -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,30 +122,62 @@ 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')
         })
       })
 
       describe('target is waiting for debugger', () => {
+        const sessionId = 'abc123'
+
         it('sends Runtime.runIfWaitingForDebugger', async () => {
-          const sessionId = 'abc123'
+          await fireCDPEvent('Target.attachedToTarget', {
+            waitingForDebugger: true,
+            sessionId,
+            targetInfo: { type: 'service_worker' } as Protocol.Target.TargetInfo,
+          })
+
+          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,
+            sessionId,
+            targetInfo: { type: 'service_worker' } as Protocol.Target.TargetInfo,
+          })
+
+          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', {
             waitingForDebugger: true,
             sessionId,
-            // @ts-ignore
-            targetInfo: { type: 'service_worker' },
+            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', undefined, sessionId).and.to.have.thrown()
+        })
       })
     })