fix: chrome crash when CRI client is disconnected (#27116)

* fix: fix chrome crash when CRI client is disconnected

* chore: build binaries for chrome recovery crash for users to test

* chore: add changelog entry

.circleci/workflows.yml

@@ -30,8 +30,7 @@ mainBuildFilters: &mainBuildFilters
         - /^release\/\d+\.\d+\.\d+$/
         # use the following branch as well to ensure that v8 snapshot cache updates are fully tested
         - 'update-v8-snapshot-cache-on-develop'
-        - 'matth/feat/chrome-headless'
-        - 'lmiller/fix-windows-regressions'
+        - 'fix/chrome_crash_recovery'
 
 # usually we don't build Mac app - it takes a long time
 # but sometimes we want to really confirm we are doing the right thing
@@ -42,7 +41,7 @@ macWorkflowFilters: &darwin-workflow-filters
     - equal: [ develop, << pipeline.git.branch >> ]
     # use the following branch as well to ensure that v8 snapshot cache updates are fully tested
     - equal: [ 'update-v8-snapshot-cache-on-develop', << pipeline.git.branch >> ]
-    - equal: [ 'matth/feat/chrome-headless', << pipeline.git.branch >> ]
+    - equal: [ 'fix/chrome_crash_recovery', << pipeline.git.branch >> ]
     - matches:
         pattern: /^release\/\d+\.\d+\.\d+$/
         value: << pipeline.git.branch >>
@@ -53,8 +52,7 @@ linuxArm64WorkflowFilters: &linux-arm64-workflow-filters
     - equal: [ develop, << pipeline.git.branch >> ]
     # use the following branch as well to ensure that v8 snapshot cache updates are fully tested
     - equal: [ 'update-v8-snapshot-cache-on-develop', << pipeline.git.branch >> ]
-    - equal: [ 'lmiller/fix-windows-regressions', << pipeline.git.branch >> ]
-    - equal: [ 'matth/feat/chrome-headless', << pipeline.git.branch >> ]
+    - equal: [ 'fix/chrome_crash_recovery', << pipeline.git.branch >> ]
     - matches:
         pattern: /^release\/\d+\.\d+\.\d+$/
         value: << pipeline.git.branch >>
@@ -74,7 +72,7 @@ windowsWorkflowFilters: &windows-workflow-filters
     - equal: [ develop, << pipeline.git.branch >> ]
     # use the following branch as well to ensure that v8 snapshot cache updates are fully tested
     - equal: [ 'update-v8-snapshot-cache-on-develop', << pipeline.git.branch >> ]
-    - equal: [ 'privileged-commands-refactor', << pipeline.git.branch >> ]
+    - equal: [ 'fix/chrome_crash_recovery', << pipeline.git.branch >> ]
     - matches:
         pattern: /^release\/\d+\.\d+\.\d+$/
         value: << pipeline.git.branch >>
@@ -141,7 +139,7 @@ commands:
       - run:
           name: Check current branch to persist artifacts
           command: |
-            if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "update-v8-snapshot-cache-on-develop" && "$CIRCLE_BRANCH" != "lmiller/fix-windows-regressions" ]]; then
+            if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "update-v8-snapshot-cache-on-develop" && "$CIRCLE_BRANCH" != "fix/chrome_crash_recovery" ]]; then
               echo "Not uploading artifacts or posting install comment for this branch."
               circleci-agent step halt
             fi

cli/CHANGELOG.md

@@ -1,4 +1,12 @@
 <!-- See the ../guides/writing-the-cypress-changelog.md for details on writing the changelog. -->
+## 12.16.1
+
+_Released 07/05/2023 (PENDING)_
+
+**Bugfixes:**
+
+- Fixed an issue where chrome was not recovering from browser crashes properly. Fixes [#24650](https://github.com/cypress-io/cypress/issues/24650).
+
 ## 12.16.0
 
 _Released 06/26/2023_

packages/server/lib/browsers/cdp_automation.ts

@@ -161,7 +161,14 @@ export class CdpAutomation {
   async startVideoRecording (writeVideoFrame: WriteVideoFrame, screencastOpts) {
     this.onFn('Page.screencastFrame', async (e) => {
       writeVideoFrame(Buffer.from(e.data, 'base64'))
-      await this.sendDebuggerCommandFn('Page.screencastFrameAck', { sessionId: e.sessionId })
+      try {
+        await this.sendDebuggerCommandFn('Page.screencastFrameAck', { sessionId: e.sessionId })
+      } catch (e) {
+        // swallow this error if the CRI connection was reset
+        if (!e.message.includes('browser CRI connection was reset')) {
+          throw e
+        }
+      }
     })
 
     await this.sendDebuggerCommandFn('Page.startScreencast', screencastOpts)

packages/server/lib/browsers/cri-client.ts

@@ -188,6 +188,11 @@ export const create = async (target: string, onAsynchronousError: Function, host
 
           await reconnect()
 
+          // if enqueued commands were wiped out from the reconnect and the socket is already closed, reject the command as it will never be run
+          if (enqueuedCommands.length === 0 && closed) {
+            return Promise.reject(new Error(`${command} will not run as browser CRI connection was reset`))
+          }
+
           return p
         }
       }

packages/server/lib/browsers/index.ts

@@ -241,8 +241,6 @@ export = {
         }
 
         await options.onError(err)
-
-        await options.relaunchBrowser!()
       }
     })
 

packages/server/lib/modes/run.ts

@@ -458,7 +458,7 @@ function listenForProjectEnd (project, exit): Bluebird<any> {
   })
 }
 
-async function waitForBrowserToConnect (options: { project: Project, socketId: string, onError: (err: Error) => void, spec: SpecWithRelativeRoot, isFirstSpec: boolean, testingType: string, experimentalSingleTabRunMode: boolean, browser: Browser, screenshots: ScreenshotMetadata[], projectRoot: string, shouldLaunchNewTab: boolean, webSecurity: boolean, videoRecording?: VideoRecording }) {
+async function waitForBrowserToConnect (options: { project: Project, socketId: string, onError: (err: Error) => void, spec: SpecWithRelativeRoot, isFirstSpecInBrowser: boolean, testingType: string, experimentalSingleTabRunMode: boolean, browser: Browser, screenshots: ScreenshotMetadata[], projectRoot: string, shouldLaunchNewTab: boolean, webSecurity: boolean, videoRecording?: VideoRecording }) {
   if (globalThis.CY_TEST_MOCK?.waitForBrowserToConnect) return Promise.resolve()
 
   const { project, socketId, onError, spec } = options
@@ -480,7 +480,7 @@ async function waitForBrowserToConnect (options: { project: Project, socketId: s
     return currentSetScreenshotMetadata(data)
   }
 
-  if (options.experimentalSingleTabRunMode && options.testingType === 'component' && !options.isFirstSpec) {
+  if (options.experimentalSingleTabRunMode && options.testingType === 'component' && !options.isFirstSpecInBrowser) {
     // reset browser state to match default behavior when opening/closing a new tab
     await openProject.resetBrowserState()
 
@@ -768,7 +768,7 @@ async function runSpecs (options: { config: Cfg, browser: Browser, sys: any, hea
     })
   }
 
-  let isFirstSpec = true
+  let isFirstSpecInBrowser = true
 
   async function runEachSpec (spec: SpecWithRelativeRoot, index: number, length: number, estimated: number) {
     const span = telemetry.startSpan({
@@ -779,16 +779,21 @@ async function runSpecs (options: { config: Cfg, browser: Browser, sys: any, hea
     span?.setAttributes({
       specName: spec.name,
       type: spec.specType,
-      firstSpec: isFirstSpec,
+      firstSpec: isFirstSpecInBrowser,
     })
 
     if (!options.quiet) {
       printResults.displaySpecHeader(spec.relativeToCommonRoot, index + 1, length, estimated)
     }
 
-    const { results } = await runSpec(config, spec, options, estimated, isFirstSpec, index === length - 1)
+    const { results } = await runSpec(config, spec, options, estimated, isFirstSpecInBrowser, index === length - 1)
 
-    isFirstSpec = false
+    if (results?.error?.includes('We detected that the Chrome process just crashed with code')) {
+      // If the browser has crashed, make sure isFirstSpecInBrowser is set to true as the browser will be relaunching
+      isFirstSpecInBrowser = true
+    } else {
+      isFirstSpecInBrowser = false
+    }
 
     debug('spec results %o', results)
 
@@ -910,7 +915,7 @@ async function runSpecs (options: { config: Cfg, browser: Browser, sys: any, hea
   return results
 }
 
-async function runSpec (config, spec: SpecWithRelativeRoot, options: { project: Project, browser: Browser, onError: (err: Error) => void, config: Cfg, quiet: boolean, exit: boolean, testingType: TestingType, socketId: string, webSecurity: boolean, projectRoot: string } & Pick<Cfg, 'video' | 'videosFolder' | 'videoCompression' | 'videoUploadOnPasses'>, estimated, isFirstSpec, isLastSpec) {
+async function runSpec (config, spec: SpecWithRelativeRoot, options: { project: Project, browser: Browser, onError: (err: Error) => void, config: Cfg, quiet: boolean, exit: boolean, testingType: TestingType, socketId: string, webSecurity: boolean, projectRoot: string } & Pick<Cfg, 'video' | 'videosFolder' | 'videoCompression' | 'videoUploadOnPasses'>, estimated, isFirstSpecInBrowser, isLastSpec) {
   const { project, browser, onError } = options
 
   const { isHeadless } = browser
@@ -935,7 +940,7 @@ async function runSpec (config, spec: SpecWithRelativeRoot, options: { project:
 
     telemetry.startSpan({ name: 'video:capture' })
 
-    if (config.experimentalSingleTabRunMode && !isFirstSpec && project.videoRecording) {
+    if (config.experimentalSingleTabRunMode && !isFirstSpecInBrowser && project.videoRecording) {
       // in single-tab mode, only the first spec needs to create a videoRecording object
       // which is then re-used between specs
       return await startVideoRecording({ ...opts, previous: project.videoRecording })
@@ -976,9 +981,9 @@ async function runSpec (config, spec: SpecWithRelativeRoot, options: { project:
       webSecurity: options.webSecurity,
       projectRoot: options.projectRoot,
       testingType: options.testingType,
-      isFirstSpec,
+      isFirstSpecInBrowser,
       experimentalSingleTabRunMode: config.experimentalSingleTabRunMode,
-      shouldLaunchNewTab: !isFirstSpec,
+      shouldLaunchNewTab: !isFirstSpecInBrowser,
     }),
   ])
 

packages/server/lib/open_project.ts

@@ -198,7 +198,13 @@ export class OpenProject {
   }
 
   async resetBrowserTabsForNextTest (shouldKeepTabOpen: boolean) {
-    return this.projectBase?.resetBrowserTabsForNextTest(shouldKeepTabOpen)
+    try {
+      await this.projectBase?.resetBrowserTabsForNextTest(shouldKeepTabOpen)
+    } catch (e) {
+      // If the CRI client disconnected or crashed, we want to no-op here so that anything
+      // depending on resetting the browser tabs can continue with further operations
+      return
+    }
   }
 
   async resetBrowserState () {

packages/server/test/unit/browsers/cri-client_spec.ts

@@ -96,6 +96,20 @@ describe('lib/browsers/cri-client', function () {
           })
         })
       })
+
+      context('closed', () => {
+        it(`when socket is closed mid send'`, async function () {
+          const err = new Error('WebSocket is not open: readyState 3 (CLOSED)')
+
+          send.onFirstCall().rejects(err)
+
+          const client = await getClient()
+
+          await client.close()
+
+          expect(client.send('Browser.getVersion', { baz: 'quux' })).to.be.rejectedWith('Browser.getVersion will not run as browser CRI connection was reset')
+        })
+      })
     })
   })