diff --git a/.changeset/fuzzy-nails-invent.md b/.changeset/fuzzy-nails-invent.md new file mode 100644 index 000000000000..e21d23648c10 --- /dev/null +++ b/.changeset/fuzzy-nails-invent.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/pages-shared": minor +--- + +feat: Return a 304 Not Modified response when matching an asset preservation cache request if appropriate diff --git a/.changeset/green-socks-trade.md b/.changeset/green-socks-trade.md new file mode 100644 index 000000000000..56a625bfceb4 --- /dev/null +++ b/.changeset/green-socks-trade.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/pages-shared": patch +--- + +chore: Remove now-unused asset preservation cache (v1) diff --git a/.changeset/proud-rules-try.md b/.changeset/proud-rules-try.md new file mode 100644 index 000000000000..a3bdede4a6bb --- /dev/null +++ b/.changeset/proud-rules-try.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/pages-shared": patch +--- + +fix: Store an empty result when Early Hints parsing returns nothing or errors. Previously, we weren't storing anything which resulted in Early Hints being parsed on every request. diff --git a/packages/pages-shared/__tests__/asset-server/handler.test.ts b/packages/pages-shared/__tests__/asset-server/handler.test.ts index b97c1a8e91f0..905ffaed1a08 100644 --- a/packages/pages-shared/__tests__/asset-server/handler.test.ts +++ b/packages/pages-shared/__tests__/asset-server/handler.test.ts @@ -510,6 +510,87 @@ describe("asset-server handler", () => { ); }); + test("early hints should cache empty link headers", async () => { + const deploymentId = "deployment-" + Math.random(); + const metadata = createMetadataObject({ deploymentId }) as Metadata; + + const findAssetEntryForPath = async (path: string) => { + if (path === "/index.html") { + return "index.html"; + } + + return null; + }; + + // Create cache storage to reuse between requests + const { caches } = createCacheStorage(); + + const getResponse = async () => + getTestResponse({ + request: new Request("https://example.com/"), + metadata, + findAssetEntryForPath, + caches, + fetchAsset: () => + Promise.resolve( + Object.assign( + new Response(` + + + +

I'm a teapot

+ + `), + { contentType: "text/html" } + ) + ), + }); + + const { response, spies } = await getResponse(); + expect(response.status).toBe(200); + // waitUntil should be called twice: once for asset-preservation, once for early hints + expect(spies.waitUntil.length).toBe(2); + + await Promise.all(spies.waitUntil); + + const earlyHintsCache = await caches.open(`eh:${deploymentId}`); + const earlyHintsRes = await earlyHintsCache.match("https://example.com/"); + + if (!earlyHintsRes) { + throw new Error( + "Did not match early hints cache on https://example.com/" + ); + } + + expect(earlyHintsRes.headers.get("link")).toBeNull(); + + // Do it again, but this time ensure that we didn't write to cache again + const { response: response2, spies: spies2 } = await getResponse(); + + expect(response2.status).toBe(200); + // waitUntil should only be called for asset-preservation + expect(spies2.waitUntil.length).toBe(1); + + await Promise.all(spies2.waitUntil); + + const earlyHintsRes2 = await earlyHintsCache.match("https://example.com/"); + + if (!earlyHintsRes2) { + throw new Error( + "Did not match early hints cache on https://example.com/" + ); + } + + expect(earlyHintsRes2.headers.get("link")).toBeNull(); + }); + + test.todo( + "early hints should temporarily cache failures to parse links", + async () => { + // I couldn't figure out a way to make HTMLRewriter error out + } + ); + describe("should serve deleted assets from preservation cache", async () => { beforeEach(() => { vi.useFakeTimers(); diff --git a/packages/pages-shared/asset-server/handler.ts b/packages/pages-shared/asset-server/handler.ts index d1f68e33e7c9..7e7db890c2e8 100644 --- a/packages/pages-shared/asset-server/handler.ts +++ b/packages/pages-shared/asset-server/handler.ts @@ -436,22 +436,30 @@ export async function generateHandler< }); const linkHeader = preEarlyHintsHeaders.get("Link"); + const earlyHintsHeaders = new Headers({ + "Cache-Control": "max-age=2592000", // 30 days + }); if (linkHeader) { - await earlyHintsCache.put( - earlyHintsCacheKey, - new Response(null, { - headers: { - Link: linkHeader, - "Cache-Control": "max-age=2592000", // 30 days - }, - }) - ); + earlyHintsHeaders.append("Link", linkHeader); } + await earlyHintsCache.put( + earlyHintsCacheKey, + new Response(null, { headers: earlyHintsHeaders }) + ); } catch (err) { // Nbd if we fail here in the deferred 'waitUntil' work. We're probably trying to parse a malformed page or something. // Totally fine to skip over any errors. // If we need to debug something, you can uncomment the following: // logError(err) + // In any case, let's not bother checking again for another day. + await earlyHintsCache.put( + earlyHintsCacheKey, + new Response(null, { + headers: { + "Cache-Control": "max-age=86400", // 1 day + }, + }) + ); } })() );