sweep pendingPreRequestsToRemove

cypress-io · Apr 23, 2024 · bc767fd · bc767fd · cypress-bot · Apr 23, 2024
1 parent f3e9fbe
commit bc767fd
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 15 deletions.
diff --git a/packages/proxy/lib/http/util/prerequests.ts b/packages/proxy/lib/http/util/prerequests.ts
@@ -116,7 +116,7 @@ export class PreRequests {
   pendingPreRequests = new QueueMap<PendingPreRequest>()
   pendingRequests = new QueueMap<PendingRequest>()
   pendingUrlsWithoutPreRequests = new QueueMap<PendingUrlWithoutPreRequest>()
-  pendingPreRequestsToRemove: Set<string> = new Set()
+  pendingPreRequestsToRemove: Map<string, number> = new Map()
   sweepIntervalTimer: NodeJS.Timeout
   protocolManager?: ProtocolManagerShape
 
@@ -153,6 +153,12 @@ export class PreRequests {
         return true
       })
 
+      this.pendingPreRequestsToRemove.forEach((timestamp, requestId) => {
+        if (timestamp + this.sweepInterval < now) {
+          this.pendingPreRequestsToRemove.delete(requestId)
+        }
+      })
+
       this.pendingUrlsWithoutPreRequests.removeMatching(({ timestamp }) => {
         return timestamp + this.sweepInterval >= now
       })
@@ -260,7 +266,7 @@ export class PreRequests {
     // the async function to add the pre-request
     if (!removed) {
       debugVerbose('No pre-request found to remove, adding to pending list: %s', requestId)
-      this.pendingPreRequestsToRemove.add(requestId)
+      this.pendingPreRequestsToRemove.set(requestId, Date.now())
     } else {
       debugVerbose('Removed pre-request %s', requestId)
     }
@@ -360,6 +366,6 @@ export class PreRequests {
 
     this.pendingRequests = new QueueMap<PendingRequest>()
     this.pendingUrlsWithoutPreRequests = new QueueMap<PendingUrlWithoutPreRequest>()
-    this.pendingPreRequestsToRemove = new Set<string>()
+    this.pendingPreRequestsToRemove = new Map()
   }
 }
diff --git a/packages/proxy/test/unit/http/util/prerequests.spec.ts b/packages/proxy/test/unit/http/util/prerequests.spec.ts
@@ -189,6 +189,9 @@ describe('http/util/prerequests', () => {
     preRequests = new PreRequests(10, 200)
     preRequests.addPending({ requestId: '1234', url: 'foo', method: 'GET', cdpRequestWillBeSentReceivedTimestamp: performance.now() + performance.timeOrigin } as BrowserPreRequest)
     preRequests.addPendingUrlWithoutPreRequest('bar')
+    preRequests.removePendingPreRequest('12345')
+
+    expectPendingCounts(0, 1, 1, 1)
 
     // preRequests garbage collects pre-requests that never matched up with an incoming request after around
     // 2 * requestTimeout. We verify that it's gone (and therefore not leaking memory) by sending in a request

diff --git a/packages/server/lib/automation/automation.ts b/packages/server/lib/automation/automation.ts
@@ -16,7 +16,6 @@ export type AutomationOptions = {
   onBrowserPreRequest?: OnBrowserPreRequest
   onRequestEvent?: OnRequestEvent
   onRemoveBrowserPreRequest?: (requestId: string) => void
-  onRequestFailed?: (requestId: string) => void
   onDownloadLinkClicked?: (downloadUrl: string) => void
   onServiceWorkerRegistrationUpdated?: OnServiceWorkerRegistrationUpdated
   onServiceWorkerVersionUpdated?: OnServiceWorkerVersionUpdated
@@ -32,7 +31,6 @@ export class Automation {
   public onBrowserPreRequest: OnBrowserPreRequest | undefined
   public onRequestEvent: OnRequestEvent | undefined
   public onRemoveBrowserPreRequest: ((requestId: string) => void) | undefined
-  public onRequestFailed: ((requestId: string) => void) | undefined
   public onDownloadLinkClicked: ((downloadUrl: string) => void) | undefined
   public onServiceWorkerRegistrationUpdated: OnServiceWorkerRegistrationUpdated | undefined
   public onServiceWorkerVersionUpdated: OnServiceWorkerVersionUpdated | undefined
@@ -43,7 +41,6 @@ export class Automation {
     this.onBrowserPreRequest = options.onBrowserPreRequest
     this.onRequestEvent = options.onRequestEvent
     this.onRemoveBrowserPreRequest = options.onRemoveBrowserPreRequest
-    this.onRequestFailed = options.onRequestFailed
     this.onDownloadLinkClicked = options.onDownloadLinkClicked
     this.onServiceWorkerRegistrationUpdated = options.onServiceWorkerRegistrationUpdated
     this.onServiceWorkerVersionUpdated = options.onServiceWorkerVersionUpdated

diff --git a/packages/server/lib/browsers/cdp_automation.ts b/packages/server/lib/browsers/cdp_automation.ts
@@ -309,7 +309,7 @@ export class CdpAutomation implements CDPClient {
   }
 
   private onRequestFailed = (params: Protocol.Network.LoadingFailedEvent) => {
-    this.automation.onRequestFailed?.(params.requestId)
+    this.automation.onRemoveBrowserPreRequest?.(params.requestId)
   }
 
   private onResponseReceived = (params: Protocol.Network.ResponseReceivedEvent) => {

diff --git a/packages/server/lib/project-base.ts b/packages/server/lib/project-base.ts
@@ -336,10 +336,6 @@ export class ProjectBase extends EE {
       this.server.removeBrowserPreRequest(requestId)
     }
 
-    const onRequestFailed = (requestId: string) => {
-      this.server.removeBrowserPreRequest(requestId)
-    }
-
     const onDownloadLinkClicked = (downloadUrl: string) => {
       this.server.addPendingUrlWithoutPreRequest(downloadUrl)
     }
@@ -367,7 +363,6 @@ export class ProjectBase extends EE {
       onBrowserPreRequest,
       onRequestEvent,
       onRemoveBrowserPreRequest,
-      onRequestFailed,
       onDownloadLinkClicked,
       onServiceWorkerRegistrationUpdated,
       onServiceWorkerVersionUpdated,

diff --git a/packages/server/test/unit/browsers/cdp_automation_spec.ts b/packages/server/test/unit/browsers/cdp_automation_spec.ts
@@ -73,7 +73,6 @@ context('lib/browsers/cdp_automation', () => {
         onBrowserPreRequest: sinon.stub(),
         onRequestEvent: sinon.stub(),
         onRemoveBrowserPreRequest: sinon.stub(),
-        onRequestFailed: sinon.stub(),
         onServiceWorkerRegistrationUpdated: sinon.stub(),
         onServiceWorkerVersionUpdated: sinon.stub(),
       }
@@ -286,7 +285,7 @@ context('lib/browsers/cdp_automation', () => {
     })
 
     describe('.onRequestFailed', function () {
-      it('triggers onRequestFailed', function () {
+      it('triggers onRemoveBrowserPreRequest', function () {
         const browserRequestFailed = {
           requestId: '0',
         }
@@ -295,7 +294,7 @@ context('lib/browsers/cdp_automation', () => {
         .withArgs('Network.loadingFailed')
         .yield(browserRequestFailed)
 
-        expect(this.automation.onRequestFailed).to.have.been.calledWith(browserRequestFailed.requestId)
+        expect(this.automation.onRemoveBrowserPreRequest).to.have.been.calledWith(browserRequestFailed.requestId)
       })
     })