Skip to content

Commit 61484b6

Browse files
dario-piotrowiczvicb
dario-piotrowicz
and
vicb
committed
fix: make sure that fetch cache sets are properly awaited
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 <[email protected]>
1 parent 2a3e896 commit 61484b6

File tree

5 files changed

+174
-1
lines changed

5 files changed

+174
-1
lines changed

‎.changeset/few-ducks-listen.md‎

Lines changed: 9 additions & 0 deletions
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‎

Lines changed: 1 addition & 1 deletion
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‎

Lines changed: 2 additions & 0 deletions
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/fetch-cache-wait-until.spec.ts‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import { describe, expect, test } from "vitest";
2+
3+
import { patchCode } from "../ast/util.js";
4+
import { ruleForMinifiedCode, ruleForNonMinifiedCode } 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, ruleForMinifiedCode)).toMatchInlineSnapshot(`
19+
"{
20+
let [o4, a2] = (0, d2.cloneResponse)(e3);
21+
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+
return a2;
27+
28+
}"
29+
`);
30+
});
31+
32+
test("on non-minified code", () => {
33+
const code = `
34+
// We're cloning the response using this utility because there
35+
// exists a bug in the undici library around response cloning.
36+
// See the following pull request for more details:
37+
// https://github.com/vercel/next.js/pull/73274
38+
const [cloned1, cloned2] = (0, _cloneresponse.cloneResponse)(res);
39+
// We are dynamically rendering including dev mode. We want to return
40+
// the response to the caller as soon as possible because it might stream
41+
// over a very long time.
42+
cloned1.arrayBuffer().then(async (arrayBuffer)=>{
43+
var _requestStore_serverComponentsHmrCache;
44+
const bodyBuffer = Buffer.from(arrayBuffer);
45+
const fetchedData = {
46+
headers: Object.fromEntries(cloned1.headers.entries()),
47+
body: bodyBuffer.toString('base64'),
48+
status: cloned1.status,
49+
url: cloned1.url
50+
};
51+
requestStore == null ? void 0 : (_requestStore_serverComponentsHmrCache = requestStore.serverComponentsHmrCache) == null ? void 0 : _requestStore_serverComponentsHmrCache.set(cacheKey, fetchedData);
52+
if (isCacheableRevalidate) {
53+
await incrementalCache.set(cacheKey, {
54+
kind: _responsecache.CachedRouteKind.FETCH,
55+
data: fetchedData,
56+
revalidate: normalizedRevalidate
57+
}, {
58+
fetchCache: true,
59+
revalidate: externalRevalidate,
60+
fetchUrl,
61+
fetchIdx,
62+
tags
63+
});
64+
}
65+
}).catch((error)=>console.warn(\`Failed to set fetch cache\`, input, error)).finally(handleUnlock);
66+
return cloned2;
67+
`;
68+
69+
expect(patchCode(code, ruleForNonMinifiedCode)).toMatchInlineSnapshot(`
70+
"// We're cloning the response using this utility because there
71+
// exists a bug in the undici library around response cloning.
72+
// See the following pull request for more details:
73+
// https://github.com/vercel/next.js/pull/73274
74+
const [cloned1, cloned2] = (0, _cloneresponse.cloneResponse)(res);
75+
// We are dynamically rendering including dev mode. We want to return
76+
// the response to the caller as soon as possible because it might stream
77+
// over a very long time.
78+
globalThis.__openNextAls?.getStore()?.waitUntil?.(cloned1.arrayBuffer().then(async (arrayBuffer)=>{
79+
var _requestStore_serverComponentsHmrCache;
80+
const bodyBuffer = Buffer.from(arrayBuffer);
81+
const fetchedData = {
82+
headers: Object.fromEntries(cloned1.headers.entries()),
83+
body: bodyBuffer.toString('base64'),
84+
status: cloned1.status,
85+
url: cloned1.url
86+
};
87+
requestStore == null ? void 0 : (_requestStore_serverComponentsHmrCache = requestStore.serverComponentsHmrCache) == null ? void 0 : _requestStore_serverComponentsHmrCache.set(cacheKey, fetchedData);
88+
if (isCacheableRevalidate) {
89+
await incrementalCache.set(cacheKey, {
90+
kind: _responsecache.CachedRouteKind.FETCH,
91+
data: fetchedData,
92+
revalidate: normalizedRevalidate
93+
}, {
94+
fetchCache: true,
95+
revalidate: externalRevalidate,
96+
fetchUrl,
97+
fetchIdx,
98+
tags
99+
});
100+
}
101+
}).catch((error)=>console.warn(\`Failed to set fetch cache\`, input, error)).finally(handleUnlock));
102+
103+
return cloned2;
104+
"
105+
`);
106+
});
107+
});

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { patchCode } from "../ast/util.js";
2+
import type { ContentUpdater } from "./content-updater.js";
3+
4+
/**
5+
* The following Next.js code sets values in the incremental cache for fetch calls:
6+
* https://github.com/vercel/next.js/blob/e5fc495e3d4/packages/next/src/server/lib/patch-fetch.ts#L690-L728
7+
*
8+
* The issue here is that this promise is never awaited in the Next.js code (since in a standard node.js server
9+
* the promise will eventually simply just run) but we do need to run it inside `waitUntil` (so that the worker
10+
* is not killed before the promise is fully executed), without that this promise gets discarded and values
11+
* don't get saved in the incremental cache.
12+
*
13+
* This function wraps the promise in a `waitUntil` call (retrieved from `globalThis.__openNextAls.getStore()`).
14+
*/
15+
export function patchFetchCacheSetMissingWaitUntil(updater: ContentUpdater) {
16+
return updater.updateContent(
17+
"patch-fetch-cache-set-missing-wait-until",
18+
{ filter: /\.(js|mjs|cjs|jsx|ts|tsx)$/, contentFilter: /Failed to set fetch cache/ },
19+
({ contents }) => {
20+
contents = patchCode(contents, ruleForMinifiedCode);
21+
return patchCode(contents, ruleForNonMinifiedCode);
22+
}
23+
);
24+
}
25+
26+
export const ruleForMinifiedCode = `
27+
rule:
28+
pattern: return $PROMISE, $CLONED2
29+
regex: Failed to set fetch cache
30+
follows:
31+
kind: lexical_declaration
32+
pattern: let [$CLONED1, $CLONED2]
33+
34+
fix: |
35+
globalThis.__openNextAls?.getStore()?.waitUntil?.($PROMISE);
36+
return $CLONED2;
37+
`;
38+
39+
export const ruleForNonMinifiedCode = `
40+
rule:
41+
regex: Failed to set fetch cache
42+
pattern: $PROMISE;
43+
follows:
44+
kind: comment
45+
follows:
46+
kind: comment
47+
follows:
48+
kind: comment
49+
follows:
50+
kind: lexical_declaration
51+
pattern: const [cloned1, cloned2]
52+
53+
fix: |
54+
globalThis.__openNextAls?.getStore()?.waitUntil?.($PROMISE);
55+
`;

0 commit comments

Comments
 (0)