Skip to content

Commit

Permalink
fix: change network request sweep interval from 1s -> 10s (#25209)
Browse files Browse the repository at this point in the history
* fix: change sweep interval from 1s -> 10s

* binaries

* update variable name

* use DI to make PreRequest class more testable

* revert code [skip ci]

* try tweaking test

Co-authored-by: Matt Henkes <mjhenkes@gmail.com>
  • Loading branch information
lmiller1990 and mjhenkes authored Dec 21, 2022
1 parent 2cc1c17 commit 1682a3f
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 6 deletions.
16 changes: 11 additions & 5 deletions packages/proxy/lib/http/util/prerequests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,27 +78,33 @@ class StackMap<T> {
// ever comes in, we don't want to block proxied requests indefinitely.
export class PreRequests {
requestTimeout: number
sweepInterval: number
pendingPreRequests = new StackMap<PendingPreRequest>()
pendingRequests = new StackMap<PendingRequest>()
sweepInterval: ReturnType<typeof setInterval>
sweepIntervalTimer: NodeJS.Timeout

constructor (requestTimeout = 500) {
constructor (
requestTimeout = 500,
// 10 seconds
sweepInterval = 1000 * 10,
) {
// If a request comes in and we don't have a matching pre-request after this timeout,
// we invoke the request callback to tell the server to proceed (we don't want to block
// user requests indefinitely).
this.requestTimeout = requestTimeout
this.sweepInterval = sweepInterval

// Discarding prerequests on the other hand is not urgent, so we do it on a regular interval
// rather than with a separate timer for each one.
// 2 times the requestTimeout is arbitrary, chosen to give plenty of time and
// make sure we don't discard any pre-requests prematurely but that we don't leak memory over time
// if a large number of pre-requests don't match up
// fixes: https://github.com/cypress-io/cypress/issues/17853
this.sweepInterval = setInterval(() => {
this.sweepIntervalTimer = setInterval(() => {
const now = Date.now()

this.pendingPreRequests.removeMatching(({ timestamp, browserPreRequest }) => {
if (timestamp + this.requestTimeout * 2 < now) {
if (timestamp + this.sweepInterval < now) {
debugVerbose('timed out unmatched pre-request: %o', browserPreRequest)
metrics.unmatchedPreRequests++

Expand All @@ -107,7 +113,7 @@ export class PreRequests {

return true
})
}, this.requestTimeout * 2)
}, this.sweepInterval)
}

addPending (browserPreRequest: BrowserPreRequest) {
Expand Down
3 changes: 2 additions & 1 deletion packages/proxy/test/unit/http/util/prerequests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ describe('http/util/prerequests', () => {

// https://github.com/cypress-io/cypress/issues/17853
it('eventually discards pre-requests that don\'t match requests', (done) => {
preRequests = new PreRequests(10, 200)
preRequests.addPending({ requestId: '1234', url: 'foo', method: 'GET' } as BrowserPreRequest)

// preRequests garbage collects pre-requests that never matched up with an incoming request after around
Expand All @@ -75,6 +76,6 @@ describe('http/util/prerequests', () => {
}

preRequests.get({ proxiedUrl: 'foo', method: 'GET' } as CypressIncomingRequest, () => {}, cb)
}, 50)
}, 1200)
})
})

0 comments on commit 1682a3f

Please sign in to comment.