Ensure fetch cache TTL is updated properly (#69164)

To reduce the number of set requests we were sending upstream we added
an optimization to skip sending the set request if the cached data
didn't change after a revalidation. The problem with this optimization
is the upstream service relies on this set request to know to update the
cache entries TTL and consider it up to date. This causes unexpected
revalidations while the cache value hasn't changed and the TTL is kept
stale.

x-ref: NEXT-3693
# Conflicts:
#	packages/next/src/server/lib/incremental-cache/fetch-cache.ts
#	test/turbopack-build-tests-manifest.json

packages/next/src/server/lib/incremental-cache/fetch-cache.ts

@@ -103,9 +103,8 @@ export default class FetchCache implements CacheHandler {
       process.env.SUSPENSE_CACHE_BASEPATH
 
     if (process.env.SUSPENSE_CACHE_AUTH_TOKEN) {
-      this.headers[
-        'Authorization'
-      ] = `Bearer ${process.env.SUSPENSE_CACHE_AUTH_TOKEN}`
+      this.headers['Authorization'] =
+        `Bearer ${process.env.SUSPENSE_CACHE_AUTH_TOKEN}`
     }
 
     if (scHost) {
@@ -331,25 +330,6 @@ export default class FetchCache implements CacheHandler {
   public async set(...args: Parameters<CacheHandler['set']>) {
     const [key, data, ctx] = args
 
-    const newValue = data?.kind === 'FETCH' ? data.data : undefined
-    const existingCache = memoryCache?.get(key)
-    const existingValue = existingCache?.value
-    if (
-      existingValue?.kind === 'FETCH' &&
-      Object.keys(existingValue.data).every(
-        (field) =>
-          JSON.stringify(
-            (existingValue.data as Record<string, string | Object>)[field]
-          ) ===
-          JSON.stringify((newValue as Record<string, string | Object>)[field])
-      )
-    ) {
-      if (DEBUG) {
-        console.log(`skipping cache set for ${key} as not modified`)
-      }
-      return
-    }
-
     const { fetchCache, fetchIdx, fetchUrl, tags } = ctx
     if (!fetchCache) return
 

test/ppr-tests-manifest.json

@@ -5,8 +5,9 @@
       "passed": [],
       "failed": [
         "fetch-cache should have correct fetchUrl field for fetches and unstable_cache",
+        "fetch-cache should not retry for failed fetch-cache GET",
         "fetch-cache should retry 3 times when revalidate times out",
-        "fetch-cache should not retry for failed fetch-cache GET"
+        "fetch-cache should update cache TTL even if cache data does not change"
       ],
       "pending": [],
       "flakey": [],

test/production/app-dir/fetch-cache/app/not-changed/page.tsx

@@ -0,0 +1,33 @@
+import { unstable_cache } from 'next/cache'
+
+export const dynamic = 'force-dynamic'
+export const fetchCache = 'default-cache'
+
+// this is ensuring we still update TTL even
+// if the cache value didn't change during revalidate
+const stableCacheItem = unstable_cache(
+  async () => {
+    return 'consistent value'
+  },
+  [],
+  {
+    revalidate: 3,
+    tags: ['thankyounext'],
+  }
+)
+
+export default async function Page() {
+  const data = await fetch('https://example.vercel.sh', {
+    next: { tags: ['thankyounext'], revalidate: 3 },
+  }).then((res) => res.text())
+
+  const cachedValue = stableCacheItem()
+
+  return (
+    <>
+      <p>hello world</p>
+      <p id="data">{data}</p>
+      <p id="cache">{cachedValue}</p>
+    </>
+  )
+}

test/production/app-dir/fetch-cache/fetch-cache.test.ts

@@ -1,6 +1,7 @@
 import http from 'http'
 import fs from 'fs-extra'
 import { join } from 'path'
+import rawBody from 'next/dist/compiled/raw-body'
 import { FileRef, NextInstance, createNext } from 'e2e-utils'
 import {
   retry,
@@ -24,6 +25,8 @@ describe('fetch-cache', () => {
     method: string
     headers: Record<string, string | string[]>
   }> = []
+  let storeCacheItems = false
+  const fetchCacheStore = new Map<string, any>()
   let fetchCacheEnv: Record<string, string> = {
     SUSPENSE_CACHE_PROTO: 'http',
   }
@@ -65,9 +68,10 @@ describe('fetch-cache', () => {
     const testServer = join(next.testDir, 'standalone/server.js')
     await fs.writeFile(
       testServer,
-      (
-        await fs.readFile(testServer, 'utf8')
-      ).replace('port:', `minimalMode: ${minimalMode},port:`)
+      (await fs.readFile(testServer, 'utf8')).replace(
+        'port:',
+        `minimalMode: ${minimalMode},port:`
+      )
     )
     appPort = await findPort()
     nextInstance = await initNextServerScript(
@@ -96,8 +100,9 @@ describe('fetch-cache', () => {
     fetchGetReqIndex = 0
     revalidateReqIndex = 0
     fetchCacheRequests = []
+    storeCacheItems = false
     fetchGetShouldError = false
-    fetchCacheServer = http.createServer((req, res) => {
+    fetchCacheServer = http.createServer(async (req, res) => {
       console.log(`fetch cache request ${req.url} ${req.method}`, req.headers)
       const parsedUrl = new URL(req.url || '/', 'http://n')
 
@@ -137,6 +142,19 @@ describe('fetch-cache', () => {
             res.end('internal server error')
             return
           }
+
+          if (storeCacheItems && fetchCacheStore.has(key)) {
+            console.log(`returned cache for ${key}`)
+            res.statusCode = 200
+            res.end(JSON.stringify(fetchCacheStore.get(key)))
+            return
+          }
+        }
+
+        if (type === 'post' && storeCacheItems) {
+          const body = await rawBody(req, { encoding: 'utf8' })
+          fetchCacheStore.set(key, JSON.parse(body.toString()))
+          console.log(`set cache for ${key}`)
         }
         res.statusCode = type === 'post' ? 200 : 404
         res.end(`${type} for ${key}`)
@@ -226,4 +244,39 @@ describe('fetch-cache', () => {
       fetchGetShouldError = false
     }
   })
+
+  it('should update cache TTL even if cache data does not change', async () => {
+    storeCacheItems = true
+    const fetchCacheRequestsIndex = fetchCacheRequests.length
+
+    try {
+      for (let i = 0; i < 3; i++) {
+        const res = await fetchViaHTTP(appPort, '/not-changed')
+        expect(res.status).toBe(200)
+        // give time for revalidate period to pass
+        await new Promise((resolve) => setTimeout(resolve, 3_000))
+      }
+
+      const newCacheGets = []
+      const newCacheSets = []
+
+      for (
+        let i = fetchCacheRequestsIndex - 1;
+        i < fetchCacheRequests.length;
+        i++
+      ) {
+        const requestItem = fetchCacheRequests[i]
+        if (requestItem.method === 'get') {
+          newCacheGets.push(requestItem)
+        }
+        if (requestItem.method === 'post') {
+          newCacheSets.push(requestItem)
+        }
+      }
+      expect(newCacheGets.length).toBeGreaterThanOrEqual(2)
+      expect(newCacheSets.length).toBeGreaterThanOrEqual(2)
+    } finally {
+      storeCacheItems = false
+    }
+  })
 })

test/turbopack-build-tests-manifest.json

@@ -1,17 +1,6 @@
 {
   "version": 2,
   "suites": {
-    "test/production/app-dir/fetch-cache/fetch-cache.test.ts": {
-      "passed": [],
-      "failed": [
-        "fetch-cache should have correct fetchUrl field for fetches and unstable_cache",
-        "fetch-cache should retry 3 times when revalidate times out",
-        "fetch-cache should not retry for failed fetch-cache GET"
-      ],
-      "pending": [],
-      "flakey": [],
-      "runtimeError": false
-    },
     "test/e2e/404-page-router/index.test.ts": {
       "passed": [
         "404-page-router 404-page-router with basePath of false and i18n of false and middleware false for /error should have the correct router parameters after it is ready",
@@ -14619,6 +14608,18 @@
       "flakey": [],
       "runtimeError": false
     },
+    "test/production/app-dir/fetch-cache/fetch-cache.test.ts": {
+      "passed": [],
+      "failed": [
+        "fetch-cache should have correct fetchUrl field for fetches and unstable_cache",
+        "fetch-cache should not retry for failed fetch-cache GET",
+        "fetch-cache should retry 3 times when revalidate times out",
+        "fetch-cache should update cache TTL even if cache data does not change"
+      ],
+      "pending": [],
+      "flakey": [],
+      "runtimeError": false
+    },
     "test/production/app-dir/mangle-reserved/mangle-reserved.test.ts": {
       "passed": [],
       "failed": ["mangle-reserved should preserve the name"],