fix: chrome crash when CRI client is disconnected

[AtofStryker]

• Closes Missing browserCriClient in connectToNewSpec #24650

Additional details

When running the browser_crash_handling_spec.js -> when the browser process crashes in chrome spec with video: false, Cypress enters an infinite test loop where it keeps trying to run chrome_process_crash.cy.js.

The reason this is happening is when the chrome_process_crash.cy.js fails and crashes the browser, the CRI client becomes disconnected. However, the browser-cri-client is unaware of the disconnect. When resetBrowserTabsForNextTest is called at the end of the test, and Target.createTarget is called, the promise never resolves and hangs, meaning that the server state is never reset. We want resetBrowserTabsForNextTest to behave as a no-op, whether is succeeds or fails. Additionally, if the CRI socket is closed and a reconnect is attempted, and there are no pending commands in the queue and the socket is STILL closed, we want to reject send commands to prevent hanging promises. This doesn't seem to happen with video on I believe since the the video processing takes time, and in that time the browser CRI client is reconnected, which allows the state to be reset.

Additionally, after fixing the hanging promise and the state resetting, the relaunchBrowser method was getting called twice, once in open_project and again in browsers/index.ts when the browser crashes. I think this due to the run spec now actually resolving, which now hits the correct relaunchBrowser, meaning the one in browsers/index.ts can be removed.

Lastly, when the browser crashes, we want shouldLaunchNewTab to be treated as false, which is handled by runSpec. After crash this was being set to true, which was trying to run connectToNewSpec when it shouldn't, resulting in the infamous error Missing browserCriClient in connectToNewSpec here as the CRI client doesn't get instantiated yet.

If a chrome crash is detected, we need to reset isFirstSpec (renamed to isFirstSpecInBrowser) to false, which in turn sets shouldLaunchNewTab to false, which creates the CRI client as expected

Steps to test

run the browser_crash_handling_spec.js -> when the browser process crashes in chrome with video on and off (added in this PR)

PR Tasks

• Have tests been added/updated?
• Has a PR for user-facing changes been opened in cypress-documentation?
• Have API changes been updated in the type definitions?

[cypress]

5 flaky tests on run #48246 ↗︎

0

672

32

0

5

Details:

Merge branch 'develop' of github.com:cypress-io/cypress into fix/chrome_crash_re...
Project: cypress	Commit: dc56fabf06
Status: Passed	Duration: 18:42 💡
Started: Jun 26, 2023 6:36 PM	Ended: Jun 26, 2023 6:54 PM

  cypress\e2e\create-from-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
... > Shows create from component card for React projects		Output Screenshots Video

  cypress\e2e\top-nav.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
... > logouts user if cloud request returns unauthorized		Output Screenshots Video

  cypress\e2e\specs_list_latest_runs.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs		Output Screenshots Video

  cypress\e2e\cypress-in-cypress-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
Cypress In Cypress CT > default config > redirects to the specs list with error if a spec is not found		Output Screenshots Video

  cypress\e2e\cypress-origin-communicator.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
Cypress In Cypress Origin Communicator > cy.origin passivity with app interactions > passes upon test reload mid test execution		Output Screenshots Video

_{This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.}

[jazpearson]

@AtofStryker i've tested out the changes and so far, i've not had the issue reproduce!!
although quite a few failures due to issues seen in 12.15.0 with cy.task

[AtofStryker]

@AtofStryker i've tested out the changes and so far, i've not had the issue reproduce!!
That's great to here. If you do run into it, let us know, but hopefully this is the fix you need

although quite a few failures due to issues seen in 12.15.0 with cy.task
That is currently expected. There was a regression in 12.15.0 which should be released in 12.16.0 today. Once that release goes out I can try updating the binaries to help reduce noise

[jazpearson]

That is currently expected. There was a regression in 12.15.0 which should be released in 12.16.0 today. Once that release goes out I can try updating the binaries to help reduce noise

Great. I will test once the binaries are updated and then it'll be MUCH easier to test this, Thanks again.

[AtofStryker]

@jazpearson updated binaries incoming here

[cypress-bot]

Released in 12.17.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v12.17.0, please open a new issue.