feat: make CDN SWR background revalidation discard stale cache content in order to produce fresh responses (#2765)

* feat: make CDN SWR background revalidation discard stale cache content in order to produce fresh responses

* test: add e2e case testing storing fresh responses in cdn when handling background SWR requests

* fix: apply code review comments/suggestions

Author: pieh

Diff for: src/build/templates/handler-monorepo.tmpl.js

@@ -28,14 +28,15 @@ export default async function (req, context) {
         'site.id': context.site.id,
         'http.method': req.method,
         'http.target': req.url,
+        isBackgroundRevalidation: requestContext.isBackgroundRevalidation,
         monorepo: true,
         cwd: '{{cwd}}',
       })
       if (!cachedHandler) {
         const { default: handler } = await import('{{nextServerHandler}}')
         cachedHandler = handler
       }
-      const response = await cachedHandler(req, context)
+      const response = await cachedHandler(req, context, span, requestContext)
       span.setAttributes({
         'http.status_code': response.status,
       })

Diff for: src/build/templates/handler.tmpl.js

@@ -25,10 +25,11 @@ export default async function handler(req, context) {
         'site.id': context.site.id,
         'http.method': req.method,
         'http.target': req.url,
+        isBackgroundRevalidation: requestContext.isBackgroundRevalidation,
         monorepo: false,
         cwd: process.cwd(),
       })
-      const response = await serverHandler(req, context)
+      const response = await serverHandler(req, context, span, requestContext)
       span.setAttributes({
         'http.status_code': response.status,
       })

Diff for: src/run/handlers/cache.cts

@@ -52,6 +52,29 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions {
     return await encodeBlobKey(key)
   }
 
+  private getTTL(blob: NetlifyCacheHandlerValue) {
+    if (
+      blob.value?.kind === 'FETCH' ||
+      blob.value?.kind === 'ROUTE' ||
+      blob.value?.kind === 'APP_ROUTE' ||
+      blob.value?.kind === 'PAGE' ||
+      blob.value?.kind === 'PAGES' ||
+      blob.value?.kind === 'APP_PAGE'
+    ) {
+      const { revalidate } = blob.value
+
+      if (typeof revalidate === 'number') {
+        const revalidateAfter = revalidate * 1_000 + blob.lastModified
+        return (revalidateAfter - Date.now()) / 1_000
+      }
+      if (revalidate === false) {
+        return 'PERMANENT'
+      }
+    }
+
+    return 'NOT SET'
+  }
+
   private captureResponseCacheLastModified(
     cacheValue: NetlifyCacheHandlerValue,
     key: string,
@@ -219,10 +242,31 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions {
         return null
       }
 
+      const ttl = this.getTTL(blob)
+
+      if (getRequestContext()?.isBackgroundRevalidation && typeof ttl === 'number' && ttl < 0) {
+        // background revalidation request should allow data that is not yet stale,
+        // but opt to discard STALE data, so that Next.js generate fresh response
+        span.addEvent('Discarding stale entry due to SWR background revalidation request', {
+          key,
+          blobKey,
+          ttl,
+        })
+        getLogger()
+          .withFields({
+            ttl,
+            key,
+          })
+          .debug(
+            `[NetlifyCacheHandler.get] Discarding stale entry due to SWR background revalidation request: ${key}`,
+          )
+        return null
+      }
+
       const staleByTags = await this.checkCacheEntryStaleByTags(blob, ctx.tags, ctx.softTags)
 
       if (staleByTags) {
-        span.addEvent('Stale', { staleByTags })
+        span.addEvent('Stale', { staleByTags, key, blobKey, ttl })
         return null
       }
 
@@ -231,7 +275,11 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions {
 
       switch (blob.value?.kind) {
         case 'FETCH':
-          span.addEvent('FETCH', { lastModified: blob.lastModified, revalidate: ctx.revalidate })
+          span.addEvent('FETCH', {
+            lastModified: blob.lastModified,
+            revalidate: ctx.revalidate,
+            ttl,
+          })
           return {
             lastModified: blob.lastModified,
             value: blob.value,
@@ -242,6 +290,8 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions {
           span.addEvent(blob.value?.kind, {
             lastModified: blob.lastModified,
             status: blob.value.status,
+            revalidate: blob.value.revalidate,
+            ttl,
           })
 
           const valueWithoutRevalidate = this.captureRouteRevalidateAndRemoveFromObject(blob.value)
@@ -256,10 +306,10 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions {
         }
         case 'PAGE':
         case 'PAGES': {
-          span.addEvent(blob.value?.kind, { lastModified: blob.lastModified })
-
           const { revalidate, ...restOfPageValue } = blob.value
 
+          span.addEvent(blob.value?.kind, { lastModified: blob.lastModified, revalidate, ttl })
+
           await this.injectEntryToPrerenderManifest(key, revalidate)
 
           return {
@@ -268,10 +318,10 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions {
           }
         }
         case 'APP_PAGE': {
-          span.addEvent(blob.value?.kind, { lastModified: blob.lastModified })
-
           const { revalidate, rscData, ...restOfPageValue } = blob.value
 
+          span.addEvent(blob.value?.kind, { lastModified: blob.lastModified, revalidate, ttl })
+
           await this.injectEntryToPrerenderManifest(key, revalidate)
 
           return {

Diff for: src/run/handlers/request-context.cts

@@ -13,6 +13,10 @@ export interface FutureContext extends Context {
 }
 
 export type RequestContext = {
+  /**
+   * Determine if this request is for CDN SWR background revalidation
+   */
+  isBackgroundRevalidation: boolean
   captureServerTiming: boolean
   responseCacheGetLastModified?: number
   responseCacheKey?: string
@@ -41,7 +45,20 @@ type RequestContextAsyncLocalStorage = AsyncLocalStorage<RequestContext>
 export function createRequestContext(request?: Request, context?: FutureContext): RequestContext {
   const backgroundWorkPromises: Promise<unknown>[] = []
 
+  const isDebugRequest =
+    request?.headers.has('x-nf-debug-logging') || request?.headers.has('x-next-debug-logging')
+
+  const logger = systemLogger.withLogLevel(isDebugRequest ? LogLevel.Debug : LogLevel.Log)
+
+  const isBackgroundRevalidation =
+    request?.headers.get('netlify-invocation-source') === 'background-revalidation'
+
+  if (isBackgroundRevalidation) {
+    logger.debug('[NetlifyNextRuntime] Background revalidation request')
+  }
+
   return {
+    isBackgroundRevalidation,
     captureServerTiming: request?.headers.has('x-next-debug-logging') ?? false,
     trackBackgroundWork: (promise) => {
       if (context?.waitUntil) {
@@ -53,11 +70,7 @@ export function createRequestContext(request?: Request, context?: FutureContext)
     get backgroundWorkPromise() {
       return Promise.allSettled(backgroundWorkPromises)
     },
-    logger: systemLogger.withLogLevel(
-      request?.headers.has('x-nf-debug-logging') || request?.headers.has('x-next-debug-logging')
-        ? LogLevel.Debug
-        : LogLevel.Log,
-    ),
+    logger,
   }
 }
 

Diff for: src/run/handlers/server.ts

@@ -1,6 +1,8 @@
 import type { OutgoingHttpHeaders } from 'http'
 
 import { ComputeJsOutgoingMessage, toComputeResponse, toReqRes } from '@fastly/http-compute-js'
+import type { Context } from '@netlify/functions'
+import { Span } from '@opentelemetry/api'
 import type { NextConfigComplete } from 'next/dist/server/config-shared.js'
 import type { WorkerRequestHandler } from 'next/dist/server/lib/types.js'
 
@@ -13,7 +15,7 @@ import {
 } from '../headers.js'
 import { nextResponseProxy } from '../revalidate.js'
 
-import { createRequestContext, getLogger, getRequestContext } from './request-context.cjs'
+import { getLogger, type RequestContext } from './request-context.cjs'
 import { getTracer } from './tracer.cjs'
 import { setupWaitUntil } from './wait-until.cjs'
 
@@ -46,7 +48,12 @@ const disableFaultyTransferEncodingHandling = (res: ComputeJsOutgoingMessage) =>
   }
 }
 
-export default async (request: Request) => {
+export default async (
+  request: Request,
+  _context: Context,
+  topLevelSpan: Span,
+  requestContext: RequestContext,
+) => {
   const tracer = getTracer()
 
   if (!nextHandler) {
@@ -85,8 +92,6 @@ export default async (request: Request) => {
 
     disableFaultyTransferEncodingHandling(res as unknown as ComputeJsOutgoingMessage)
 
-    const requestContext = getRequestContext() ?? createRequestContext()
-
     const resProxy = nextResponseProxy(res, requestContext)
 
     // We don't await this here, because it won't resolve until the response is finished.
@@ -103,15 +108,31 @@ export default async (request: Request) => {
     const response = await toComputeResponse(resProxy)
 
     if (requestContext.responseCacheKey) {
-      span.setAttribute('responseCacheKey', requestContext.responseCacheKey)
+      topLevelSpan.setAttribute('responseCacheKey', requestContext.responseCacheKey)
     }
 
-    await adjustDateHeader({ headers: response.headers, request, span, tracer, requestContext })
+    const nextCache = response.headers.get('x-nextjs-cache')
+    const isServedFromNextCache = nextCache === 'HIT' || nextCache === 'STALE'
+
+    topLevelSpan.setAttributes({
+      'x-nextjs-cache': nextCache ?? undefined,
+      isServedFromNextCache,
+    })
+
+    if (isServedFromNextCache) {
+      await adjustDateHeader({
+        headers: response.headers,
+        request,
+        span,
+        tracer,
+        requestContext,
+      })
+    }
 
     setCacheControlHeaders(response, request, requestContext, nextConfig)
     setCacheTagsHeaders(response.headers, requestContext)
     setVaryHeaders(response.headers, request, nextConfig)
-    setCacheStatusHeader(response.headers)
+    setCacheStatusHeader(response.headers, nextCache)
 
     async function waitForBackgroundWork() {
       // it's important to keep the stream open until the next handler has finished

Diff for: src/run/headers.ts

@@ -137,17 +137,6 @@ export const adjustDateHeader = async ({
   tracer: RuntimeTracer
   requestContext: RequestContext
 }) => {
-  const cacheState = headers.get('x-nextjs-cache')
-  const isServedFromCache = cacheState === 'HIT' || cacheState === 'STALE'
-
-  span.setAttributes({
-    'x-nextjs-cache': cacheState ?? undefined,
-    isServedFromCache,
-  })
-
-  if (!isServedFromCache) {
-    return
-  }
   const key = new URL(request.url).pathname
 
   let lastModified: number | undefined
@@ -317,8 +306,7 @@ const NEXT_CACHE_TO_CACHE_STATUS: Record<string, string> = {
  * a Cache-Status header for Next cache so users inspect that together with CDN cache status
  * and not on its own.
  */
-export const setCacheStatusHeader = (headers: Headers) => {
-  const nextCache = headers.get('x-nextjs-cache')
+export const setCacheStatusHeader = (headers: Headers, nextCache: string | null) => {
   if (typeof nextCache === 'string') {
     if (nextCache in NEXT_CACHE_TO_CACHE_STATUS) {
       const cacheStatus = NEXT_CACHE_TO_CACHE_STATUS[nextCache]

Diff for: src/shared/cache-types.cts

@@ -78,7 +78,9 @@ type CachedRouteValueToNetlify<T> = T extends CachedRouteValue
       ? NetlifyCachedAppPageValue
       : T
 
-type MapCachedRouteValueToNetlify<T> = { [K in keyof T]: CachedRouteValueToNetlify<T[K]> }
+type MapCachedRouteValueToNetlify<T> = { [K in keyof T]: CachedRouteValueToNetlify<T[K]> } & {
+  lastModified: number
+}
 
 /**
  * Used for storing in blobs and reading from blobs

Diff for: tests/e2e/page-router.test.ts

@@ -390,6 +390,63 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => {
     expect(beforeFetch.localeCompare(date2)).toBeLessThan(0)
   })
 
+  test('Background SWR invocations can store fresh responses in CDN cache', async ({
+    page,
+    pageRouter,
+  }) => {
+    const slug = Date.now()
+    const pathname = `/revalidate-60/${slug}`
+
+    const beforeFirstFetch = new Date().toISOString()
+
+    const response1 = await page.goto(new URL(pathname, pageRouter.url).href)
+    expect(response1?.status()).toBe(200)
+    expect(response1?.headers()['cache-status']).toMatch(
+      /"Netlify (Edge|Durable)"; fwd=(uri-miss(; stored)?|miss)/m,
+    )
+    expect(response1?.headers()['netlify-cdn-cache-control']).toMatch(
+      /s-maxage=60, stale-while-revalidate=[0-9]+, durable/,
+    )
+
+    // ensure response was NOT produced before invocation
+    const date1 = (await page.textContent('[data-testid="date-now"]')) ?? ''
+    expect(date1.localeCompare(beforeFirstFetch)).toBeGreaterThan(0)
+
+    // allow page to get stale
+    await page.waitForTimeout(60_000)
+
+    const response2 = await page.goto(new URL(pathname, pageRouter.url).href)
+    expect(response2?.status()).toBe(200)
+    expect(response2?.headers()['cache-status']).toMatch(
+      /"Netlify (Edge|Durable)"; hit; fwd=stale/m,
+    )
+    expect(response2?.headers()['netlify-cdn-cache-control']).toMatch(
+      /s-maxage=60, stale-while-revalidate=[0-9]+, durable/,
+    )
+
+    const date2 = (await page.textContent('[data-testid="date-now"]')) ?? ''
+    expect(date2).toBe(date1)
+
+    // wait a bit to ensure background work has a chance to finish
+    // (it should take at least 5 seconds to regenerate, so we should wait at least that much to get fresh response)
+    await page.waitForTimeout(10_000)
+
+    // subsequent request should be served with fresh response from cdn cache, as previous request
+    // should result in background SWR invocation that serves fresh response that was stored in CDN cache
+    const response3 = await page.goto(new URL(pathname, pageRouter.url).href)
+    expect(response3?.status()).toBe(200)
+    expect(response3?.headers()['cache-status']).toMatch(
+      // hit, without being followed by ';fwd=stale'
+      /"Netlify (Edge|Durable)"; hit(?!; fwd=stale)/m,
+    )
+    expect(response3?.headers()['netlify-cdn-cache-control']).toMatch(
+      /s-maxage=60, stale-while-revalidate=[0-9]+, durable/,
+    )
+
+    const date3 = (await page.textContent('[data-testid="date-now"]')) ?? ''
+    expect(date3.localeCompare(date2)).toBeGreaterThan(0)
+  })
+
   test('should serve 404 page when requesting non existing page (no matching route)', async ({
     page,
     pageRouter,