Merge branch 'develop' into feature/experimental-retries

cli/CHANGELOG.md

@@ -1,4 +1,5 @@
 <!-- See the ../guides/writing-the-cypress-changelog.md for details on writing the changelog. -->
+
 ## 13.4.0
 
 _Released 10/25/2023 (PENDING)_
@@ -7,6 +8,10 @@ _Released 10/25/2023 (PENDING)_
 
 - Introduced experimental configuration options for advanced retry logic: adds `experimentalStrategy` and `experimentalOptions` keys to the `retry` configuration key. See [Experimental Flake Detection Features](https://docs.cypress.io/guides/references/experiments/#Experimental-Flake-Detection-Features) in the documentation. Addressed in [#27930](https://github.com/cypress-io/cypress/pull/27930).
 
+**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_

cli/__snapshots__/build_spec.js

@@ -4,7 +4,7 @@ exports['package.json build outputs expected properties 1'] = {
   'version': 'x.y.z',
   'buildInfo': 'replaced by normalizePackageJson',
   'description': 'Cypress is a next generation front end testing tool built for the modern web',
-  'homepage': 'https://github.com/cypress-io/cypress',
+  'homepage': 'https://cypress.io',
   'license': 'MIT',
   'bugs': {
     'url': 'https://github.com/cypress-io/cypress/issues',

package.json

@@ -221,7 +221,7 @@
     "type": "git",
     "url": "https://github.com/cypress-io/cypress.git"
   },
-  "homepage": "https://github.com/cypress-io/cypress",
+  "homepage": "https://cypress.io",
   "bugs": {
     "url": "https://github.com/cypress-io/cypress/issues"
   },

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)
           }
         })
 

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)
           }
         })
 

packages/server/test/integration/http_requests_spec.js

@@ -1035,7 +1035,7 @@ describe('Routes', () => {
         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)
+        this.timeout(900)
 
         nock(this.server.remoteStates.current().origin)
         .get('/')
@@ -1057,6 +1057,7 @@ describe('Routes', () => {
 
         // Wait 100 ms to make sure the request times out
         return Promise.delay(100).then(() => {
+          this.networkProxy.setPreRequestTimeout(1000)
           nock(this.server.remoteStates.current().origin)
           .get('/')
           .once()

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)

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',

system-tests/projects/e2e/shared-worker.js

@@ -1,14 +1,14 @@
-// eslint-disable-next-line no-undef
-importScripts('/sw.js')
-
-self.addEventListener('connect', (event) => {
+self.onconnect = (event) => {
   const port = event.ports[0]
 
   port.onmessage = (e) => {
+    // eslint-disable-next-line no-undef
+    importScripts('/sw.js')
+
     if (e.data.foo === 'baz') {
       port.postMessage({
         foo: 'baz2',
       })
     }
   }
-})
+}

system-tests/projects/e2e/web-worker.js

@@ -1,7 +1,7 @@
-// eslint-disable-next-line no-undef
-importScripts('/ww.js')
-
 onmessage = (e) => {
+  // eslint-disable-next-line no-undef
+  importScripts('/ww.js')
+
   if (e.data.foo === 'bar') {
     postMessage({
       foo: 'bar2',