fix: proxy issues with service workers and clean up at end of specs (#…

…28060)

cli/CHANGELOG.md

@@ -5,6 +5,9 @@ _Released 10/25/2023 (PENDING)_
 
 **Bugfixes:**
 
+- Fixed a performance regression in `13.3.1` with proxy correlation timeouts and requests issued from service workers. Fixes [#28054](https://github.com/cypress-io/cypress/issues/28054) and [#28056](https://github.com/cypress-io/cypress/issues/28056).
+- Fixed an issue where proxy correlation would leak over from a previous spec causing performance problems, `cy.intercept` problems, and Test Replay asset capturing issues. Addressed in [#28060](https://github.com/cypress-io/cypress/pull/28060).
+- Fixed an issue where redirects of requests that knowingly don't have CDP traffic should also be assumed to not have CDP traffic. Addressed in [#28060](https://github.com/cypress-io/cypress/pull/28060).
 - Fixed an issue with Accept Encoding headers by forcing gzip when no accept encoding header is sent and using identity if gzip is not sent. Fixes [#28025](https://github.com/cypress-io/cypress/issues/28025).
 
 **Dependency Updates:**

packages/proxy/lib/http/index.ts

@@ -66,6 +66,7 @@ type HttpMiddlewareCtx<T> = {
   getCookieJar: () => CookieJar
   deferSourceMapRewrite: (opts: { js: string, url: string }) => string
   getPreRequest: (cb: GetPreRequestCb) => void
+  addPendingUrlWithoutPreRequest: (url: string) => void
   getAUTUrl: Http['getAUTUrl']
   setAUTUrl: Http['setAUTUrl']
   simulatedCookies: SerializableAutomationCookie[]
@@ -326,6 +327,9 @@ export class Http {
       getPreRequest: (cb) => {
         this.preRequests.get(ctx.req, ctx.debug, cb)
       },
+      addPendingUrlWithoutPreRequest: (url) => {
+        this.preRequests.addPendingUrlWithoutPreRequest(url)
+      },
       protocolManager: this.protocolManager,
     }
 
@@ -411,6 +415,7 @@ export class Http {
   reset () {
     this.buffers.reset()
     this.setAUTUrl(undefined)
+    this.preRequests.reset()
   }
 
   setBuffer (buffer) {
@@ -433,4 +438,8 @@ export class Http {
     this.protocolManager = protocolManager
     this.preRequests.setProtocolManager(protocolManager)
   }
+
+  setPreRequestTimeout (timeout: number) {
+    this.preRequests.setPreRequestTimeout(timeout)
+  }
 }

packages/proxy/lib/http/request-middleware.ts

@@ -128,8 +128,9 @@ const CorrelateBrowserPreRequest: RequestMiddleware = async function () {
   }
 
   this.debug('waiting for prerequest')
-  this.getPreRequest(((browserPreRequest) => {
+  this.getPreRequest((({ browserPreRequest, noPreRequestExpected }) => {
     this.req.browserPreRequest = browserPreRequest
+    this.req.noPreRequestExpected = noPreRequestExpected
     copyResourceTypeAndNext()
   }))
 }

packages/proxy/lib/http/response-middleware.ts

@@ -656,6 +656,11 @@ const MaybeSendRedirectToClient: ResponseMiddleware = function () {
     return this.next()
   }
 
+  // If we're redirecting from a request that doesn't expect to have a preRequest (e.g. download links), we need to treat the redirected url as such as well.
+  if (this.req.noPreRequestExpected) {
+    this.addPendingUrlWithoutPreRequest(newUrl)
+  }
+
   setInitialCookie(this.res, this.remoteStates.current(), true)
 
   this.debug('redirecting to new url %o', { statusCode, newUrl })

packages/proxy/lib/http/util/prerequests.ts

@@ -21,11 +21,16 @@ process.once('exit', () => {
   debug('metrics: %o', metrics)
 })
 
-export type GetPreRequestCb = (browserPreRequest?: BrowserPreRequestWithTimings) => void
+export type CorrelationInformation = {
+  browserPreRequest?: BrowserPreRequestWithTimings
+  noPreRequestExpected?: boolean
+}
+
+export type GetPreRequestCb = (correlationInformation: CorrelationInformation) => void
 
 type PendingRequest = {
   ctxDebug
-  callback: GetPreRequestCb
+  callback?: GetPreRequestCb
   timeout: NodeJS.Timeout
   timedOut?: boolean
   proxyRequestReceivedTimestamp: number
@@ -74,6 +79,13 @@ class QueueMap<T> {
     this.queues[queueKey].splice(i, 1)
     if (this.queues[queueKey].length === 0) delete this.queues[queueKey]
   }
+
+  forEach (fn: (value: T) => void) {
+    Object.values(this.queues).forEach((queue) => {
+      queue.forEach(fn)
+    })
+  }
+
   get length () {
     return Object.values(this.queues).reduce((prev, cur) => prev + cur.length, 0)
   }
@@ -148,11 +160,16 @@ export class PreRequests {
       debugVerbose('Incoming pre-request %s matches pending request. %o', key, browserPreRequest)
       if (!pendingRequest.timedOut) {
         clearTimeout(pendingRequest.timeout)
-        pendingRequest.callback({
-          ...browserPreRequest,
-          ...timings,
+        pendingRequest.callback?.({
+          browserPreRequest: {
+            ...browserPreRequest,
+            ...timings,
+          },
+          noPreRequestExpected: false,
         })
 
+        delete pendingRequest.callback
+
         return
       }
 
@@ -179,7 +196,11 @@ export class PreRequests {
     if (pendingRequest) {
       debugVerbose('Handling %s without a CDP prerequest', key)
       clearTimeout(pendingRequest.timeout)
-      pendingRequest.callback()
+      pendingRequest.callback?.({
+        noPreRequestExpected: true,
+      })
+
+      delete pendingRequest.callback
 
       return
     }
@@ -196,6 +217,19 @@ export class PreRequests {
   }
 
   get (req: CypressIncomingRequest, ctxDebug, callback: GetPreRequestCb) {
+    // The initial request that loads the service worker does not get sent to CDP and it happens prior
+    // to the service worker target being added. Thus, we need to explicitly ignore it. We determine
+    // it's the service worker request via the `sec-fetch-dest` header
+    if (req.headers['sec-fetch-dest'] === 'serviceworker') {
+      ctxDebug('Ignoring request with sec-fetch-dest: serviceworker', req.proxiedUrl)
+
+      callback({
+        noPreRequestExpected: true,
+      })
+
+      return
+    }
+
     const proxyRequestReceivedTimestamp = performance.now() + performance.timeOrigin
 
     metrics.proxyRequestsReceived++
@@ -206,12 +240,15 @@ export class PreRequests {
       metrics.immediatelyMatchedRequests++
       ctxDebug('Incoming request %s matches known pre-request: %o', key, pendingPreRequest)
       callback({
-        ...pendingPreRequest.browserPreRequest,
-        cdpRequestWillBeSentTimestamp: pendingPreRequest.cdpRequestWillBeSentTimestamp,
-        cdpRequestWillBeSentReceivedTimestamp: pendingPreRequest.cdpRequestWillBeSentReceivedTimestamp,
-        proxyRequestReceivedTimestamp,
-        cdpLagDuration: pendingPreRequest.cdpRequestWillBeSentReceivedTimestamp - pendingPreRequest.cdpRequestWillBeSentTimestamp,
-        proxyRequestCorrelationDuration: Math.max(pendingPreRequest.cdpRequestWillBeSentReceivedTimestamp - proxyRequestReceivedTimestamp, 0),
+        browserPreRequest: {
+          ...pendingPreRequest.browserPreRequest,
+          cdpRequestWillBeSentTimestamp: pendingPreRequest.cdpRequestWillBeSentTimestamp,
+          cdpRequestWillBeSentReceivedTimestamp: pendingPreRequest.cdpRequestWillBeSentReceivedTimestamp,
+          proxyRequestReceivedTimestamp,
+          cdpLagDuration: pendingPreRequest.cdpRequestWillBeSentReceivedTimestamp - pendingPreRequest.cdpRequestWillBeSentTimestamp,
+          proxyRequestCorrelationDuration: Math.max(pendingPreRequest.cdpRequestWillBeSentReceivedTimestamp - proxyRequestReceivedTimestamp, 0),
+        },
+        noPreRequestExpected: false,
       })
 
       return
@@ -222,7 +259,9 @@ export class PreRequests {
     if (pendingUrlWithoutPreRequests) {
       metrics.immediatelyMatchedRequests++
       ctxDebug('Incoming request %s matches known pending url without pre request', key)
-      callback()
+      callback({
+        noPreRequestExpected: true,
+      })
 
       return
     }
@@ -235,7 +274,11 @@ export class PreRequests {
         ctxDebug('Never received pre-request or url without pre-request for request %s after waiting %sms. Continuing without one.', key, this.requestTimeout)
         metrics.unmatchedRequests++
         pendingRequest.timedOut = true
-        callback()
+        callback({
+          noPreRequestExpected: false,
+        })
+
+        delete pendingRequest.callback
       }, this.requestTimeout),
     }
 
@@ -245,4 +288,24 @@ export class PreRequests {
   setProtocolManager (protocolManager: ProtocolManagerShape) {
     this.protocolManager = protocolManager
   }
+
+  setPreRequestTimeout (requestTimeout: number) {
+    this.requestTimeout = requestTimeout
+  }
+
+  reset () {
+    this.pendingPreRequests = new QueueMap<PendingPreRequest>()
+
+    // Clear out the pending requests timeout callbacks first then clear the queue
+    this.pendingRequests.forEach(({ callback, timeout }) => {
+      clearTimeout(timeout)
+      metrics.unmatchedRequests++
+      callback?.({
+        noPreRequestExpected: false,
+      })
+    })
+
+    this.pendingRequests = new QueueMap<PendingRequest>()
+    this.pendingUrlsWithoutPreRequests = new QueueMap<PendingUrlWithoutPreRequest>()
+  }
 }

packages/proxy/lib/network-proxy.ts

@@ -53,4 +53,8 @@ export class NetworkProxy {
   setProtocolManager (protocolManager) {
     this.http.setProtocolManager(protocolManager)
   }
+
+  setPreRequestTimeout (timeout) {
+    this.http.setPreRequestTimeout(timeout)
+  }
 }

packages/proxy/lib/types.ts

@@ -12,6 +12,7 @@ export type CypressIncomingRequest = Request & {
   abort: () => void
   requestId: string
   browserPreRequest?: BrowserPreRequestWithTimings
+  noPreRequestExpected?: boolean
   body?: string
   responseTimeout?: number
   followRedirect?: boolean