Skip to content

fix: proxy spec flake in firefox due to improper sweeping and correlation #31651

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ _Released 6/17/2025 (PENDING)_
**Bugfixes:**

- Fixed an issue when using `Cypress.stop()` where a run may be aborted prior to receiving the required runner events causing Test Replay to not be available. Addresses [#31781](https://github.com/cypress-io/cypress/issues/31781).
- Fixed an issue where prerequests with Firefox BiDi were prematurely being removed or matched incorrectly. Addresses [#31482](https://github.com/cypress-io/cypress/issues/31482).

## 14.4.1

Expand Down
2 changes: 1 addition & 1 deletion packages/proxy/lib/http/util/prerequests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ export class PreRequests {
const now = Date.now()

this.pendingPreRequests.removeMatching(({ cdpRequestWillBeSentReceivedTimestamp, browserPreRequest }) => {
if (cdpRequestWillBeSentReceivedTimestamp + this.sweepInterval < now) {
if (cdpRequestWillBeSentReceivedTimestamp !== 0 && (cdpRequestWillBeSentReceivedTimestamp + this.sweepInterval < now)) {
debugVerbose('timed out unmatched pre-request: %o', browserPreRequest)
metrics.unmatchedPreRequests++

Expand Down
16 changes: 16 additions & 0 deletions packages/proxy/test/unit/http/util/prerequests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -309,4 +309,20 @@ describe('http/util/prerequests', () => {
expect(preRequests.pendingPreRequests.length).to.eq(1)
expect(preRequests.pendingPreRequests.shift('GET-foo|bar')).not.to.be.undefined
})

it('does not remove pre-requests when sweeping if cdpRequestWillBeSentReceivedTimestamp is 0', async () => {
// set the current time to 1000 ms
sinon.stub(Date, 'now').returns(1000)

// set a sweeper timer of 10 ms
preRequests = new PreRequests(10, 10)
preRequests.setProtocolManager(protocolManager)

preRequests.addPending({ requestId: '1234', url: 'foo', method: 'GET', cdpRequestWillBeSentReceivedTimestamp: 0 } as BrowserPreRequest)

// give the sweeper plenty of time to run. Iur request should still not be removed
await new Promise((resolve) => setTimeout(resolve, 1000))

expect(preRequests.pendingPreRequests.length).to.eq(1)
})
})
5 changes: 4 additions & 1 deletion packages/server/lib/browsers/bidi_automation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,13 @@ export class BidiAutomation {

const resourceType = normalizeResourceType(params.request.initiatorType)

const urlWithoutHash = url.substring(0, url.indexOf('#')) || url

const browserPreRequest: BrowserPreRequest = {
requestId: params.request.request,
method: params.request.method,
url,
// urls coming into the http middleware contain query params, but lack the hash. To get an accurate key to match on the prerequest, we need to remove the hash.
url: urlWithoutHash,
headers: parsedHeaders,
resourceType,
originalResourceType: params.request.initiatorType || params.request.destination,
Expand Down
2 changes: 1 addition & 1 deletion packages/server/lib/project-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ export class ProjectBase extends EE {

if (this.browser.family !== 'chromium') {
// If we're not in chromium, our strategy for correlating service worker prerequests doesn't work in non-chromium browsers (https://github.com/cypress-io/cypress/issues/28079)
// in order to not hang for 2 seconds, we override the prerequest timeout to be 500 ms (which is what it has been historically)
// in order to not hang for 2 seconds, we override the prerequest timeout to be 500 ms (which is what it has been historically).
this._server?.setPreRequestTimeout(500)
}
}
Expand Down
26 changes: 26 additions & 0 deletions packages/server/test/unit/browsers/bidi_automation_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,32 @@ describe('lib/browsers/bidi_automation', () => {

expect(mockAutomationClient.onRemoveBrowserPreRequest).to.have.been.calledWith('request1')
})

it('strips hashes out of the url when adding the prerequest', async () => {
BidiAutomation.create(mockWebdriverClient, mockAutomationClient)

mockRequest.request.url = 'https://www.foobar.com?foo=bar#hash'

mockWebdriverClient.emit('network.beforeRequestSent', mockRequest)

await flushPromises()

expect(mockAutomationClient.onBrowserPreRequest).to.have.been.calledWith({
requestId: 'request1',
method: 'GET',
url: 'https://www.foobar.com?foo=bar',
resourceType: 'xhr',
originalResourceType: 'xmlhttprequest',
initiator: {
type: 'preflight',
},
headers: {
foo: 'bar',
},
cdpRequestWillBeSentTimestamp: 0,
cdpRequestWillBeSentReceivedTimestamp: 0,
})
})
})

describe('responseStarted / responseCompleted', () => {
Expand Down
Loading