fix: fixed dedupe response cloning

Author: wyattjoh

packages/next/src/server/lib/clone-response.ts

@@ -0,0 +1,31 @@
+export function cloneResponse(original: Response): [Response, Response] {
+  // If the response has no body, then we can just return the original response
+  // twice because it's immutable.
+  if (!original.body) {
+    return [original, original]
+  }
+
+  const [body1, body2] = original.body.tee()
+
+  const cloned1 = new Response(body1, {
+    status: original.status,
+    statusText: original.statusText,
+    headers: original.headers,
+  })
+
+  Object.defineProperty(cloned1, 'url', {
+    value: original.url,
+  })
+
+  const cloned2 = new Response(body2, {
+    status: original.status,
+    statusText: original.statusText,
+    headers: original.headers,
+  })
+
+  Object.defineProperty(cloned2, 'url', {
+    value: original.url,
+  })
+
+  return [cloned1, cloned2]
+}

packages/next/src/server/lib/dedupe-fetch.ts

@@ -2,6 +2,8 @@
  * Based on https://github.com/facebook/react/blob/d4e78c42a94be027b4dc7ed2659a5fddfbf9bd4e/packages/react/src/ReactFetch.js
  */
 import * as React from 'react'
+import { cloneResponse } from './clone-response'
+import { InvariantError } from '../../shared/lib/invariant-error'
 
 const simpleCacheKey = '["GET",[],null,"follow",null,null,null,null]' // generateCacheKey(new Request('https://blank'));
 
@@ -24,14 +26,22 @@ function generateCacheKey(request: Request): string {
   ])
 }
 
+type CacheEntry = [
+  key: string,
+  promise: Promise<Response>,
+  response: Response | null,
+]
+
 export function createDedupeFetch(originalFetch: typeof fetch) {
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars -- url is the cache key
-  const getCacheEntries = React.cache((url: string): Array<any> => [])
+  const getCacheEntries = React.cache(
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars -- url is the cache key
+    (url: string): CacheEntry[] => []
+  )
 
   return function dedupeFetch(
     resource: URL | RequestInfo,
     options?: RequestInit
-  ) {
+  ): Promise<Response> {
     if (options && options.signal) {
       // If we're passed a signal, then we assume that
       // someone else controls the lifetime of this object and opts out of
@@ -60,7 +70,6 @@ export function createDedupeFetch(originalFetch: typeof fetch) {
           : resource
       if (
         (request.method !== 'GET' && request.method !== 'HEAD') ||
-        // $FlowFixMe[prop-missing]: keepalive is real
         request.keepalive
       ) {
         // We currently don't dedupe requests that might have side-effects. Those
@@ -74,29 +83,34 @@ export function createDedupeFetch(originalFetch: typeof fetch) {
     }
 
     const cacheEntries = getCacheEntries(url)
-    let match
-    if (cacheEntries.length === 0) {
-      // We pass the original arguments here in case normalizing the Request
-      // doesn't include all the options in this environment.
-      match = originalFetch(resource, options)
-      cacheEntries.push(cacheKey, match)
-    } else {
-      // We use an array as the inner data structure since it's lighter and
-      // we typically only expect to see one or two entries here.
-      for (let i = 0, l = cacheEntries.length; i < l; i += 2) {
-        const key = cacheEntries[i]
-        const value = cacheEntries[i + 1]
-        if (key === cacheKey) {
-          match = value
-          // I would've preferred a labelled break but lint says no.
-          return match.then((response: Response) => response.clone())
-        }
+    for (let i = 0, j = cacheEntries.length; i < j; i += 1) {
+      const [key, promise] = cacheEntries[i]
+      if (key === cacheKey) {
+        return promise.then(() => {
+          const response = cacheEntries[i][2]
+          if (!response) throw new InvariantError('No cached response')
+
+          const [cloned1, cloned2] = cloneResponse(response)
+          cacheEntries[i][2] = cloned2
+          return cloned1
+        })
       }
-      match = originalFetch(resource, options)
-      cacheEntries.push(cacheKey, match)
     }
-    // We clone the response so that each time you call this you get a new read
-    // of the body so that it can be read multiple times.
-    return match.then((response) => response.clone())
+
+    // We pass the original arguments here in case normalizing the Request
+    // doesn't include all the options in this environment.
+    const promise = originalFetch(resource, options)
+    const entry: CacheEntry = [cacheKey, promise, null]
+    cacheEntries.push(entry)
+
+    // Attach an empty catch here so we don't get a "unhandled promise
+    // rejection" warning
+    promise.catch(() => {})
+
+    return promise.then((response) => {
+      const [cloned1, cloned2] = cloneResponse(response)
+      entry[2] = cloned2
+      return cloned1
+    })
   }
 }

packages/next/src/server/lib/patch-fetch.ts

@@ -25,6 +25,7 @@ import {
   type CachedFetchData,
 } from '../response-cache'
 import { waitAtLeastOneReactRenderTask } from '../../lib/scheduler'
+import { cloneResponse } from './clone-response'
 
 const isEdgeRuntime = process.env.NEXT_RUNTIME === 'edge'
 
@@ -676,20 +677,21 @@ export function createPatchedFetcher(
                   statusText: res.statusText,
                 })
               } else {
+                const [cloned1, cloned2] = cloneResponse(res)
+
                 // We are dynamically rendering including dev mode. We want to return
                 // the response to the caller as soon as possible because it might stream
                 // over a very long time.
-                res
-                  .clone()
+                cloned1
                   .arrayBuffer()
                   .then(async (arrayBuffer) => {
                     const bodyBuffer = Buffer.from(arrayBuffer)
 
                     const fetchedData = {
-                      headers: Object.fromEntries(res.headers.entries()),
+                      headers: Object.fromEntries(cloned1.headers.entries()),
                       body: bodyBuffer.toString('base64'),
-                      status: res.status,
-                      url: res.url,
+                      status: cloned1.status,
+                      url: cloned1.url,
                     }
 
                     requestStore?.serverComponentsHmrCache?.set(
@@ -720,7 +722,7 @@ export function createPatchedFetcher(
                   )
                   .finally(handleUnlock)
 
-                return res
+                return cloned2
               }
             }
 
@@ -788,14 +790,23 @@ export function createPatchedFetcher(
                 if (entry.isStale) {
                   workStore.pendingRevalidates ??= {}
                   if (!workStore.pendingRevalidates[cacheKey]) {
-                    workStore.pendingRevalidates[cacheKey] = doOriginalFetch(
-                      true
-                    )
-                      .catch(console.error)
+                    const pendingRevalidate = doOriginalFetch(true)
+                      .then(async (response) => ({
+                        body: await response.arrayBuffer(),
+                        headers: response.headers,
+                        status: response.status,
+                        statusText: response.statusText,
+                      }))
                       .finally(() => {
                         workStore.pendingRevalidates ??= {}
                         delete workStore.pendingRevalidates[cacheKey || '']
                       })
+
+                    // Attach the empty catch here so we don't get a "unhandled
+                    // promise rejection" warning.
+                    pendingRevalidate.catch(console.error)
+
+                    workStore.pendingRevalidates[cacheKey] = pendingRevalidate
                   }
                 }
 
@@ -895,7 +906,7 @@ export function createPatchedFetcher(
         if (cacheKey && isForegroundRevalidate) {
           const pendingRevalidateKey = cacheKey
           workStore.pendingRevalidates ??= {}
-          const pendingRevalidate =
+          let pendingRevalidate =
             workStore.pendingRevalidates[pendingRevalidateKey]
 
           if (pendingRevalidate) {
@@ -920,21 +931,19 @@ export function createPatchedFetcher(
           // available we construct manually cloned Response objects with the
           // body as an ArrayBuffer. This will be resolvable in a microtask
           // making it compatible with dynamicIO.
-          const pendingResponse = doOriginalFetch(true, cacheReasonOverride)
-
-          const nextRevalidate = pendingResponse
-            .then(async (response) => {
-              // Clone the response here. It'll run first because we attached
-              // the resolve before we returned below. We have to clone it
-              // because the original response is going to be consumed by
-              // at a later point in time.
-              const clonedResponse = response.clone()
-
+          const pendingResponse = doOriginalFetch(
+            true,
+            cacheReasonOverride
+          ).then(cloneResponse)
+
+          pendingRevalidate = pendingResponse
+            .then(async (responses) => {
+              const response = responses[0]
               return {
-                body: await clonedResponse.arrayBuffer(),
-                headers: clonedResponse.headers,
-                status: clonedResponse.status,
-                statusText: clonedResponse.statusText,
+                body: await response.arrayBuffer(),
+                headers: response.headers,
+                status: response.status,
+                statusText: response.statusText,
               }
             })
             .finally(() => {
@@ -949,11 +958,11 @@ export function createPatchedFetcher(
 
           // Attach the empty catch here so we don't get a "unhandled promise
           // rejection" warning
-          nextRevalidate.catch(() => {})
+          pendingRevalidate.catch(() => {})
 
-          workStore.pendingRevalidates[pendingRevalidateKey] = nextRevalidate
+          workStore.pendingRevalidates[pendingRevalidateKey] = pendingRevalidate
 
-          return pendingResponse
+          return pendingResponse.then((responses) => responses[1])
         } else {
           return doOriginalFetch(false, cacheReasonOverride)
         }