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

Failed requests retry 25 times #19043

Closed
mjhenkes opened this issue Nov 22, 2021 · 3 comments · Fixed by #19161
Closed

Failed requests retry 25 times #19043

mjhenkes opened this issue Nov 22, 2021 · 3 comments · Fixed by #19161
Assignees

Comments

@mjhenkes
Copy link
Member

mjhenkes commented Nov 22, 2021

Current behavior

Failed requests will retry 25 times.

From my understanding previously chrome would not retry failed requests during automation. So cypress implemented our own retry logic. Since that time it seems that chrome has enabled retries for automation. It seems that the combination of our retries and chromes retries end up causing failed requests to retry 25 times. I think that we will retry 5 times for each time chrome retries.

Current location of chrome's auto reloading code: https://source.chromium.org/chromium/chromium/src/+/main:components/error_page/content/browser/net_error_auto_reloader.cc;l=22;drc=a618b831a17a7d178f56ee720e55483e7403b01e

Desired behavior

I believe tests should retry only 5 times, but we're going to have to also investigate the retry behviour in firefox and webkit to find out what the optimal solution is.

Possibilities

*disable browser retries and handle it with cypress
*disable cypress retries and let the browsers handle it.

We also need to identify when this change was made and determine if we wish to support reties for older versions of chrome.

Test code to reproduce

Using branch mimatched-xhr-calls
I've created a test case called 'retry_spec` in the driver to showcase the problem.

The test introduces a route that will fail x number of times before succeeding. When set lower than 25 it will eventually succeed. Higher than 24, it will fail.

Also these tests can be enabled to check on chrome browser retry behavior.
https://github.com/cypress-io/cypress/blob/develop/system-tests/test/network_error_handling_spec.js#L257

Cypress Version

9.0.0

Other

No response

@flotwig
Copy link
Contributor

flotwig commented Nov 30, 2021

Using the script https://www.chromium.org/developers/bisect-builds-py I was able to bisect to the commit that broke the expected behavior in Chromium.

bisect-chrome-builds.py --good=612437 --bad=942385 --archive=linux64 --not-interactive --verify-range --command="zsh -c 'cd `pwd`/system-tests && DEBUG=cypress:server:network-error-handling-spec CHROME_PATH=%p yarn test network_error'" 

--verify-range checks the start and end of the range first to see if a change has occurred at all.

The revision numbers I chose bisected from 72.0.3626.0 to 98.0.4710.4, and I ran this against most of the tests in the Google Chrome suite of system-tests/test/network_error_handling_spec.js, one at a time using .only for ease of reading results.

The results:

Without Proxy Server

  • retries 3+ times when receiving immediate reset - no change
  • retries 3+ times when receiving reset after headers - changed: Bad revision: 639996
    • You are probably looking for a change made after 639989 (known good), but no later than 639996 (first known bad). CHANGELOG
  • does not retry if reset during body - no change

With Proxy Server

  • retries 3+ times when receiving immediate reset - no change
  • retries 3+ times when receiving reset after headers - changed: Bad revision: 639996
    • You are probably looking for a change made after 639989 (known good), but no later than 639996 (first known bad). CHANGELOG
  • does not retry if reset during body - no change

The commit initially landed in a change captured 75.0.3732.0 - 2019-06-04. Version info via https://omahaproxy.appspot.com/.


The way I see it there are a few possible approaches to fixing this

  1. Try to find the version number and behavior for retries and account for it in the network layer
    • Has to be maintained for every possible browser, which is super error prone and maintenance heavy
    • We can't even always know what kind of retry is happening
    • The above caveats mean we probably can't do this.
  2. We could run a detection procedure before running the tests in the browser, to determine at runtime the behavior of the browser when it comes to retries.
    • This removes the maintenance burden of option 1.
    • However, it has the same caveat: We can't even always know what kind of retry is happening to handle it.
    • Would also slow down test runs.
    • The above caveats mean we probably can't do this.
  3. We can remove the proxy-layer retry behavior.
    • Google Chrome broke this in 2019, they now automatically retry in the browser, so we don't need it if users use newer browsers.
    • We don't know for Firefox or WebKit what the behavior is either.
    • Probably the most sane option today compared to the unreliability of 1 or 2.

It's also worth noting that our retry layer adds complexity in unexpected ways; for example, #19039

I'll open a PR to disable/remove Cypress's proxy retry layer since that looks like the only realistic path forward here.

@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review stage: work in progress and removed stage: to do stage: needs review The PR code is done & tested, needs review labels Nov 30, 2021
@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Dec 8, 2021
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Dec 8, 2021

The code for this is done in cypress-io/cypress#19161, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Dec 21, 2021

Released in 9.2.0.

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

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Dec 21, 2021
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 a pull request may close this issue.

2 participants