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: fix failures and correlation in proxy #28094

Merged
merged 9 commits into from Oct 23, 2023
Merged

Conversation

ryanthemanuel
Copy link
Collaborator

Additional details

We recently made a change to clean up browser prerequests when requests are aborted, but we don't do the same thing with proxy prerequests. This PR ensures that we clean things up on both sides.

Steps to test

I added some integration tests to cover if CDP failures come in first vs. proxy failures.

How has the user experience changed?

n/a

PR Tasks

@ryanthemanuel ryanthemanuel requested review from chrisbreiding, cacieprins and mschile and removed request for cacieprins October 20, 2023 02:08
@cypress
Copy link

cypress bot commented Oct 20, 2023

35 flaky tests on run #51821 ↗︎

0 28150 1345 0 Flakiness 35

Details:

Merge branch 'develop' into ryanm/fix/proxy-fun
Project: cypress Commit: fbc266d394
Status: Passed Duration: 19:02 💡
Started: Oct 23, 2023 8:39 PM Ended: Oct 23, 2023 8:59 PM
Flakiness  cypress/cypress.cy.js • 3 flaky tests • 5x-driver-firefox

View Output Video

Test Artifacts
... > correctly returns currentRetry Output
... > correctly returns currentRetry Output
... > correctly returns currentRetry Output
Flakiness  cypress/cypress.cy.js • 3 flaky tests • 5x-driver-electron

View Output Video

Test Artifacts
... > correctly returns currentRetry Test Replay Output
... > correctly returns currentRetry Test Replay Output
... > correctly returns currentRetry Test Replay Output
Flakiness  e2e/origin/snapshots.cy.ts • 1 flaky test • 5x-driver-electron

View Output Video

Test Artifacts
cy.origin - snapshots > e2e log verification > Does not take snapshots of XHR/fetch requests from secondary origin if the wrong origin is visited / origin mismatch, but instead the primary origin (existing behavior) Test Replay Output
Flakiness  scaffold-component-testing.cy.ts • 1 flaky test • launchpad-e2e

View Output Video

Test Artifacts
scaffolding component testing > react-vite-ts-unconfigured > scaffolds component testing for React and Vite Test Replay Output Screenshots
Flakiness  project-setup.cy.ts • 4 flaky tests • launchpad-e2e

View Output Video

Test Artifacts
... > can reconfigure config after CT has been set up Test Replay Output Screenshots
... > skips the setup steps when choosing component tests to run Test Replay Output Screenshots
Launchpad: Setup Project > openLink > opens docs link in the default browser Test Replay Output Screenshots
Launchpad: Setup Project > switch testing types > takes the user to first step of e2e setup when switching from app Test Replay Output Screenshots

The first 5 flaky specs are shown, see all 20 specs in Cypress Cloud.

Review all test suite changes for PR #28094 ↗︎

protocolManager: this.protocolManager,
}

const onError = (error: Error): Promise<void> => {
const pendingRequest = ctx.pendingRequest as PendingRequest | undefined
Copy link
Contributor

Choose a reason for hiding this comment

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

This coercion is needed because ctx is HttpMiddlewareCtx<any>, which causes ctx to be any.

Removing this templated any results in a couple of typescript errors that may be indicative of bugs; one of which is in deferSourceMapRewrite which should be returning a string instead of void (unless the fn definition on L#320 is more correct than the type definition on L#68). Can resolving the ambiguous definition of ctx be rolled into this PR, or is that out of scope?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think at this point given I want to try and get this in for tomorrow, that I'll call this out of scope for now. It's a good find though. This can definitely be cleaned up at some point.

packages/proxy/lib/http/util/prerequests.ts Show resolved Hide resolved
cli/CHANGELOG.md Outdated Show resolved Hide resolved
@ryanthemanuel ryanthemanuel merged commit d960686 into develop Oct 23, 2023
76 of 80 checks passed
@ryanthemanuel ryanthemanuel deleted the ryanm/fix/proxy-fun branch October 23, 2023 21:08
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Oct 24, 2023

Released in 13.3.3.

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

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

None yet

3 participants