fix(workers): do not cache screenshot failures

.changeset/clean-caches-settle.md

@@ -0,0 +1,2 @@
+---
+---

test/workerScreenshotCache.test.ts

@@ -199,6 +199,63 @@ test("malformed hits and unsafe capture responses never escape into shared cache
   }
 });
 
+test("Browser Rendering errors stay errors and never enter the edge cache", async () => {
+  const request = new Request(cardUrl());
+  const { url, plan: screenshot } = plan(cardUrl());
+  const { cache, entries } = memoryCache();
+  const deferred: Promise<unknown>[] = [];
+
+  const response = await servePostScreenshot({
+    request,
+    requestUrl: url,
+    postId: "post_1",
+    plan: screenshot,
+    rendererGeneration: GENERATION,
+    clientCacheControl: "private, max-age=300",
+    defer: (promise) => deferred.push(promise),
+    authorize: async () => new Response("renderable"),
+    capture: async () =>
+      new Response('{"error":"rate limited"}', {
+        status: 429,
+        headers: { "content-type": "application/json" },
+      }),
+    cache,
+  });
+  await Promise.all(deferred);
+
+  assert.equal(response.status, 429);
+  assert.equal(response.headers.get("content-type"), "application/json");
+  assert.equal(response.headers.get("cache-control"), "no-store");
+  assert.equal(await response.text(), '{"error":"rate limited"}');
+  assert.equal(entries.size, 0);
+});
+
+test("a non-PNG capture never gains a client cache policy", async () => {
+  const request = new Request(cardUrl());
+  const { url, plan: screenshot } = plan(cardUrl());
+  const { cache, entries } = memoryCache();
+
+  const response = await servePostScreenshot({
+    request,
+    requestUrl: url,
+    postId: "post_1",
+    plan: screenshot,
+    rendererGeneration: GENERATION,
+    clientCacheControl: "public, max-age=300",
+    defer: () => {},
+    authorize: async () => new Response("renderable"),
+    capture: async () =>
+      new Response("not an image", { headers: { "content-type": "text/plain" } }),
+    cache,
+  });
+
+  assert.equal(response.status, 200);
+  assert.equal(response.headers.get("content-type"), "text/plain");
+  assert.equal(response.headers.get("cache-control"), "no-store");
+  assert.equal(await response.text(), "not an image");
+  assert.equal(entries.size, 0);
+});
+
 test("orchestration revalidates before hits and applies the current access policy", async () => {
   const request = () => new Request(cardUrl());
   const { url, plan: screenshot } = plan(cardUrl());

workers/screenshotCache.ts

@@ -203,6 +203,10 @@ export async function servePostScreenshot({
     defer,
     async () => {
       const screenshot = await capture();
+      // Browser Rendering returns JSON error responses for failures. Preserve an
+      // unsafe response rather than relabeling its body as a successful PNG:
+      // doing otherwise would let the cache admit an hour-long error payload.
+      if (screenshot.status !== 200 || !isPng(screenshot)) return screenshot;
       return new Response(await screenshot.arrayBuffer(), {
         headers: { "content-type": "image/png", "cache-control": clientCacheControl },
       });
@@ -214,6 +218,12 @@ export async function servePostScreenshot({
   // above uses the CURRENT policy, so its client directive must also win over
   // the policy recorded with an older internal entry.
   const out = new Response(response.body, response);
-  out.headers.set("cache-control", clientCacheControl);
+  // A rejected Browser Rendering response must not acquire the normal positive
+  // client TTL: an intermediary could otherwise cache a transient error even
+  // though the Worker did not put it in the edge cache.
+  out.headers.set(
+    "cache-control",
+    response.status === 200 && isPng(response) ? clientCacheControl : "no-store",
+  );
   return out;
 }