fix: make sure that fetch cache sets are properly awaited

.changeset/few-ducks-listen.md

@@ -0,0 +1,9 @@
+---
+"@opennextjs/cloudflare": patch
+---
+
+fix: make sure that fetch cache `set`s 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`

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

@@ -28,7 +28,7 @@ test.skip("Server Side Render and loading.tsx", async ({ page }) => {
   }
 });
 
-test.skip("Fetch cache properly cached", async ({ page }) => {
+test("Fetch cache properly cached", async ({ page }) => {
   await page.goto("/ssr");
   const originalDate = await page.getByText("Cached fetch:").textContent();
   await page.waitForTimeout(2000);

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

@@ -10,6 +10,7 @@ import { patchVercelOgLibrary } from "./patches/ast/patch-vercel-og-library.js";
 import { patchWebpackRuntime } from "./patches/ast/webpack-runtime.js";
 import * as patches from "./patches/index.js";
 import { ContentUpdater } from "./patches/plugins/content-updater.js";
+import { patchFetchCacheSetMissingWaitUntil } from "./patches/plugins/fetch-cache-wait-until.js";
 import { patchLoadInstrumentation } from "./patches/plugins/load-instrumentation.js";
 import { handleOptionalDependencies } from "./patches/plugins/optional-deps.js";
 import { fixRequire } from "./patches/plugins/require.js";
@@ -87,6 +88,7 @@ export async function bundleServer(buildOpts: BuildOptions): Promise<void> {
       fixRequire(updater),
       handleOptionalDependencies(optionalDependencies),
       patchLoadInstrumentation(updater),
+      patchFetchCacheSetMissingWaitUntil(updater),
       // Apply updater updaters, must be the last plugin
       updater.plugin,
     ],

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

@@ -0,0 +1,107 @@
+import { describe, expect, test } from "vitest";
+
+import { patchCode } from "../ast/util.js";
+import { ruleForMinifiedCode, ruleForNonMinifiedCode } from "./fetch-cache-wait-until.js";
+
+describe("patchFetchCacheSetMissingWaitUntil", () => {
+  test("on minified code", () => {
+    const code = `
+{
+  let [o4, a2] = (0, d2.cloneResponse)(e3);
+  return o4.arrayBuffer().then(async (e4) => {
+    var a3;
+    let i4 = Buffer.from(e4), s3 = { headers: Object.fromEntries(o4.headers.entries()), body: i4.toString("base64"), status: o4.status, url: o4.url };
+    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 });
+  }).catch((e4) => console.warn("Failed to set fetch cache", u4, e4)).finally(X), a2;
+}`;
+
+    expect(patchCode(code, ruleForMinifiedCode)).toMatchInlineSnapshot(`
+      "{
+        let [o4, a2] = (0, d2.cloneResponse)(e3);
+        globalThis.__openNextAls?.getStore()?.waitUntil?.(o4.arrayBuffer().then(async (e4) => {
+          var a3;
+          let i4 = Buffer.from(e4), s3 = { headers: Object.fromEntries(o4.headers.entries()), body: i4.toString("base64"), status: o4.status, url: o4.url };
+          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 });
+        }).catch((e4) => console.warn("Failed to set fetch cache", u4, e4)).finally(X));
+      return a2;
+
+      }"
+    `);
+  });
+
+  test("on non-minified code", () => {
+    const code = `
+      // We're cloning the response using this utility because there
+      // exists a bug in the undici library around response cloning.
+      // See the following pull request for more details:
+      // https://github.com/vercel/next.js/pull/73274
+      const [cloned1, cloned2] = (0, _cloneresponse.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.
+      cloned1.arrayBuffer().then(async (arrayBuffer)=>{
+          var _requestStore_serverComponentsHmrCache;
+          const bodyBuffer = Buffer.from(arrayBuffer);
+          const fetchedData = {
+              headers: Object.fromEntries(cloned1.headers.entries()),
+              body: bodyBuffer.toString('base64'),
+              status: cloned1.status,
+              url: cloned1.url
+          };
+          requestStore == null ? void 0 : (_requestStore_serverComponentsHmrCache = requestStore.serverComponentsHmrCache) == null ? void 0 : _requestStore_serverComponentsHmrCache.set(cacheKey, fetchedData);
+          if (isCacheableRevalidate) {
+              await incrementalCache.set(cacheKey, {
+                  kind: _responsecache.CachedRouteKind.FETCH,
+                  data: fetchedData,
+                  revalidate: normalizedRevalidate
+              }, {
+                  fetchCache: true,
+                  revalidate: externalRevalidate,
+                  fetchUrl,
+                  fetchIdx,
+                  tags
+              });
+          }
+      }).catch((error)=>console.warn(\`Failed to set fetch cache\`, input, error)).finally(handleUnlock);
+      return cloned2;
+    `;
+
+    expect(patchCode(code, ruleForNonMinifiedCode)).toMatchInlineSnapshot(`
+      "// We're cloning the response using this utility because there
+            // exists a bug in the undici library around response cloning.
+            // See the following pull request for more details:
+            // https://github.com/vercel/next.js/pull/73274
+            const [cloned1, cloned2] = (0, _cloneresponse.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.
+            globalThis.__openNextAls?.getStore()?.waitUntil?.(cloned1.arrayBuffer().then(async (arrayBuffer)=>{
+                var _requestStore_serverComponentsHmrCache;
+                const bodyBuffer = Buffer.from(arrayBuffer);
+                const fetchedData = {
+                    headers: Object.fromEntries(cloned1.headers.entries()),
+                    body: bodyBuffer.toString('base64'),
+                    status: cloned1.status,
+                    url: cloned1.url
+                };
+                requestStore == null ? void 0 : (_requestStore_serverComponentsHmrCache = requestStore.serverComponentsHmrCache) == null ? void 0 : _requestStore_serverComponentsHmrCache.set(cacheKey, fetchedData);
+                if (isCacheableRevalidate) {
+                    await incrementalCache.set(cacheKey, {
+                        kind: _responsecache.CachedRouteKind.FETCH,
+                        data: fetchedData,
+                        revalidate: normalizedRevalidate
+                    }, {
+                        fetchCache: true,
+                        revalidate: externalRevalidate,
+                        fetchUrl,
+                        fetchIdx,
+                        tags
+                    });
+                }
+            }).catch((error)=>console.warn(\`Failed to set fetch cache\`, input, error)).finally(handleUnlock));
+
+            return cloned2;
+          "
+    `);
+  });
+});

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

@@ -0,0 +1,58 @@
+import { patchCode } from "../ast/util.js";
+import type { ContentUpdater } from "./content-updater.js";
+
+/**
+ * The following Next.js code sets values in the incremental cache for fetch calls:
+ *  https://github.com/vercel/next.js/blob/e5fc495e3d4/packages/next/src/server/lib/patch-fetch.ts#L690-L728
+ *
+ * The issue here is that this promise is never awaited in the Next.js code (since in a standard node.js server
+ * the promise will eventually simply just run) but we do need to run it inside `waitUntil` (so that the worker
+ * is not killed before the promise is fully executed), without that this promise gets discarded and values
+ * don't get saved in the incremental cache.
+ *
+ * This function wraps the promise in a `waitUntil` call (retrieved from `globalThis.__openNextAls.getStore()`).
+ */
+export function patchFetchCacheSetMissingWaitUntil(updater: ContentUpdater) {
+  return updater.updateContent(
+    "patch-fetch-cache-set-missing-wait-until",
+    {
+      filter: /(server\/chunks\/.*\.js|.*\.runtime\..*\.js|patch-fetch\.js)$/,
+      contentFilter: /Failed to set fetch cache/,
+    },
+    ({ contents }) => {
+      contents = patchCode(contents, ruleForMinifiedCode);
+      return patchCode(contents, ruleForNonMinifiedCode);
+    }
+  );
+}
+
+export const ruleForMinifiedCode = `
+rule:
+  pattern: return $PROMISE, $CLONED2
+  regex: Failed to set fetch cache
+  follows:
+    kind: lexical_declaration
+    pattern: let [$CLONED1, $CLONED2]
+
+fix: |
+  globalThis.__openNextAls?.getStore()?.waitUntil?.($PROMISE);
+  return $CLONED2;
+`;
+
+export const ruleForNonMinifiedCode = `
+rule:
+  regex: Failed to set fetch cache
+  pattern: $PROMISE;
+  follows:
+    kind: comment
+    follows:
+      kind: comment
+      follows:
+        kind: comment
+        follows:
+          kind: lexical_declaration
+          pattern: const [cloned1, cloned2]
+
+fix: |
+  globalThis.__openNextAls?.getStore()?.waitUntil?.($PROMISE);
+`;