Skip to content

Commit 530421d

Browse files
authored
[backport] Fix/dedupe fetch clone (#73532)
<!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: ## For Contributors ### Improving Documentation - Run `pnpm prettier-fix` to fix formatting issues before opening the PR. - Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide ### Adding or Updating Examples - The "examples guidelines" are followed from our contributing doc https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md - Make sure the linting passes by running `pnpm build && pnpm lint`. See https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md ### Fixing a bug - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ### Adding a feature - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ## For Maintainers - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change ### What? ### Why? ### How? Closes NEXT- Fixes # --> This is a backport of #73274 which resolves a bug with response cloning.
1 parent cbc62ad commit 530421d

File tree

3 files changed

+222
-17
lines changed

3 files changed

+222
-17
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/**
2+
* Clones a response by teeing the body so we can return two independent
3+
* ReadableStreams from it. This avoids the bug in the undici library around
4+
* response cloning.
5+
*
6+
* After cloning, the original response's body will be consumed and closed.
7+
*
8+
* @see https://github.com/vercel/next.js/pull/73274
9+
*
10+
* @param original - The original response to clone.
11+
* @returns A tuple containing two independent clones of the original response.
12+
*/
13+
export function cloneResponse(original: Response): [Response, Response] {
14+
// If the response has no body, then we can just return the original response
15+
// twice because it's immutable.
16+
if (!original.body) {
17+
return [original, original]
18+
}
19+
20+
const [body1, body2] = original.body.tee()
21+
22+
const cloned1 = new Response(body1, {
23+
status: original.status,
24+
statusText: original.statusText,
25+
headers: original.headers,
26+
})
27+
28+
Object.defineProperty(cloned1, 'url', {
29+
value: original.url,
30+
})
31+
32+
const cloned2 = new Response(body2, {
33+
status: original.status,
34+
statusText: original.statusText,
35+
headers: original.headers,
36+
})
37+
38+
Object.defineProperty(cloned2, 'url', {
39+
value: original.url,
40+
})
41+
42+
return [cloned1, cloned2]
43+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
/**
2+
* Based on https://github.com/facebook/react/blob/d4e78c42a94be027b4dc7ed2659a5fddfbf9bd4e/packages/react/src/ReactFetch.js
3+
*/
4+
import * as React from 'react'
5+
import { cloneResponse } from './clone-response'
6+
7+
const simpleCacheKey = '["GET",[],null,"follow",null,null,null,null]' // generateCacheKey(new Request('https://blank'));
8+
9+
function generateCacheKey(request: Request): string {
10+
// We pick the fields that goes into the key used to dedupe requests.
11+
// We don't include the `cache` field, because we end up using whatever
12+
// caching resulted from the first request.
13+
// Notably we currently don't consider non-standard (or future) options.
14+
// This might not be safe. TODO: warn for non-standard extensions differing.
15+
// IF YOU CHANGE THIS UPDATE THE simpleCacheKey ABOVE.
16+
return JSON.stringify([
17+
request.method,
18+
Array.from(request.headers.entries()),
19+
request.mode,
20+
request.redirect,
21+
request.credentials,
22+
request.referrer,
23+
request.referrerPolicy,
24+
request.integrity,
25+
])
26+
}
27+
28+
type CacheEntry = [
29+
key: string,
30+
promise: Promise<Response>,
31+
response: Response | null
32+
]
33+
34+
export function createDedupeFetch(originalFetch: typeof fetch) {
35+
const getCacheEntries = React.cache(
36+
// eslint-disable-next-line @typescript-eslint/no-unused-vars -- url is the cache key
37+
(url: string): CacheEntry[] => []
38+
)
39+
40+
return function dedupeFetch(
41+
resource: URL | RequestInfo,
42+
options?: RequestInit
43+
): Promise<Response> {
44+
if (options && options.signal) {
45+
// If we're passed a signal, then we assume that
46+
// someone else controls the lifetime of this object and opts out of
47+
// caching. It's effectively the opt-out mechanism.
48+
// Ideally we should be able to check this on the Request but
49+
// it always gets initialized with its own signal so we don't
50+
// know if it's supposed to override - unless we also override the
51+
// Request constructor.
52+
return originalFetch(resource, options)
53+
}
54+
// Normalize the Request
55+
let url: string
56+
let cacheKey: string
57+
if (typeof resource === 'string' && !options) {
58+
// Fast path.
59+
cacheKey = simpleCacheKey
60+
url = resource
61+
} else {
62+
// Normalize the request.
63+
// if resource is not a string or a URL (its an instance of Request)
64+
// then do not instantiate a new Request but instead
65+
// reuse the request as to not disturb the body in the event it's a ReadableStream.
66+
const request =
67+
typeof resource === 'string' || resource instanceof URL
68+
? new Request(resource, options)
69+
: resource
70+
if (
71+
(request.method !== 'GET' && request.method !== 'HEAD') ||
72+
request.keepalive
73+
) {
74+
// We currently don't dedupe requests that might have side-effects. Those
75+
// have to be explicitly cached. We assume that the request doesn't have a
76+
// body if it's GET or HEAD.
77+
// keepalive gets treated the same as if you passed a custom cache signal.
78+
return originalFetch(resource, options)
79+
}
80+
cacheKey = generateCacheKey(request)
81+
url = request.url
82+
}
83+
84+
const cacheEntries = getCacheEntries(url)
85+
for (let i = 0, j = cacheEntries.length; i < j; i += 1) {
86+
const [key, promise] = cacheEntries[i]
87+
if (key === cacheKey) {
88+
return promise.then(() => {
89+
const response = cacheEntries[i][2]
90+
if (!response) throw new Error('No cached response')
91+
92+
// We're cloning the response using this utility because there exists
93+
// a bug in the undici library around response cloning. See the
94+
// following pull request for more details:
95+
// https://github.com/vercel/next.js/pull/73274
96+
const [cloned1, cloned2] = cloneResponse(response)
97+
cacheEntries[i][2] = cloned2
98+
return cloned1
99+
})
100+
}
101+
}
102+
103+
// We pass the original arguments here in case normalizing the Request
104+
// doesn't include all the options in this environment. We also pass a
105+
// signal down to the original fetch as to bypass the underlying React fetch
106+
// cache.
107+
const controller = new AbortController()
108+
const promise = originalFetch(resource, {
109+
...options,
110+
signal: controller.signal,
111+
})
112+
const entry: CacheEntry = [cacheKey, promise, null]
113+
cacheEntries.push(entry)
114+
115+
return promise.then((response) => {
116+
// We're cloning the response using this utility because there exists
117+
// a bug in the undici library around response cloning. See the
118+
// following pull request for more details:
119+
// https://github.com/vercel/next.js/pull/73274
120+
const [cloned1, cloned2] = cloneResponse(response)
121+
entry[2] = cloned2
122+
return cloned1
123+
})
124+
}
125+
}

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

+54-17
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import {
1515
import * as Log from '../../build/output/log'
1616
import { trackDynamicFetch } from '../app-render/dynamic-rendering'
1717
import type { FetchMetric } from '../base-http'
18+
import { createDedupeFetch } from './dedupe-fetch'
19+
import { cloneResponse } from './clone-response'
1820

1921
const isEdgeRuntime = process.env.NEXT_RUNTIME === 'edge'
2022

@@ -623,15 +625,26 @@ function createPatchedFetcher(
623625
if (entry.isStale) {
624626
staticGenerationStore.pendingRevalidates ??= {}
625627
if (!staticGenerationStore.pendingRevalidates[cacheKey]) {
628+
const pendingRevalidate = doOriginalFetch(true)
629+
.then(async (response) => ({
630+
body: await response.arrayBuffer(),
631+
headers: response.headers,
632+
status: response.status,
633+
statusText: response.statusText,
634+
}))
635+
.finally(() => {
636+
staticGenerationStore.pendingRevalidates ??= {}
637+
delete staticGenerationStore.pendingRevalidates[
638+
cacheKey || ''
639+
]
640+
})
641+
642+
// Attach the empty catch here so we don't get a "unhandled
643+
// promise rejection" warning.
644+
pendingRevalidate.catch(console.error)
645+
626646
staticGenerationStore.pendingRevalidates[cacheKey] =
627-
doOriginalFetch(true)
628-
.catch(console.error)
629-
.finally(() => {
630-
staticGenerationStore.pendingRevalidates ??= {}
631-
delete staticGenerationStore.pendingRevalidates[
632-
cacheKey || ''
633-
]
634-
})
647+
pendingRevalidate
635648
}
636649
}
637650
const resData = entry.value.data
@@ -730,16 +743,40 @@ function createPatchedFetcher(
730743
// origin hit if it's a cache-able entry
731744
if (cacheKey && isForegroundRevalidate) {
732745
staticGenerationStore.pendingRevalidates ??= {}
733-
const pendingRevalidate =
746+
let pendingRevalidate =
734747
staticGenerationStore.pendingRevalidates[cacheKey]
735748

736749
if (pendingRevalidate) {
737-
const res: Response = await pendingRevalidate
738-
return res.clone()
750+
const revalidatedResult: {
751+
body: ArrayBuffer
752+
headers: Headers
753+
status: number
754+
statusText: string
755+
} = await pendingRevalidate
756+
return new Response(revalidatedResult.body, {
757+
headers: revalidatedResult.headers,
758+
status: revalidatedResult.status,
759+
statusText: revalidatedResult.statusText,
760+
})
739761
}
762+
740763
const pendingResponse = doOriginalFetch(true, cacheReasonOverride)
741-
const nextRevalidate = pendingResponse
742-
.then((res) => res.clone())
764+
// We're cloning the response using this utility because there
765+
// exists a bug in the undici library around response cloning.
766+
// See the following pull request for more details:
767+
// https://github.com/vercel/next.js/pull/73274
768+
.then(cloneResponse)
769+
770+
pendingRevalidate = pendingResponse
771+
.then(async (responses) => {
772+
const response = responses[0]
773+
return {
774+
body: await response.arrayBuffer(),
775+
headers: response.headers,
776+
status: response.status,
777+
statusText: response.statusText,
778+
}
779+
})
743780
.finally(() => {
744781
if (cacheKey) {
745782
// If the pending revalidate is not present in the store, then
@@ -754,11 +791,11 @@ function createPatchedFetcher(
754791

755792
// Attach the empty catch here so we don't get a "unhandled promise
756793
// rejection" warning
757-
nextRevalidate.catch(() => {})
794+
pendingRevalidate.catch(() => {})
758795

759-
staticGenerationStore.pendingRevalidates[cacheKey] = nextRevalidate
796+
staticGenerationStore.pendingRevalidates[cacheKey] = pendingRevalidate
760797

761-
return pendingResponse
798+
return pendingResponse.then((responses) => responses[1])
762799
} else {
763800
return doOriginalFetch(false, cacheReasonOverride).finally(
764801
handleUnlock
@@ -784,7 +821,7 @@ export function patchFetch(options: PatchableModule) {
784821

785822
// Grab the original fetch function. We'll attach this so we can use it in
786823
// the patched fetch function.
787-
const original = globalThis.fetch
824+
const original = createDedupeFetch(globalThis.fetch)
788825

789826
// Set the global fetch to the patched fetch.
790827
globalThis.fetch = createPatchedFetcher(original, options)

0 commit comments

Comments
 (0)