Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/defer-regional-cache-write.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@opennextjs/cloudflare": patch
---

fix: defer regional cache writes so they do not block the response

Next.js awaits `incrementalCache.set` while producing the response, which put the R2
write on the critical path. The returned value is unused, so the writes now run in
`ctx.waitUntil` — matching how the read path already defers its cache updates.
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import type { IncrementalCache } from "@opennextjs/aws/types/overrides.js";
import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";

import { getCloudflareContext } from "../../cloudflare-context.js";
import { withRegionalCache } from "./regional-cache.js";

vi.mock("../../cloudflare-context.js", () => ({
getCloudflareContext: vi.fn(),
}));

const mockedStore = {
name: "mocked-store",
get: vi.fn().mockResolvedValue(null),
set: vi.fn().mockResolvedValue(undefined),
delete: vi.fn().mockResolvedValue(undefined),
} satisfies IncrementalCache;

const mockedWaitUntil = vi.fn();
const mockedPut = vi.fn();

describe("regional-cache", () => {
beforeEach(() => {
// @ts-ignore
globalThis.caches = {
open: vi.fn().mockResolvedValue({
put: mockedPut,
match: vi.fn().mockResolvedValue(undefined),
}),
};
vi.mocked(getCloudflareContext).mockReturnValue({
ctx: { waitUntil: mockedWaitUntil },
// @ts-ignore only `ctx` is used here
env: {},
});
});

afterEach(() => {
vi.resetAllMocks();
});

describe("set", () => {
test("does not wait for the store write to complete", async () => {
let resolveStoreWrite: () => void = () => {};
mockedStore.set.mockReturnValue(
new Promise<void>((resolve) => {
resolveStoreWrite = resolve;
})
);
const cache = withRegionalCache(mockedStore, { mode: "long-lived" });

await cache.set("key", { type: "route", body: "", meta: {} });

// The store write is still pending, yet `set` has already resolved.
expect(mockedWaitUntil).toHaveBeenCalledTimes(1);
resolveStoreWrite();
await mockedWaitUntil.mock.calls[0]![0];
expect(mockedStore.set).toHaveBeenCalledTimes(1);
});

test("writes to the store and the regional cache in the background", async () => {
const cache = withRegionalCache(mockedStore, { mode: "long-lived" });

await cache.set("key", { type: "route", body: "", meta: {} });
await mockedWaitUntil.mock.calls[0]![0];

expect(mockedStore.set).toHaveBeenCalledWith("key", { type: "route", body: "", meta: {} }, undefined);
expect(mockedPut).toHaveBeenCalledTimes(1);
});

test("does not reject when the store write fails", async () => {
mockedStore.set.mockRejectedValue(new Error("store is unavailable"));
const cache = withRegionalCache(mockedStore, { mode: "long-lived" });

await expect(cache.set("key", { type: "route", body: "", meta: {} })).resolves.toBeUndefined();
// A rejected promise handed to `waitUntil` would surface as an unhandled rejection.
await expect(mockedWaitUntil.mock.calls[0]![0]).resolves.toBeUndefined();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -179,24 +179,31 @@ class RegionalCache implements IncrementalCache {
value: CacheValue<CacheType>,
cacheType?: CacheType
): Promise<void> {
try {
debugCache("RegionalCache", `set ${key}`);

await this.store.set(key, value, cacheType);

await this.putToCache({
key,
cacheType,
entry: {
value,
// Note: `Date.now()` returns the time of the last IO rather than the actual time.
// See https://developers.cloudflare.com/workers/reference/security-model/
lastModified: Date.now(),
},
});
} catch (e) {
error(`Failed to set the regional cache`, e);
}
debugCache("RegionalCache", `set ${key}`);

// Next.js awaits `set` while it produces the response, so awaiting the writes here
// puts them on the critical path. The returned value is not used, so they can run
// in the background without changing what is served.
getCloudflareContext().ctx.waitUntil(
(async () => {
try {
await this.store.set(key, value, cacheType);

await this.putToCache({
key,
cacheType,
entry: {
value,
// Note: `Date.now()` returns the time of the last IO rather than the actual time.
// See https://developers.cloudflare.com/workers/reference/security-model/
lastModified: Date.now(),
},
});
} catch (e) {
error(`Failed to set the regional cache`, e);
}
})()
);
}

async delete(key: string): Promise<void> {
Expand Down