Enhance cache stale checking logic (#1142)

* feat(cache): implement isStale check for cache entries and add related tests

* feat(tagCache): enhance isStale logic to consider revalidatedAt timestamp

* update comment

* update e2e test

* linting

* linting

* changeset

* review

Author: conico974

.changeset/big-turtles-move.md

@@ -0,0 +1,6 @@
+---
+"@opennextjs/aws": patch
+---
+
+Fix cache interceptor isStale handling for Next 16+
+Fix next mode tag cache not retrieving stale data properly

packages/open-next/src/core/routing/cacheInterceptor.ts

@@ -10,7 +10,7 @@ import type { CacheValue } from "types/overrides";
 import { emptyReadableStream, toReadableStream } from "utils/stream";
 
 import { isBinaryContentType } from "utils/binary";
-import { getTagsFromValue, hasBeenRevalidated } from "utils/cache";
+import { getTagsFromValue, hasBeenRevalidated, isStale } from "utils/cache";
 import { debug, error } from "../../adapters/logger";
 import { localizePath } from "./i18n";
 import { generateMessageGroupId } from "./queue";
@@ -39,6 +39,7 @@ async function computeCacheControl(
   host: string,
   revalidate?: number | false,
   lastModified?: number,
+  isStaleFromTagCache = false,
 ) {
   let finalRevalidate = CACHE_ONE_YEAR;
 
@@ -67,19 +68,26 @@ async function computeCacheControl(
       etag,
     };
   }
-  if (finalRevalidate !== CACHE_ONE_YEAR) {
-    const sMaxAge = Math.max(finalRevalidate - age, 1);
+
+  // SSG uses one year cache
+  const isSSG = finalRevalidate === CACHE_ONE_YEAR;
+  const remainingTtl = Math.max(finalRevalidate - age, 1);
+
+  const isStaleFromTime = !isSSG && remainingTtl === 1;
+  const isStale = isStaleFromTime || isStaleFromTagCache;
+
+  if (!isSSG || isStaleFromTagCache) {
+    const sMaxAge = isStaleFromTagCache ? 1 : remainingTtl;
     debug("sMaxAge", {
       finalRevalidate,
       age,
       lastModified,
       revalidate,
+      isStaleFromTagCache,
     });
-    const isStale = sMaxAge === 1;
     if (isStale) {
       let url = NextConfig.trailingSlash ? `${path}/` : path;
       if (NextConfig.basePath) {
-        // We need to add the basePath to the url
         url = `${NextConfig.basePath}${url}`;
       }
       await globalThis.queue.send({
@@ -140,6 +148,7 @@ async function generateResult(
   localizedPath: string,
   cachedValue: CacheValue<"cache">,
   lastModified?: number,
+  isStaleFromTagCache = false,
 ): Promise<InternalResult> {
   debug("Returning result from experimental cache");
   let body = "";
@@ -172,6 +181,7 @@ async function generateResult(
     event.headers.host,
     cachedValue.revalidate,
     lastModified,
+    isStaleFromTagCache,
   );
   return {
     type: "core",
@@ -277,13 +287,12 @@ export async function cacheInterceptor(
       if (!cachedData?.value) {
         return event;
       }
+      const tags = getTagsFromValue(cachedData.value);
       // We need to check the tag cache now
       if (
         cachedData.value?.type === "app" ||
         cachedData.value?.type === "route"
       ) {
-        const tags = getTagsFromValue(cachedData.value);
-
         const _hasBeenRevalidated = cachedData.shouldBypassTagCache
           ? false
           : await hasBeenRevalidated(localizedPath, tags, cachedData);
@@ -292,6 +301,16 @@ export async function cacheInterceptor(
           return event;
         }
       }
+
+      // Check if the cache entry is stale (valid but needs background revalidation)
+      const _isStale = cachedData.shouldBypassTagCache
+        ? false
+        : await isStale(
+            localizedPath,
+            tags,
+            cachedData.lastModified ?? Date.now(),
+          );
+
       const host = event.headers.host;
       switch (cachedData?.value?.type) {
         case "app":
@@ -301,6 +320,7 @@ export async function cacheInterceptor(
             localizedPath,
             cachedData.value,
             cachedData.lastModified,
+            _isStale,
           );
         case "redirect": {
           const cacheControl = await computeCacheControl(
@@ -309,6 +329,7 @@ export async function cacheInterceptor(
             host,
             cachedData.value.revalidate,
             cachedData.lastModified,
+            _isStale,
           );
           return {
             type: "core",
@@ -329,6 +350,7 @@ export async function cacheInterceptor(
             host,
             cachedData.value.revalidate,
             cachedData.lastModified,
+            _isStale,
           );
 
           const isBinary = isBinaryContentType(

packages/open-next/src/overrides/tagCache/dynamodb-nextMode.ts

@@ -208,7 +208,14 @@ export default {
 
     const compute = (item: any): boolean => {
       if (!item?.stale?.N) return false;
-      return Number.parseInt(item.stale.N) > (lastModified ?? 0);
+      const revalidatedAt = Number.parseInt(item.revalidatedAt?.N ?? "0");
+      // A tag is stale when both its stale timestamp and its revalidatedAt are newer than the page.
+      // revalidatedAt > lastModified ensures the revalidation that set this stale window happened
+      // after the page was generated, preventing a stale signal from a previous ISR cycle.
+      return (
+        revalidatedAt > (lastModified ?? 0) &&
+        Number.parseInt(item.stale.N) >= (lastModified ?? 0)
+      );
     };
 
     const { uncachedTags, hasMatch } = checkItemsCache(

packages/open-next/src/overrides/tagCache/fs-dev-nextMode.ts

@@ -61,7 +61,13 @@ export default {
       if (!tagData || typeof tagData.stale !== "number") {
         return false;
       }
-      return tagData.stale > (lastModified ?? 0);
+      // A tag is stale when both its stale timestamp and its revalidatedAt are newer than the page.
+      // revalidatedAt > lastModified ensures the revalidation that set this stale window happened
+      // after the page was generated, preventing a stale signal from a previous ISR cycle.
+      return (
+        tagData.revalidatedAt > (lastModified ?? 0) &&
+        tagData.stale >= (lastModified ?? 0)
+      );
     });
     debug("isStale result:", hasStaleTag);
     return hasStaleTag;

packages/open-next/src/overrides/tagCache/fs-dev.ts

@@ -68,7 +68,13 @@ const tagCache: TagCache = {
     return tags.some((entry) => {
       if (entry.path.S !== buildKey(path)) return false;
       if (!entry.stale?.N) return false;
-      return Number.parseInt(entry.stale.N) > (lastModified ?? 0);
+      // A tag is stale when both its stale timestamp and its revalidatedAt are newer than the page.
+      // revalidatedAt > lastModified ensures the revalidation that set this stale window happened
+      // after the page was generated, preventing a stale signal from a previous ISR cycle.
+      return (
+        Number.parseInt(entry.revalidatedAt.N) > (lastModified ?? 0) &&
+        Number.parseInt(entry.stale.N) > (lastModified ?? 0)
+      );
     });
   },
   writeTags: async (newTags) => {

packages/tests-e2e/tests/appRouter/revalidateTag.test.ts

@@ -113,7 +113,7 @@ test("Revalidate tag - stale data served first", async ({ page, request }) => {
   const staleHeaders = staleResponse.headers();
   const staleCache =
     staleHeaders["x-nextjs-cache"] ?? staleHeaders["x-opennext-cache"];
-  expect(staleCache).toMatch(/^(STALE|HIT)$/);
+  expect(staleCache).toMatch(/^(STALE)$/);
 
   const staleTime = await page.getByTestId("cached-time").textContent();
   // Stale content must match the pre-revalidation value
@@ -137,6 +137,22 @@ test("Revalidate tag - stale data served first", async ({ page, request }) => {
   // After background regen the cached value must have been updated
   expect(freshTime).not.toBeNull();
   expect(freshTime).not.toEqual(originalTime);
+
+  // Now we want to verfiy that the next entries stays fresh (HIT) after the first stale entry
+  responsePromise = page.waitForResponse(
+    (response) =>
+      response.url().includes("/revalidate-tag/stale") &&
+      response.status() === 200,
+  );
+  await page.goto("/revalidate-tag/stale");
+  const finalResponse = await responsePromise;
+  const finalHeaders = finalResponse.headers();
+  const finalCache =
+    finalHeaders["x-nextjs-cache"] ?? finalHeaders["x-opennext-cache"];
+  expect(finalCache).toEqual("HIT");
+
+  const finalTime = await page.getByTestId("cached-time").textContent();
+  expect(finalTime).toEqual(freshTime);
 });
 
 test("Revalidate path", async ({ page, request }) => {

packages/tests-unit/tests/core/routing/cacheInterceptor.test.ts

@@ -59,9 +59,11 @@ const incrementalCache = {
 
 const tagCache = {
   name: "mock",
+  mode: "original",
   getByTag: vi.fn(),
   getByPath: vi.fn(),
   getLastModified: vi.fn(),
+  isStale: vi.fn().mockResolvedValue(false),
   writeTags: vi.fn(),
 };
 
@@ -74,6 +76,7 @@ declare global {
   var queue: Queue;
   var incrementalCache: any;
   var tagCache: any;
+  var nextVersion: string;
 }
 
 globalThis.incrementalCache = incrementalCache;
@@ -83,6 +86,7 @@ globalThis.queue = queue;
 beforeEach(() => {
   vi.useFakeTimers().setSystemTime("2024-01-02T00:00:00Z");
   vi.clearAllMocks();
+  globalThis.nextVersion = "16.0.0";
   globalThis.openNextConfig = {
     dangerous: {
       disableTagCache: false,
@@ -653,4 +657,119 @@ describe("cacheInterceptor", () => {
       expect((result as any).headers["x-nextjs-postponed"]).toBeUndefined();
     });
   });
+
+  describe("isStale", () => {
+    it("should serve stale app content when isStale returns true", async () => {
+      const event = createEvent({ url: "/albums" });
+      incrementalCache.get.mockResolvedValueOnce({
+        value: {
+          type: "app",
+          html: "Hello, world!",
+        },
+        lastModified: new Date("2024-01-02T00:00:00Z").getTime(),
+      });
+      tagCache.isStale.mockResolvedValueOnce(true);
+
+      const result = await cacheInterceptor(event);
+
+      const body = await fromReadableStream(result.body);
+      expect(body).toEqual("Hello, world!");
+      expect(result).toEqual(
+        expect.objectContaining({
+          type: "core",
+          headers: expect.objectContaining({
+            "cache-control": "s-maxage=1, stale-while-revalidate=2592000",
+            "x-opennext-cache": "STALE",
+          }),
+        }),
+      );
+      expect(queue.send).toHaveBeenCalled();
+    });
+
+    it("should serve stale route content when isStale returns true", async () => {
+      const event = createEvent({ url: "/albums" });
+      incrementalCache.get.mockResolvedValueOnce({
+        value: {
+          type: "route",
+          body: "API response",
+          meta: {
+            status: 200,
+            headers: { "content-type": "application/json" },
+          },
+          revalidate: 300,
+        },
+        lastModified: new Date("2024-01-02T00:00:00Z").getTime(),
+      });
+      tagCache.isStale.mockResolvedValueOnce(true);
+
+      const result = await cacheInterceptor(event);
+
+      expect(result).toEqual(
+        expect.objectContaining({
+          headers: expect.objectContaining({
+            "cache-control": "s-maxage=1, stale-while-revalidate=2592000",
+            "x-opennext-cache": "STALE",
+          }),
+        }),
+      );
+      expect(queue.send).toHaveBeenCalled();
+    });
+
+    it("should not check isStale when shouldBypassTagCache is true", async () => {
+      const event = createEvent({ url: "/albums" });
+      incrementalCache.get.mockResolvedValueOnce({
+        value: {
+          type: "app",
+          html: "Hello, world!",
+        },
+        lastModified: new Date("2024-01-02T00:00:00Z").getTime(),
+        shouldBypassTagCache: true,
+      });
+
+      await cacheInterceptor(event);
+
+      expect(tagCache.isStale).not.toHaveBeenCalled();
+    });
+
+    it("should not call isStale when nextVersion is below 16", async () => {
+      globalThis.nextVersion = "15.0.0";
+      const event = createEvent({ url: "/albums" });
+      incrementalCache.get.mockResolvedValueOnce({
+        value: {
+          type: "app",
+          html: "Hello, world!",
+        },
+        lastModified: new Date("2024-01-02T00:00:00Z").getTime(),
+      });
+
+      await cacheInterceptor(event);
+
+      expect(tagCache.isStale).not.toHaveBeenCalled();
+    });
+
+    it("should serve fresh content when isStale returns false", async () => {
+      const event = createEvent({ url: "/albums" });
+      incrementalCache.get.mockResolvedValueOnce({
+        value: {
+          type: "app",
+          html: "Hello, world!",
+        },
+        lastModified: new Date("2024-01-02T00:00:00Z").getTime(),
+      });
+      tagCache.isStale.mockResolvedValueOnce(false);
+
+      const result = await cacheInterceptor(event);
+
+      expect(result).toEqual(
+        expect.objectContaining({
+          headers: expect.objectContaining({
+            "cache-control":
+              "s-maxage=31536000, stale-while-revalidate=2592000",
+            "x-opennext-cache": "HIT",
+          }),
+        }),
+      );
+      expect(queue.send).not.toHaveBeenCalled();
+    });
+  });
 });