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

chore: additional context on protocol upload network errors #28986

Merged
merged 7 commits into from
Mar 6, 2024

Conversation

cacieprins
Copy link
Contributor

@cacieprins cacieprins commented Feb 21, 2024

Additional details

HTTP status codes and URLs that fail are useful data when investigating network errors. Previously, we were only reporting the status text.

In order to reduce time spent on uploading capture databases in a failure state, retry count has been reduced from 7 to 2, for 3 total attempts.

Steps to test

How has the user experience changed?

PR Tasks

@cacieprins cacieprins marked this pull request as ready for review February 21, 2024 15:22
Copy link

cypress bot commented Feb 21, 2024

1 flaky test on run #54401 ↗︎

0 6083 81 0 Flakiness 1

Details:

Merge branch 'develop' into cacie/chore/additional-upload-error-info
Project: cypress Commit: b2b3bfb6d0
Status: Passed Duration: 17:19 💡
Started: Mar 6, 2024 5:00 PM Ended: Mar 6, 2024 5:17 PM
Flakiness  cypress/e2e/specs_list_latest_runs.cy.ts • 1 flaky test • app-e2e

View Output

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

Review all test suite changes for PR #28986 ↗︎

@@ -323,7 +323,17 @@ export class ProtocolManager implements ProtocolManagerShape {
}
}

const errorMessage = await res.json().catch(() => res.statusText)
const errorMessage = await res.json().catch(() => {
const url = new URL(uploadUrl)
Copy link
Member

Choose a reason for hiding this comment

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

Can't imagine why this wouldn't be a valid url, but should we gaurd against if this ends up raising an error as invalid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While investigating this, I don't think we need to guard at this point, because this code is unreachable if uploadUrl is not a valid URL: fetch on L#305 will throw in this case, which gets caught on L#345.

@cacieprins cacieprins marked this pull request as draft February 22, 2024 17:40
@cacieprins cacieprins marked this pull request as ready for review February 22, 2024 18:24
@cacieprins cacieprins requested review from mschile and ryanthemanuel and removed request for mschile March 5, 2024 16:35
@cacieprins cacieprins merged commit 006faaa into develop Mar 6, 2024
81 of 82 checks passed
@cacieprins cacieprins deleted the cacie/chore/additional-upload-error-info branch March 6, 2024 17:49
cacieprins added a commit that referenced this pull request Mar 12, 2024
@cypress cypress bot mentioned this pull request Mar 12, 2024
3 tasks
cacieprins added a commit that referenced this pull request Mar 12, 2024
* chore: release 13.7.0

* fix incorrect regression note in changelog

* Update cli/CHANGELOG.md

Co-authored-by: Matt Schile <mschile@cypress.io>

* changelog entries for #28986

---------

Co-authored-by: Jennifer Shehane <jennifer@cypress.io>
Co-authored-by: Matt Schile <mschile@cypress.io>
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Mar 13, 2024

Released in 13.7.0.

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

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