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: websocket closed (tentative) #29347

Merged
merged 12 commits into from
Apr 22, 2024
Merged

Conversation

cacieprins
Copy link
Contributor

@cacieprins cacieprins commented Apr 17, 2024

Additional details

This is a tentative fix. This issue has been very difficult if not impossible to reproduce, and this is my best guess solution.

Occasionally, certain systems can send CDP commands that will fail due to the CRI websocket being closed with those commands mid-flight. This is most common with video recording ACK commands, but can potentially occur with any automation command. Prior to chrome-remote-interface@0.33.0, it seems as though these commands failed silently. 0.33.0 introduced behavior where it will throw an error in this situation. This exception was uncaught, causing Cypress to intermittently exit with code 1 even thought a given spec had succeeded.

This PR:

  • Adds additional verbose debug steps to the CRI client, for non-send/receive websocket events
  • handles Error: WebSocket connection closed errors similarly to WebSocket is not open and WebSocket already in CLOSING or CLOSED state errors: the command is enqueued, and reconnection to the CRI websocket is attempted
  • When reconnecting to the CRI websocket due to a failed command, CRI Client now checks to see if there is an in-flight reconnection process and if so, awaits that instead of kicking off a new one
  • removes enablement commands (e.g., Runtime.addBinding, DOM.enable) from the command queue, if present, when sending them upon reconnection. This prevents them from being re-sent when iterating through the queued commands.
  • Updates integration tests so they will fail if this exception is uncaught

Steps to test

Run server unit and integration tests. A reproduction of the original issue is unavailable, and confirmation on a fix will be pending user feedback.

How has the user experience changed?

PR Tasks

Copy link

cypress bot commented Apr 19, 2024

Passing run #55090 ↗︎

0 19 0 0 Flakiness 0

Details:

Merge branch 'develop' into cacie/fix/websocket-closed
Project: cypress Commit: 26477fda05
Status: Passed Duration: 07:26 💡
Started: Apr 22, 2024 7:15 PM Ended: Apr 22, 2024 7:23 PM

Review all test suite changes for PR #29347 ↗︎

@cacieprins cacieprins marked this pull request as ready for review April 22, 2024 14:22
cli/CHANGELOG.md Outdated Show resolved Hide resolved
@AtofStryker AtofStryker self-requested a review April 22, 2024 17:23
cli/CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Bill Glesias <bglesias@gmail.com>

const WEBSOCKET_NOT_OPEN_RE = /^WebSocket is (?:not open|already in CLOSING or CLOSED state)/
const WEBSOCKET_NOT_OPEN_RE = /^WebSocket (?:connection closed|is (?:not open|already in CLOSING or CLOSED state))/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we document what the various messages are that we're covering here? I always like to have examples around regexes for my future self.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in a613bca

@cacieprins cacieprins merged commit 4e7e3a2 into develop Apr 22, 2024
120 of 127 checks passed
@cacieprins cacieprins deleted the cacie/fix/websocket-closed branch April 22, 2024 19:55
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Apr 23, 2024

Released in 13.8.1.

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

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Apr 23, 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.

Error: WebSocket connection closed (in cypress 13.6)
4 participants