Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
---

perf: defer regional cache writes so they do not block the response
Comment thread
km-tr marked this conversation as resolved.
Outdated

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