Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: chrome missing CRI client after browser relaunch #29663

Merged
merged 5 commits into from
Jun 21, 2024

Conversation

AtofStryker
Copy link
Contributor

@AtofStryker AtofStryker commented Jun 12, 2024

Additional details

This PR addresses the missing CRI client issue when chrome unexpectedly closes. When this would happen, we wait for the browser to reconnect up to 3 attempts. In this case though, since the browser closed, we closed the CRI client as well and we are able to successfully relaunch the browser, expecting the CRI client to be there, but its not so we get this error.

To fix this, we need to make sure we detect when the browser unexpectedly closes and let the launcher know that the instance is no longer active. This way, we can set shouldLaunchNewTab to false which will prompt the recreation of the CRI client as the browser instance is new.

That being said, I am unable to create a system test where the browser closes in the manner I need it to. I was only able to recreate the issue on this branch against this test suite. In the actual repo, I have added a brief assertion to the launcher unit test and added a new integration test for the run mode in the server, which integrates most the aspects of the server while stubbing out the config loader child process and a few other directory specific things since we are mocking the file system.

Steps to test

run the integration tests in the server for the monorepo, or try the specified branch (which is the reprod branch but with the fix) against the test repo to see the crash in action. You should see the log which indicates we recovered and rehydrate the CRI client correctly.

How has the user experience changed?

PR Tasks

Copy link

cypress bot commented Jun 12, 2024

5 flaky tests on run #55855 ↗︎

0 29292 1328 0 Flakiness 5

Details:

Merge branch 'develop' of github.com:cypress-io/cypress into fix/browser_missing...
Project: cypress Commit: 5d46189119
Status: Passed Duration: 21:43 💡
Started: Jun 20, 2024 3:00 PM Ended: Jun 20, 2024 3:22 PM
Flakiness  commands/waiting.cy.js • 1 flaky test • 5x-driver-chrome:beta

View Output

Test Artifacts
... > errors > throws when waiting for 2nd response to route Test Replay
Flakiness  e2e/origin/navigation.cy.ts • 1 flaky test • 5x-driver-chrome:beta

View Output

Test Artifacts
... > times out in cy.origin with foobar spec bridge defined Test Replay
Flakiness  commands/net_stubbing.cy.ts • 2 flaky tests • 5x-driver-webkit

View Output

Test Artifacts
... > with `resourceType` > can match a proxied image request by resourceType
    </td>
  </tr>
  <tr>
    <td colspan="2">
      <a href="https://cloud.cypress.io/projects/ypt4pf/runs/55855/overview/4f8fb43b-1589-4b63-86c5-9dd18de8bc8f?reviewViewBy=FLAKY&utm_source=github&utm_medium=flaky&utm_campaign=view%20test">
        ... > stops waiting when an xhr request is canceled
      </a>
    </td>
    <td>
      
    </td>
  </tr></table>
Flakiness  commands/waiting.cy.js • 1 flaky test • 5x-driver-webkit

View Output

Test Artifacts
... > errors > throws waiting for the 3rd response
    </td>
  </tr></table>

Review all test suite changes for PR #29663 ↗︎

@AtofStryker AtofStryker force-pushed the fix/browser_missing_cri_client branch 3 times, most recently from b390d76 to a313ef8 Compare June 13, 2024 14:13
@AtofStryker AtofStryker force-pushed the fix/browser_missing_cri_client branch from a313ef8 to cbc2b02 Compare June 13, 2024 17:57
@AtofStryker AtofStryker marked this pull request as ready for review June 13, 2024 18:09
@AtofStryker AtofStryker force-pushed the fix/browser_missing_cri_client branch from cbc2b02 to a26f387 Compare June 13, 2024 18:10
@AtofStryker AtofStryker requested review from mschile, ryanthemanuel and cacieprins and removed request for mschile June 13, 2024 18:11
@AtofStryker AtofStryker requested review from mschile and removed request for cacieprins June 19, 2024 12:36
@AtofStryker AtofStryker force-pushed the fix/browser_missing_cri_client branch from dc4945f to 794dd03 Compare June 19, 2024 12:47
@jennifer-shehane jennifer-shehane merged commit af2b764 into develop Jun 21, 2024
80 of 82 checks passed
@jennifer-shehane jennifer-shehane deleted the fix/browser_missing_cri_client branch June 21, 2024 17:27
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 1, 2024

Released in 13.13.0.

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

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Jul 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing browserCriClient in connectToNewSpec
4 participants