Skip to content

Commit 2e48d4f

Browse files
dario-piotrowiczvicb
dario-piotrowicz
and
vicb
authoredFeb 12, 2025··
fix: make sure that fetch cache sets are properly awaited (#364)
Next.js does not await promises that update the incremental cache for fetch requests, that is needed in our runtime otherwise the cache updates get lost, so this change makes sure that the promise is properly awaited via `waitUntil` --------- Co-authored-by: Victor Berchet <victor@suumit.com>
·
@opennextjs/cloudflare@1.0.0-beta.0@opennextjs/cloudflare@0.5.0
1 parent e1845d9 commit 2e48d4f

File tree

6 files changed

+525
-3
lines changed

6 files changed

+525
-3
lines changed
 

‎.changeset/few-ducks-listen.md

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"@opennextjs/cloudflare": patch
3+
---
4+
5+
fix: make sure that fetch cache `set`s are properly awaited
6+
7+
Next.js does not await promises that update the incremental cache for fetch requests,
8+
that is needed in our runtime otherwise the cache updates get lost, so this change
9+
makes sure that the promise is properly awaited via `waitUntil`

‎examples/e2e/app-router/e2e/ssr.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ test.skip("Server Side Render and loading.tsx", async ({ page }) => {
2828
}
2929
});
3030

31-
test.skip("Fetch cache properly cached", async ({ page }) => {
31+
test("Fetch cache properly cached", async ({ page }) => {
3232
await page.goto("/ssr");
3333
const originalDate = await page.getByText("Cached fetch:").textContent();
3434
await page.waitForTimeout(2000);

‎packages/cloudflare/src/cli/build/bundle-server.ts

+2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { patchVercelOgLibrary } from "./patches/ast/patch-vercel-og-library.js";
1010
import { patchWebpackRuntime } from "./patches/ast/webpack-runtime.js";
1111
import * as patches from "./patches/index.js";
1212
import { ContentUpdater } from "./patches/plugins/content-updater.js";
13+
import { patchFetchCacheSetMissingWaitUntil } from "./patches/plugins/fetch-cache-wait-until.js";
1314
import { patchLoadInstrumentation } from "./patches/plugins/load-instrumentation.js";
1415
import { handleOptionalDependencies } from "./patches/plugins/optional-deps.js";
1516
import { fixRequire } from "./patches/plugins/require.js";
@@ -87,6 +88,7 @@ export async function bundleServer(buildOpts: BuildOptions): Promise<void> {
8788
fixRequire(updater),
8889
handleOptionalDependencies(optionalDependencies),
8990
patchLoadInstrumentation(updater),
91+
patchFetchCacheSetMissingWaitUntil(updater),
9092
// Apply updater updaters, must be the last plugin
9193
updater.plugin,
9294
],

‎packages/cloudflare/src/cli/build/patches/plugins/content-updater.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,10 @@ export class ContentUpdater {
6262
if (namespace !== undefined && args.namespace !== namespace) {
6363
continue;
6464
}
65-
if (!filter.test(args.path)) {
65+
if (!args.path.match(filter)) {
6666
continue;
6767
}
68-
if (!contentFilter.test(contents)) {
68+
if (!contents.match(contentFilter)) {
6969
continue;
7070
}
7171
contents = (await callback({ contents, path: args.path })) ?? contents;

‎packages/cloudflare/src/cli/build/patches/plugins/fetch-cache-wait-until.spec.ts

+460
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,460 @@
1+
import { describe, expect, test } from "vitest";
2+
3+
import { patchCode } from "../ast/util.js";
4+
import { rule } from "./fetch-cache-wait-until.js";
5+
6+
describe("patchFetchCacheSetMissingWaitUntil", () => {
7+
test("on minified code", () => {
8+
const code = `
9+
{
10+
let [o4, a2] = (0, d2.cloneResponse)(e3);
11+
return o4.arrayBuffer().then(async (e4) => {
12+
var a3;
13+
let i4 = Buffer.from(e4), s3 = { headers: Object.fromEntries(o4.headers.entries()), body: i4.toString("base64"), status: o4.status, url: o4.url };
14+
null == $ || null == (a3 = $.serverComponentsHmrCache) || a3.set(n2, s3), F && await H.set(n2, { kind: c2.CachedRouteKind.FETCH, data: s3, revalidate: t5 }, { fetchCache: true, revalidate: r4, fetchUrl: _, fetchIdx: q, tags: A2 });
15+
}).catch((e4) => console.warn("Failed to set fetch cache", u4, e4)).finally(X), a2;
16+
}`;
17+
18+
expect(patchCode(code, rule)).toMatchInlineSnapshot(`
19+
"{
20+
let [o4, a2] = (0, d2.cloneResponse)(e3);
21+
return globalThis.__openNextAls?.getStore()?.waitUntil?.(o4.arrayBuffer().then(async (e4) => {
22+
var a3;
23+
let i4 = Buffer.from(e4), s3 = { headers: Object.fromEntries(o4.headers.entries()), body: i4.toString("base64"), status: o4.status, url: o4.url };
24+
null == $ || null == (a3 = $.serverComponentsHmrCache) || a3.set(n2, s3), F && await H.set(n2, { kind: c2.CachedRouteKind.FETCH, data: s3, revalidate: t5 }, { fetchCache: true, revalidate: r4, fetchUrl: _, fetchIdx: q, tags: A2 });
25+
}).catch((e4) => console.warn("Failed to set fetch cache", u4, e4)).finally(X))
26+
, a2;
27+
}"
28+
`);
29+
});
30+
31+
describe("on non-minified code", () => {
32+
test("15.1.0", () => {
33+
// source: https://github.com/vercel/next.js/blob/fe45b74fdac83d3/packages/next/src/server/lib/patch-fetch.ts#L627-L732
34+
const code = `if (
35+
res.status === 200 &&
36+
incrementalCache &&
37+
cacheKey &&
38+
(isCacheableRevalidate ||
39+
useCacheOrRequestStore?.serverComponentsHmrCache)
40+
) {
41+
const normalizedRevalidate =
42+
finalRevalidate >= INFINITE_CACHE
43+
? CACHE_ONE_YEAR
44+
: finalRevalidate
45+
const externalRevalidate =
46+
finalRevalidate >= INFINITE_CACHE ? false : finalRevalidate
47+
48+
if (workUnitStore && workUnitStore.type === 'prerender') {
49+
// We are prerendering at build time or revalidate time with dynamicIO so we need to
50+
// buffer the response so we can guarantee it can be read in a microtask
51+
const bodyBuffer = await res.arrayBuffer()
52+
53+
const fetchedData = {
54+
headers: Object.fromEntries(res.headers.entries()),
55+
body: Buffer.from(bodyBuffer).toString('base64'),
56+
status: res.status,
57+
url: res.url,
58+
}
59+
60+
// We can skip checking the serverComponentsHmrCache because we aren't in
61+
// dev mode.
62+
63+
await incrementalCache.set(
64+
cacheKey,
65+
{
66+
kind: CachedRouteKind.FETCH,
67+
data: fetchedData,
68+
revalidate: normalizedRevalidate,
69+
},
70+
{
71+
fetchCache: true,
72+
revalidate: externalRevalidate,
73+
fetchUrl,
74+
fetchIdx,
75+
tags,
76+
}
77+
)
78+
await handleUnlock()
79+
80+
// We return a new Response to the caller.
81+
return new Response(bodyBuffer, {
82+
headers: res.headers,
83+
status: res.status,
84+
statusText: res.statusText,
85+
})
86+
} else {
87+
// We're cloning the response using this utility because there
88+
// exists a bug in the undici library around response cloning.
89+
// See the following pull request for more details:
90+
// https://github.com/vercel/next.js/pull/73274
91+
92+
const [cloned1, cloned2] = cloneResponse(res)
93+
94+
// We are dynamically rendering including dev mode. We want to return
95+
// the response to the caller as soon as possible because it might stream
96+
// over a very long time.
97+
cloned1
98+
.arrayBuffer()
99+
.then(async (arrayBuffer) => {
100+
const bodyBuffer = Buffer.from(arrayBuffer)
101+
102+
const fetchedData = {
103+
headers: Object.fromEntries(cloned1.headers.entries()),
104+
body: bodyBuffer.toString('base64'),
105+
status: cloned1.status,
106+
url: cloned1.url,
107+
}
108+
109+
useCacheOrRequestStore?.serverComponentsHmrCache?.set(
110+
cacheKey,
111+
fetchedData
112+
)
113+
114+
if (isCacheableRevalidate) {
115+
await incrementalCache.set(
116+
cacheKey,
117+
{
118+
kind: CachedRouteKind.FETCH,
119+
data: fetchedData,
120+
revalidate: normalizedRevalidate,
121+
},
122+
{
123+
fetchCache: true,
124+
revalidate: externalRevalidate,
125+
fetchUrl,
126+
fetchIdx,
127+
tags,
128+
}
129+
)
130+
}
131+
})
132+
.catch((error) =>
133+
console.warn(\`Failed to set fetch cache\`, input, error)
134+
)
135+
.finally(handleUnlock)
136+
137+
return cloned2
138+
}
139+
}
140+
`;
141+
142+
expect(patchCode(code, rule)).toMatchInlineSnapshot(`
143+
"if (
144+
res.status === 200 &&
145+
incrementalCache &&
146+
cacheKey &&
147+
(isCacheableRevalidate ||
148+
useCacheOrRequestStore?.serverComponentsHmrCache)
149+
) {
150+
const normalizedRevalidate =
151+
finalRevalidate >= INFINITE_CACHE
152+
? CACHE_ONE_YEAR
153+
: finalRevalidate
154+
const externalRevalidate =
155+
finalRevalidate >= INFINITE_CACHE ? false : finalRevalidate
156+
157+
if (workUnitStore && workUnitStore.type === 'prerender') {
158+
// We are prerendering at build time or revalidate time with dynamicIO so we need to
159+
// buffer the response so we can guarantee it can be read in a microtask
160+
const bodyBuffer = await res.arrayBuffer()
161+
162+
const fetchedData = {
163+
headers: Object.fromEntries(res.headers.entries()),
164+
body: Buffer.from(bodyBuffer).toString('base64'),
165+
status: res.status,
166+
url: res.url,
167+
}
168+
169+
// We can skip checking the serverComponentsHmrCache because we aren't in
170+
// dev mode.
171+
172+
await incrementalCache.set(
173+
cacheKey,
174+
{
175+
kind: CachedRouteKind.FETCH,
176+
data: fetchedData,
177+
revalidate: normalizedRevalidate,
178+
},
179+
{
180+
fetchCache: true,
181+
revalidate: externalRevalidate,
182+
fetchUrl,
183+
fetchIdx,
184+
tags,
185+
}
186+
)
187+
await handleUnlock()
188+
189+
// We return a new Response to the caller.
190+
return new Response(bodyBuffer, {
191+
headers: res.headers,
192+
status: res.status,
193+
statusText: res.statusText,
194+
})
195+
} else {
196+
// We're cloning the response using this utility because there
197+
// exists a bug in the undici library around response cloning.
198+
// See the following pull request for more details:
199+
// https://github.com/vercel/next.js/pull/73274
200+
201+
const [cloned1, cloned2] = cloneResponse(res)
202+
203+
// We are dynamically rendering including dev mode. We want to return
204+
// the response to the caller as soon as possible because it might stream
205+
// over a very long time.
206+
globalThis.__openNextAls?.getStore()?.waitUntil?.(cloned1
207+
.arrayBuffer()
208+
.then(async (arrayBuffer) => {
209+
const bodyBuffer = Buffer.from(arrayBuffer)
210+
211+
const fetchedData = {
212+
headers: Object.fromEntries(cloned1.headers.entries()),
213+
body: bodyBuffer.toString('base64'),
214+
status: cloned1.status,
215+
url: cloned1.url,
216+
}
217+
218+
useCacheOrRequestStore?.serverComponentsHmrCache?.set(
219+
cacheKey,
220+
fetchedData
221+
)
222+
223+
if (isCacheableRevalidate) {
224+
await incrementalCache.set(
225+
cacheKey,
226+
{
227+
kind: CachedRouteKind.FETCH,
228+
data: fetchedData,
229+
revalidate: normalizedRevalidate,
230+
},
231+
{
232+
fetchCache: true,
233+
revalidate: externalRevalidate,
234+
fetchUrl,
235+
fetchIdx,
236+
tags,
237+
}
238+
)
239+
}
240+
})
241+
.catch((error) =>
242+
console.warn(\`Failed to set fetch cache\`, input, error)
243+
)
244+
.finally(handleUnlock))
245+
246+
247+
return cloned2
248+
}
249+
}
250+
"
251+
`);
252+
});
253+
254+
test("Next.js 15.0.4", () => {
255+
// source: https://github.com/vercel/next.js/blob/d6a6aa14069/packages/next/src/server/lib/patch-fetch.ts#L627-L725
256+
const code = `if (
257+
res.status === 200 &&
258+
incrementalCache &&
259+
cacheKey &&
260+
(isCacheableRevalidate || requestStore?.serverComponentsHmrCache)
261+
) {
262+
const normalizedRevalidate =
263+
finalRevalidate >= INFINITE_CACHE
264+
? CACHE_ONE_YEAR
265+
: finalRevalidate
266+
const externalRevalidate =
267+
finalRevalidate >= INFINITE_CACHE ? false : finalRevalidate
268+
269+
if (workUnitStore && workUnitStore.type === 'prerender') {
270+
// We are prerendering at build time or revalidate time with dynamicIO so we need to
271+
// buffer the response so we can guarantee it can be read in a microtask
272+
const bodyBuffer = await res.arrayBuffer()
273+
274+
const fetchedData = {
275+
headers: Object.fromEntries(res.headers.entries()),
276+
body: Buffer.from(bodyBuffer).toString('base64'),
277+
status: res.status,
278+
url: res.url,
279+
}
280+
281+
// We can skip checking the serverComponentsHmrCache because we aren't in
282+
// dev mode.
283+
284+
await incrementalCache.set(
285+
cacheKey,
286+
{
287+
kind: CachedRouteKind.FETCH,
288+
data: fetchedData,
289+
revalidate: normalizedRevalidate,
290+
},
291+
{
292+
fetchCache: true,
293+
revalidate: externalRevalidate,
294+
fetchUrl,
295+
fetchIdx,
296+
tags,
297+
}
298+
)
299+
await handleUnlock()
300+
301+
// We we return a new Response to the caller.
302+
return new Response(bodyBuffer, {
303+
headers: res.headers,
304+
status: res.status,
305+
statusText: res.statusText,
306+
})
307+
} else {
308+
// We are dynamically rendering including dev mode. We want to return
309+
// the response to the caller as soon as possible because it might stream
310+
// over a very long time.
311+
res
312+
.clone()
313+
.arrayBuffer()
314+
.then(async (arrayBuffer) => {
315+
const bodyBuffer = Buffer.from(arrayBuffer)
316+
317+
const fetchedData = {
318+
headers: Object.fromEntries(res.headers.entries()),
319+
body: bodyBuffer.toString('base64'),
320+
status: res.status,
321+
url: res.url,
322+
}
323+
324+
requestStore?.serverComponentsHmrCache?.set(
325+
cacheKey,
326+
fetchedData
327+
)
328+
329+
if (isCacheableRevalidate) {
330+
await incrementalCache.set(
331+
cacheKey,
332+
{
333+
kind: CachedRouteKind.FETCH,
334+
data: fetchedData,
335+
revalidate: normalizedRevalidate,
336+
},
337+
{
338+
fetchCache: true,
339+
revalidate: externalRevalidate,
340+
fetchUrl,
341+
fetchIdx,
342+
tags,
343+
}
344+
)
345+
}
346+
})
347+
.catch((error) =>
348+
console.warn(\`Failed to set fetch cache\`, input, error)
349+
)
350+
.finally(handleUnlock)
351+
352+
return res
353+
}
354+
}`;
355+
356+
expect(patchCode(code, rule)).toMatchInlineSnapshot(`
357+
"if (
358+
res.status === 200 &&
359+
incrementalCache &&
360+
cacheKey &&
361+
(isCacheableRevalidate || requestStore?.serverComponentsHmrCache)
362+
) {
363+
const normalizedRevalidate =
364+
finalRevalidate >= INFINITE_CACHE
365+
? CACHE_ONE_YEAR
366+
: finalRevalidate
367+
const externalRevalidate =
368+
finalRevalidate >= INFINITE_CACHE ? false : finalRevalidate
369+
370+
if (workUnitStore && workUnitStore.type === 'prerender') {
371+
// We are prerendering at build time or revalidate time with dynamicIO so we need to
372+
// buffer the response so we can guarantee it can be read in a microtask
373+
const bodyBuffer = await res.arrayBuffer()
374+
375+
const fetchedData = {
376+
headers: Object.fromEntries(res.headers.entries()),
377+
body: Buffer.from(bodyBuffer).toString('base64'),
378+
status: res.status,
379+
url: res.url,
380+
}
381+
382+
// We can skip checking the serverComponentsHmrCache because we aren't in
383+
// dev mode.
384+
385+
await incrementalCache.set(
386+
cacheKey,
387+
{
388+
kind: CachedRouteKind.FETCH,
389+
data: fetchedData,
390+
revalidate: normalizedRevalidate,
391+
},
392+
{
393+
fetchCache: true,
394+
revalidate: externalRevalidate,
395+
fetchUrl,
396+
fetchIdx,
397+
tags,
398+
}
399+
)
400+
await handleUnlock()
401+
402+
// We we return a new Response to the caller.
403+
return new Response(bodyBuffer, {
404+
headers: res.headers,
405+
status: res.status,
406+
statusText: res.statusText,
407+
})
408+
} else {
409+
// We are dynamically rendering including dev mode. We want to return
410+
// the response to the caller as soon as possible because it might stream
411+
// over a very long time.
412+
globalThis.__openNextAls?.getStore()?.waitUntil?.(res
413+
.clone()
414+
.arrayBuffer()
415+
.then(async (arrayBuffer) => {
416+
const bodyBuffer = Buffer.from(arrayBuffer)
417+
418+
const fetchedData = {
419+
headers: Object.fromEntries(res.headers.entries()),
420+
body: bodyBuffer.toString('base64'),
421+
status: res.status,
422+
url: res.url,
423+
}
424+
425+
requestStore?.serverComponentsHmrCache?.set(
426+
cacheKey,
427+
fetchedData
428+
)
429+
430+
if (isCacheableRevalidate) {
431+
await incrementalCache.set(
432+
cacheKey,
433+
{
434+
kind: CachedRouteKind.FETCH,
435+
data: fetchedData,
436+
revalidate: normalizedRevalidate,
437+
},
438+
{
439+
fetchCache: true,
440+
revalidate: externalRevalidate,
441+
fetchUrl,
442+
fetchIdx,
443+
tags,
444+
}
445+
)
446+
}
447+
})
448+
.catch((error) =>
449+
console.warn(\`Failed to set fetch cache\`, input, error)
450+
)
451+
.finally(handleUnlock))
452+
453+
454+
return res
455+
}
456+
}"
457+
`);
458+
});
459+
});
460+
});

‎packages/cloudflare/src/cli/build/patches/plugins/fetch-cache-wait-until.ts

+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { getCrossPlatformPathRegex } from "@opennextjs/aws/utils/regex.js";
2+
3+
import { patchCode } from "../ast/util.js";
4+
import type { ContentUpdater } from "./content-updater.js";
5+
6+
/**
7+
* The following Next.js code sets values in the incremental cache for fetch calls:
8+
* https://github.com/vercel/next.js/blob/e5fc495e3d4/packages/next/src/server/lib/patch-fetch.ts#L690-L728
9+
*
10+
* The issue here is that this promise is never awaited in the Next.js code (since in a standard node.js server
11+
* the promise will eventually simply just run) but we do need to run it inside `waitUntil` (so that the worker
12+
* is not killed before the promise is fully executed), without that this promise gets discarded and values
13+
* don't get saved in the incremental cache.
14+
*
15+
* This function wraps the promise in a `waitUntil` call (retrieved from `globalThis.__openNextAls.getStore()`).
16+
*/
17+
export function patchFetchCacheSetMissingWaitUntil(updater: ContentUpdater) {
18+
return updater.updateContent(
19+
"patch-fetch-cache-set-missing-wait-until",
20+
{
21+
filter: getCrossPlatformPathRegex(
22+
String.raw`(server/chunks/.*\.js|.*\.runtime\..*\.js|patch-fetch\.js)$`,
23+
{ escape: false }
24+
),
25+
contentFilter: /arrayBuffer\(\)\s*\.then/,
26+
},
27+
({ contents }) => patchCode(contents, rule)
28+
);
29+
}
30+
31+
export const rule = `
32+
rule:
33+
kind: call_expression
34+
pattern: $PROMISE
35+
all:
36+
- has: { pattern: $_.arrayBuffer().then, stopBy: end }
37+
- has: { pattern: "Buffer.from", stopBy: end }
38+
- any:
39+
- inside:
40+
kind: sequence_expression
41+
inside:
42+
kind: return_statement
43+
- inside:
44+
kind: expression_statement
45+
precedes:
46+
kind: return_statement
47+
- has: { pattern: $_.FETCH, stopBy: end }
48+
49+
fix: |
50+
globalThis.__openNextAls?.getStore()?.waitUntil?.($PROMISE)
51+
`;

0 commit comments

Comments
 (0)
Please sign in to comment.