Add timeout/retry handling for fetch cache (#66652)

As discussed this adds handling to timeout at a max of 500ms for fetch
cache request and retries a max of 3 times due to network instability.
This also adds cache service tests and fixes a case we've been trying to
track down where we were seeing `undefined` cache URL values which made
debugging fetches tricky.

packages/next/src/server/base-server.ts

@@ -2792,7 +2792,6 @@ export default abstract class Server<
             kind: 'PAGE',
             html: RenderResult.fromStatic(''),
             pageData: {},
-            postponed: undefined,
             headers: undefined,
             status: undefined,
           },

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

@@ -24,10 +24,40 @@ const CACHE_REVALIDATE_HEADER = 'x-vercel-revalidate' as const
 const CACHE_FETCH_URL_HEADER = 'x-vercel-cache-item-name' as const
 const CACHE_CONTROL_VALUE_HEADER = 'x-vercel-cache-control' as const
 
+const DEBUG = Boolean(process.env.NEXT_PRIVATE_DEBUG_CACHE)
+
+async function fetchRetryWithTimeout(
+  url: Parameters<typeof fetch>[0],
+  init: Parameters<typeof fetch>[1],
+  retryIndex = 0
+): Promise<Response> {
+  const controller = new AbortController()
+  const timeout = setTimeout(() => {
+    controller.abort()
+  }, 500)
+
+  return fetch(url, {
+    ...(init || {}),
+    signal: controller.signal,
+  })
+    .catch((err) => {
+      if (retryIndex === 3) {
+        throw err
+      } else {
+        if (DEBUG) {
+          console.log(`Fetch failed for ${url} retry ${retryIndex}`)
+        }
+        return fetchRetryWithTimeout(url, init, retryIndex + 1)
+      }
+    })
+    .finally(() => {
+      clearTimeout(timeout)
+    })
+}
+
 export default class FetchCache implements CacheHandler {
   private headers: Record<string, string>
   private cacheEndpoint?: string
-  private debug: boolean
 
   private hasMatchingTags(arr1: string[], arr2: string[]) {
     if (arr1.length !== arr2.length) return false
@@ -53,7 +83,6 @@ export default class FetchCache implements CacheHandler {
   }
 
   constructor(ctx: CacheHandlerContext) {
-    this.debug = !!process.env.NEXT_PRIVATE_DEBUG_CACHE
     this.headers = {}
     this.headers['Content-Type'] = 'application/json'
 
@@ -79,17 +108,18 @@ export default class FetchCache implements CacheHandler {
     }
 
     if (scHost) {
-      this.cacheEndpoint = `https://${scHost}${scBasePath || ''}`
-      if (this.debug) {
+      const scProto = process.env.SUSPENSE_CACHE_PROTO || 'https'
+      this.cacheEndpoint = `${scProto}://${scHost}${scBasePath || ''}`
+      if (DEBUG) {
         console.log('using cache endpoint', this.cacheEndpoint)
       }
-    } else if (this.debug) {
+    } else if (DEBUG) {
       console.log('no cache endpoint available')
     }
 
     if (ctx.maxMemoryCacheSize) {
       if (!memoryCache) {
-        if (this.debug) {
+        if (DEBUG) {
           console.log('using memory store for fetch cache')
         }
 
@@ -118,7 +148,7 @@ export default class FetchCache implements CacheHandler {
         })
       }
     } else {
-      if (this.debug) {
+      if (DEBUG) {
         console.log('not using memory store for fetch cache')
       }
     }
@@ -133,21 +163,21 @@ export default class FetchCache implements CacheHandler {
   ) {
     let [tags] = args
     tags = typeof tags === 'string' ? [tags] : tags
-    if (this.debug) {
+    if (DEBUG) {
       console.log('revalidateTag', tags)
     }
 
     if (!tags.length) return
 
     if (Date.now() < rateLimitedUntil) {
-      if (this.debug) {
+      if (DEBUG) {
         console.log('rate limited ', rateLimitedUntil)
       }
       return
     }
 
     try {
-      const res = await fetch(
+      const res = await fetchRetryWithTimeout(
         `${this.cacheEndpoint}/v1/suspense-cache/revalidate?tags=${tags
           .map((tag) => encodeURIComponent(tag))
           .join(',')}`,
@@ -181,7 +211,7 @@ export default class FetchCache implements CacheHandler {
     }
 
     if (Date.now() < rateLimitedUntil) {
-      if (this.debug) {
+      if (DEBUG) {
         console.log('rate limited')
       }
       return null
@@ -227,7 +257,7 @@ export default class FetchCache implements CacheHandler {
         }
 
         if (res.status === 404) {
-          if (this.debug) {
+          if (DEBUG) {
             console.log(
               `no fetch cache entry for ${key}, duration: ${
                 Date.now() - start
@@ -245,7 +275,7 @@ export default class FetchCache implements CacheHandler {
         const cached: IncrementalCacheValue = await res.json()
 
         if (!cached || cached.kind !== 'FETCH') {
-          this.debug && console.log({ cached })
+          DEBUG && console.log({ cached })
           throw new Error('invalid cache value')
         }
 
@@ -272,7 +302,7 @@ export default class FetchCache implements CacheHandler {
               : Date.now() - parseInt(age || '0', 10) * 1000,
         }
 
-        if (this.debug) {
+        if (DEBUG) {
           console.log(
             `got fetch cache entry for ${key}, duration: ${
               Date.now() - start
@@ -289,7 +319,7 @@ export default class FetchCache implements CacheHandler {
         }
       } catch (err) {
         // unable to get data from fetch-cache
-        if (this.debug) {
+        if (DEBUG) {
           console.error(`Failed to get from fetch-cache`, err)
         }
       }
@@ -314,7 +344,7 @@ export default class FetchCache implements CacheHandler {
           JSON.stringify((newValue as Record<string, string | Object>)[field])
       )
     ) {
-      if (this.debug) {
+      if (DEBUG) {
         console.log(`skipping cache set for ${key} as not modified`)
       }
       return
@@ -324,7 +354,7 @@ export default class FetchCache implements CacheHandler {
     if (!fetchCache) return
 
     if (Date.now() < rateLimitedUntil) {
-      if (this.debug) {
+      if (DEBUG) {
         console.log('rate limited')
       }
       return
@@ -356,7 +386,7 @@ export default class FetchCache implements CacheHandler {
           tags: undefined,
         })
 
-        if (this.debug) {
+        if (DEBUG) {
           console.log('set cache', key)
         }
         const fetchParams: NextFetchCacheParams = {
@@ -385,11 +415,11 @@ export default class FetchCache implements CacheHandler {
         }
 
         if (!res.ok) {
-          this.debug && console.log(await res.text())
+          DEBUG && console.log(await res.text())
           throw new Error(`invalid response ${res.status}`)
         }
 
-        if (this.debug) {
+        if (DEBUG) {
           console.log(
             `successfully set to fetch-cache for ${key}, duration: ${
               Date.now() - start
@@ -398,7 +428,7 @@ export default class FetchCache implements CacheHandler {
         }
       } catch (err) {
         // unable to set to fetch-cache
-        if (this.debug) {
+        if (DEBUG) {
           console.error(`Failed to update fetch cache`, err)
         }
       }

packages/next/src/server/web/spec-extension/unstable-cache.ts

@@ -113,7 +113,7 @@ export function unstable_cache<T extends Callback>(
       return a.localeCompare(b)
     })
     const sortedSearch = sortedSearchKeys
-      .map((key) => searchParams.get(key))
+      .map((key) => `${key}=${searchParams.get(key)}`)
       .join('&')
 
     // Construct the complete cache key for this function invocation
@@ -180,6 +180,7 @@ export function unstable_cache<T extends Callback>(
           tags,
           softTags: implicitTags,
           fetchIdx,
+          fetchUrl,
         })
 
         if (cacheEntry && cacheEntry.value) {
@@ -276,10 +277,17 @@ export function unstable_cache<T extends Callback>(
       if (!incrementalCache.isOnDemandRevalidate) {
         // We aren't doing an on demand revalidation so we check use the cache if valid
 
+        // @TODO check on this API. addImplicitTags mutates the store and returns the implicit tags. The naming
+        // of this function is potentially a little confusing
+        const implicitTags = store && addImplicitTags(store)
+
         const cacheEntry = await incrementalCache.get(cacheKey, {
           kindHint: 'fetch',
           revalidate: options.revalidate,
           tags,
+          fetchIdx,
+          fetchUrl,
+          softTags: implicitTags,
         })
 
         if (cacheEntry && cacheEntry.value) {

test/ppr-tests-manifest.json

@@ -1,6 +1,17 @@
 {
   "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/app-dir/app-static/app-static.test.ts": {
       "failed": [
         "app-dir static/dynamic handling usePathname should have values from canonical url on rewrite",

test/production/app-dir/fetch-cache/app/api/revalidate/route.ts

@@ -0,0 +1,9 @@
+import { revalidateTag } from 'next/cache'
+import { NextRequest, NextResponse } from 'next/server'
+
+export const dynamic = 'force-dynamic'
+
+export function GET(req: NextRequest) {
+  revalidateTag('thankyounext')
+  return NextResponse.json({ done: true })
+}

test/production/app-dir/fetch-cache/app/layout.tsx

@@ -0,0 +1,8 @@
+import { ReactNode } from 'react'
+export default function Root({ children }: { children: ReactNode }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}

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

@@ -0,0 +1,32 @@
+import { unstable_cache } from 'next/cache'
+
+export const dynamic = 'force-dynamic'
+export const fetchCache = 'default-cache'
+
+const getCachedRandom = unstable_cache(
+  async () => {
+    return Math.random()
+  },
+  [],
+  {
+    revalidate: 3,
+    tags: ['thankyounext'],
+  }
+)
+
+export default async function Page() {
+  const data = await fetch(
+    'https://next-data-api-endpoint.vercel.app/api/random?a=b',
+    { next: { tags: ['thankyounext'], revalidate: 3 } }
+  ).then((res) => res.text())
+
+  const cachedRandom = getCachedRandom()
+
+  return (
+    <>
+      <p>hello world</p>
+      <p id="data">{data}</p>
+      <p id="random">{cachedRandom}</p>
+    </>
+  )
+}

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

@@ -0,0 +1,240 @@
+import glob from 'glob'
+import http from 'http'
+import fs from 'fs-extra'
+import { join } from 'path'
+import { FileRef, NextInstance, createNext } from 'e2e-utils'
+import {
+  retry,
+  killApp,
+  findPort,
+  fetchViaHTTP,
+  initNextServerScript,
+} from 'next-test-utils'
+
+describe('fetch-cache', () => {
+  let next: NextInstance
+  let appPort: any
+  let cliOuptut = ''
+  let nextInstance: any
+  let fetchGetReqIndex = 0
+  let revalidateReqIndex = 0
+  let fetchGetShouldError = false
+  let fetchCacheServer: http.Server
+  let fetchCacheRequests: Array<{
+    url: string
+    method: string
+    headers: Record<string, string | string[]>
+  }> = []
+  let fetchCacheEnv: Record<string, string> = {
+    SUSPENSE_CACHE_PROTO: 'http',
+  }
+
+  const setupNext = async ({
+    nextEnv,
+    minimalMode,
+  }: {
+    nextEnv?: boolean
+    minimalMode?: boolean
+  }) => {
+    // test build against environment with next support
+    process.env.NOW_BUILDER = nextEnv ? '1' : ''
+
+    next = await createNext({
+      files: {
+        app: new FileRef(join(__dirname, 'app')),
+      },
+      nextConfig: {
+        eslint: {
+          ignoreDuringBuilds: true,
+        },
+        output: 'standalone',
+      },
+    })
+    await next.stop()
+
+    await fs.move(
+      join(next.testDir, '.next/standalone'),
+      join(next.testDir, 'standalone')
+    )
+    for (const file of await fs.readdir(next.testDir)) {
+      if (file !== 'standalone') {
+        await fs.remove(join(next.testDir, file))
+        console.log('removed', file)
+      }
+    }
+    const files = glob.sync('**/*', {
+      cwd: join(next.testDir, 'standalone/.next/server/pages'),
+      dot: true,
+    })
+
+    for (const file of files) {
+      if (file.endsWith('.json') || file.endsWith('.html')) {
+        await fs.remove(join(next.testDir, '.next/server', file))
+      }
+    }
+
+    const testServer = join(next.testDir, 'standalone/server.js')
+    await fs.writeFile(
+      testServer,
+      (await fs.readFile(testServer, 'utf8')).replace(
+        'port:',
+        `minimalMode: ${minimalMode},port:`
+      )
+    )
+    appPort = await findPort()
+    nextInstance = await initNextServerScript(
+      testServer,
+      /- Local:/,
+      {
+        ...process.env,
+        ...fetchCacheEnv,
+        PORT: appPort,
+      },
+      undefined,
+      {
+        cwd: next.testDir,
+        onStderr(data) {
+          cliOuptut += data
+        },
+        onStdout(data) {
+          cliOuptut += data
+        },
+      }
+    )
+  }
+
+  beforeAll(async () => {
+    fetchGetReqIndex = 0
+    revalidateReqIndex = 0
+    fetchCacheRequests = []
+    fetchGetShouldError = false
+    fetchCacheServer = http.createServer((req, res) => {
+      console.log(`fetch cache request ${req.url} ${req.method}`, req.headers)
+      const parsedUrl = new URL(req.url || '/', 'http://n')
+
+      fetchCacheRequests.push({
+        url: req.url,
+        method: req.method?.toLowerCase(),
+        headers: req.headers,
+      })
+
+      if (parsedUrl.pathname === '/v1/suspense-cache/revalidate') {
+        revalidateReqIndex += 1
+        // timeout unless it's 3rd retry
+        const shouldTimeout = revalidateReqIndex % 3 !== 0
+
+        if (shouldTimeout) {
+          console.log('not responding for', req.url, { revalidateReqIndex })
+          return
+        }
+        res.statusCode = 200
+        res.end(`revalidated ${parsedUrl.searchParams.get('tags')}`)
+        return
+      }
+      const keyMatches = parsedUrl.pathname.match(
+        /\/v1\/suspense-cache\/(.*?)\/?$/
+      )
+      const key = keyMatches?.[0]
+
+      if (key) {
+        const type = req.method?.toLowerCase()
+        console.log(`got ${type} for ${key}`)
+
+        if (type === 'get') {
+          fetchGetReqIndex += 1
+
+          if (fetchGetShouldError) {
+            res.statusCode = 500
+            res.end('internal server error')
+            return
+          }
+        }
+        res.statusCode = type === 'post' ? 200 : 404
+        res.end(`${type} for ${key}`)
+        return
+      }
+      res.statusCode = 404
+      res.end('not found')
+    })
+    await new Promise<void>(async (resolve) => {
+      let fetchCachePort = await findPort()
+      fetchCacheServer.listen(fetchCachePort, () => {
+        fetchCacheEnv['SUSPENSE_CACHE_URL'] = `[::]:${fetchCachePort}`
+        console.log(
+          `Started fetch cache server at http://${fetchCacheEnv['SUSPENSE_CACHE_URL']}`
+        )
+        resolve()
+      })
+    })
+    await setupNext({ nextEnv: true, minimalMode: true })
+  })
+  afterAll(async () => {
+    await next.destroy()
+    if (fetchCacheServer) fetchCacheServer.close()
+    if (nextInstance) await killApp(nextInstance)
+  })
+
+  it('should have correct fetchUrl field for fetches and unstable_cache', async () => {
+    const res = await fetchViaHTTP(appPort, '/?myKey=myValue')
+    const html = await res.text()
+
+    expect(res.status).toBe(200)
+    expect(html).toContain('hello world')
+
+    const fetchUrlHeader = 'x-vercel-cache-item-name'
+    const fetchTagsHeader = 'x-vercel-cache-tags'
+    const fetchSoftTagsHeader = 'x-next-cache-soft-tags'
+    const unstableCacheSet = fetchCacheRequests.find((item) => {
+      return (
+        item.method === 'get' &&
+        item.headers[fetchUrlHeader]?.includes('unstable_cache')
+      )
+    })
+    const fetchSet = fetchCacheRequests.find((item) => {
+      return (
+        item.method === 'get' &&
+        item.headers[fetchUrlHeader]?.includes('next-data-api-endpoint')
+      )
+    })
+
+    expect(unstableCacheSet.headers[fetchUrlHeader]).toMatch(
+      /unstable_cache \/\?myKey=myValue .*?/
+    )
+    expect(unstableCacheSet.headers[fetchTagsHeader]).toBe('thankyounext')
+    expect(unstableCacheSet.headers[fetchSoftTagsHeader]).toBe(
+      '_N_T_/layout,_N_T_/page,_N_T_/'
+    )
+    expect(fetchSet.headers[fetchUrlHeader]).toBe(
+      'https://next-data-api-endpoint.vercel.app/api/random?a=b'
+    )
+    expect(fetchSet.headers[fetchSoftTagsHeader]).toBe(
+      '_N_T_/layout,_N_T_/page,_N_T_/'
+    )
+    expect(fetchSet.headers[fetchTagsHeader]).toBe('thankyounext')
+  })
+
+  it('should retry 3 times when revalidate times out', async () => {
+    await fetchViaHTTP(appPort, '/api/revalidate')
+
+    await retry(() => {
+      expect(revalidateReqIndex).toBe(3)
+    })
+    expect(cliOuptut).not.toContain('Failed to revalidate')
+    expect(cliOuptut).not.toContain('Error')
+  })
+
+  it('should not retry for failed fetch-cache GET', async () => {
+    fetchGetShouldError = true
+    const fetchGetReqIndexStart = fetchGetReqIndex
+
+    try {
+      await fetchViaHTTP(appPort, '/api/revalidate')
+      const res = await fetchViaHTTP(appPort, '/')
+      expect(res.status).toBe(200)
+      expect(await res.text()).toContain('hello world')
+      expect(fetchGetReqIndex).toBe(fetchGetReqIndexStart + 2)
+    } finally {
+      fetchGetShouldError = false
+    }
+  })
+})

test/turbopack-build-tests-manifest.json

@@ -1,6 +1,17 @@
 {
   "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",