feat: remove x-nextjs-cache header and add Cache-Status instead (#158)

* feat: remove x-nextjs-cache header and add Cache-Status instead

* chore: rename cache name from 'Next Cache' to 'Next.js'

* test: adjust test after removing x-nextjs-cache header and adding cache-status

* test: cache-status can have multiple values, adjust assertions for that

* test: e2e quirks worked out

---------

Co-authored-by: Rob Stanford <[email protected]>

Author: pieh

src/run/handlers/server.ts

@@ -5,6 +5,7 @@ import type { WorkerRequestHandler } from 'next/dist/server/lib/types.js'
 import { TagsManifest, getTagsManifest } from '../config.js'
 import {
   adjustDateHeader,
+  handleNextCacheHeader,
   setCacheControlHeaders,
   setCacheTagsHeaders,
   setVaryHeaders,
@@ -59,6 +60,7 @@ export default async (request: Request) => {
   setCacheControlHeaders(response.headers, request)
   setCacheTagsHeaders(response.headers, request, tagsManifest)
   setVaryHeaders(response.headers, request, nextConfig)
+  handleNextCacheHeader(response.headers)
 
   // Temporary workaround for an issue where sending a response with an empty
   // body causes an unhandled error. This doesn't catch everything, but redirects are the

src/run/headers.ts

@@ -135,3 +135,32 @@ export const setCacheTagsHeaders = (headers: Headers, request: Request, manifest
   const tags = manifest[path]
   headers.set('cache-tag', tags)
 }
+
+/**
+ * https://httpwg.org/specs/rfc9211.html
+ *
+ * We only should get HIT and MISS statuses from Next cache.
+ * We will ignore other statuses and will not set Cache-Status header in those cases.
+ */
+const NEXT_CACHE_TO_CACHE_STATUS: Record<string, string> = {
+  HIT: `hit`,
+  MISS: `miss,`,
+}
+
+/**
+ * x-nextjs-cache header will be confusing to users as very often it will be MISS
+ * even when the CDN serves cached the response. So we'll remove it and instead add
+ * a Cache-Status header for Next cache so users inspect that together with CDN cache status
+ * and not on its own.
+ */
+export const handleNextCacheHeader = (headers: Headers) => {
+  const nextCache = headers.get('x-nextjs-cache')
+  if (typeof nextCache === 'string') {
+    if (nextCache in NEXT_CACHE_TO_CACHE_STATUS) {
+      const cacheStatus = NEXT_CACHE_TO_CACHE_STATUS[nextCache]
+      headers.set('cache-status', `"Next.js"; ${cacheStatus}`)
+    }
+
+    headers.delete('x-nextjs-cache')
+  }
+}

tests/e2e/page-router.test.ts

@@ -12,10 +12,28 @@ test.afterAll(async ({}, testInfo) => {
 })
 
 test('Static revalidate works correctly', async ({ page }) => {
+  // this disables browser cache - otherwise If-None-Match header
+  // is added to repeat requests which result in actual 304 response
+  // with custom headers from function not being returned
+  // additionally Playwright wrongly report 304 as 200
+  // https://github.com/microsoft/playwright/issues/27573
+  // which makes the assertions confusing
+  // see https://playwright.dev/docs/api/class-browsercontext#browser-context-route
+  // > Enabling routing disables http cache.
+  // and https://stackoverflow.com/questions/68522170/playwright-disable-caching-of-webpage-so-i-can-fetch-new-elements-after-scrollin
+  // note - this is likely the same problem that cause assertions at the bottom to be commented out
+  // generally we shouldn't do that
+  page.route('**', (route) => route.continue())
+
   const response1 = await page.goto(new URL('static/revalidate-manual', ctx.url).href)
   const headers1 = response1?.headers() || {}
   expect(response1?.status()).toBe(200)
-  expect(headers1['x-nextjs-cache']).toBe('HIT')
+  expect(headers1['x-nextjs-cache']).toBeUndefined()
+  // first time hitting this route - we will invoke function and see
+  // Next cache hit status in the response because it was prerendered
+  // at build time
+  expect(headers1['cache-status']).toMatch(/"Netlify Edge"; fwd=miss/m)
+  expect(headers1['cache-status']).toMatch(/"Next.js"; hit/m)
 
   const date1 = await page.textContent('[data-testid="date-now"]')
   const h1 = await page.textContent('h1')
@@ -24,7 +42,11 @@ test('Static revalidate works correctly', async ({ page }) => {
   const response2 = await page.goto(new URL('static/revalidate-manual', ctx.url).href)
   const headers2 = response2?.headers() || {}
   expect(response2?.status()).toBe(200)
-  expect(headers2['x-nextjs-cache']).toBe('HIT')
+  expect(headers2['x-nextjs-cache']).toBeUndefined()
+  // On CDN hit, Next cache status is not added to response anymore
+  // (any cache-status set by functions is not added - this is a platform behavior
+  // not runtime behavior)
+  expect(headers2['cache-status']).toMatch(/"Netlify Edge"; hit/m)
 
   // the page is cached
   const date2 = await page.textContent('[data-testid="date-now"]')
@@ -40,7 +62,14 @@ test('Static revalidate works correctly', async ({ page }) => {
   const response3 = await page.goto(new URL('static/revalidate-manual', ctx.url).href)
   const headers3 = response3?.headers() || {}
   expect(response3?.status()).toBe(200)
-  expect(headers3['x-nextjs-cache']).toBe('HIT')
+  expect(headers3?.['x-nextjs-cache']).toBeUndefined()
+  // revalidate refreshes Next cache, but not CDN cache
+  // so our request after revalidation means that Next cache is already
+  // warmed up with fresh response, but CDN cache just knows that previously
+  // cached response is stale, so we are hitting our function that serve
+  // already cached response
+  expect(headers3['cache-status']).toMatch(/"Next.js"; hit/m)
+  expect(headers3['cache-status']).toMatch(/"Netlify Edge"; fwd=stale/m)
 
   // the page has now an updated date
   // TODO: Cache purge is currently not working as expected

tests/integration/cache-handler.test.ts

@@ -55,7 +55,7 @@ describe('page router', () => {
     expect(load(call1.body)('h1').text()).toBe('Show #71')
     expect(call1.headers, 'a cache hit on the first invocation of a prerendered page').toEqual(
       expect.objectContaining({
-        'x-nextjs-cache': 'HIT',
+        'cache-status': expect.stringMatching(/"Next.js"; hit/),
         'netlify-cdn-cache-control': 's-maxage=5, stale-while-revalidate=31536000',
       }),
     )
@@ -72,7 +72,7 @@ describe('page router', () => {
     expect(call2.statusCode).toBe(200)
     expect(call2.headers, 'a cache miss on a stale page').toEqual(
       expect.objectContaining({
-        'x-nextjs-cache': 'MISS',
+        'cache-status': expect.stringMatching(/"Next.js"; miss/),
       }),
     )
     expect(call2Date, 'the date was cached and is matching the initial one').not.toBe(call1Date)
@@ -84,7 +84,7 @@ describe('page router', () => {
 
     expect(callLater2.headers, 'date header matches the cached value').toEqual(
       expect.objectContaining({
-        'x-nextjs-cache': 'HIT',
+        'cache-status': expect.stringMatching(/"Next.js"; hit/),
         date: callLater.headers['date'],
       }),
     )
@@ -99,7 +99,7 @@ describe('page router', () => {
     expect(call3.statusCode).toBe(200)
     expect(call3.headers, 'a cache hit after dynamically regenerating the stale page').toEqual(
       expect.objectContaining({
-        'x-nextjs-cache': 'HIT',
+        'cache-status': expect.stringMatching(/"Next.js"; hit/),
       }),
     )
   })
@@ -131,7 +131,7 @@ describe('app router', () => {
     expect(load(post1.body)('h1').text()).toBe('Revalidate Fetch')
     expect(post1.headers, 'a cache hit on the first invocation of a prerendered page').toEqual(
       expect.objectContaining({
-        'x-nextjs-cache': 'HIT',
+        'cache-status': expect.stringMatching(/"Next.js"; hit/),
         'netlify-cdn-cache-control': 's-maxage=5, stale-while-revalidate=31536000',
       }),
     )
@@ -143,7 +143,7 @@ describe('app router', () => {
     expect(load(post3.body)('h1').text()).toBe('Revalidate Fetch')
     expect(post3.headers, 'a cache miss on a non prerendered page').toEqual(
       expect.objectContaining({
-        'x-nextjs-cache': 'MISS',
+        'cache-status': expect.stringMatching(/"Next.js"; miss/),
       }),
     )
 
@@ -157,7 +157,7 @@ describe('app router', () => {
     expect(stale.statusCode).toBe(200)
     expect(stale.headers, 'a cache miss on a stale page').toEqual(
       expect.objectContaining({
-        'x-nextjs-cache': 'MISS',
+        'cache-status': expect.stringMatching(/"Next.js"; miss/),
       }),
     )
     // it should have a new date rendered
@@ -173,7 +173,7 @@ describe('app router', () => {
     expect(staleDate, 'the date was not cached').toBe(cachedDate)
     expect(cached.headers, 'a cache hit after dynamically regenerating the stale page').toEqual(
       expect.objectContaining({
-        'x-nextjs-cache': 'HIT',
+        'cache-status': expect.stringMatching(/"Next.js"; hit/),
       }),
     )
   })
@@ -228,7 +228,7 @@ describe('route', () => {
     })
     expect(call1.headers, 'a cache hit on the first invocation of a prerendered route').toEqual(
       expect.objectContaining({
-        'x-nextjs-cache': 'HIT',
+        'cache-status': expect.stringMatching(/"Next.js"; hit/),
       }),
     )
     // wait to have a stale route
@@ -242,7 +242,7 @@ describe('route', () => {
     expect(call2Body).toMatchObject({ data: expect.objectContaining({ id: 1 }) })
     expect(call2.headers, 'a cache miss on a stale route').toEqual(
       expect.objectContaining({
-        'x-nextjs-cache': 'MISS',
+        'cache-status': expect.stringMatching(/"Next.js"; miss/),
       }),
     )
     expect(call1Time, 'the date is a new one on a stale route').not.toBe(call2Time)
@@ -257,7 +257,7 @@ describe('route', () => {
     expect(call3Body).toMatchObject({ data: expect.objectContaining({ id: 1 }) })
     expect(call3.headers, 'a cache hit after dynamically regenerating the stale  route').toEqual(
       expect.objectContaining({
-        'x-nextjs-cache': 'HIT',
+        'cache-status': expect.stringMatching(/"Next.js"; hit/),
       }),
     )
     expect(call3Time, 'the date was cached as well').toBe(call2Time)

tests/integration/fetch-handler.test.ts

@@ -111,7 +111,7 @@ test<FixtureTestContext>('if the fetch call is cached correctly', async (ctx) =>
   expect(post1Name).toBe('Under the Dome')
   expect(post1.headers, 'the page should be a miss').toEqual(
     expect.objectContaining({
-      'x-nextjs-cache': 'MISS',
+      'cache-status': expect.stringMatching(/"Next.js"; miss/),
     }),
   )
 

tests/integration/page-router.test.ts

@@ -70,7 +70,7 @@ test<FixtureTestContext>('Should revalidate path with On-demand Revalidation', a
   const dateCacheInitial = load(staticPageInitial.body)('[data-testid="date-now"]').text()
 
   expect(staticPageInitial.statusCode).toBe(200)
-  expect(staticPageInitial.headers?.['x-nextjs-cache']).toBe('HIT')
+  expect(staticPageInitial.headers?.['cache-status']).toMatch(/"Next.js"; hit/)
   const blobDataInitial = await ctx.blobStore.get(encodeBlobKey('static/revalidate-manual'), {
     type: 'json',
   })
@@ -91,7 +91,7 @@ test<FixtureTestContext>('Should revalidate path with On-demand Revalidation', a
   expect(blobDateInitial).not.toBe(blobDateRevalidated)
 
   const staticPageRevalidated = await invokeFunction(ctx, { url: '/static/revalidate-manual' })
-  expect(staticPageRevalidated.headers?.['x-nextjs-cache']).toBe('HIT')
+  expect(staticPageRevalidated.headers?.['cache-status']).toMatch(/"Next.js"; hit/)
   const dateCacheRevalidated = load(staticPageRevalidated.body)('[data-testid="date-now"]').text()
 
   console.log({ dateCacheInitial, dateCacheRevalidated })

tests/integration/revalidate-path.test.ts

@@ -47,15 +47,16 @@ test<FixtureTestContext>('should revalidate a route by path', async (ctx) => {
   expect(post1.statusCode).toBe(200)
   expect(post1Route2.statusCode).toBe(200)
   expect(load(post1.body)('h1').text()).toBe('Hello, Statically fetched show 1')
+
   expect(post1.headers, 'a cache hit on the first invocation of a prerendered page').toEqual(
     expect.objectContaining({
-      'x-nextjs-cache': 'HIT',
+      'cache-status': expect.stringMatching(/"Next.js"; hit/),
       'netlify-cdn-cache-control': 's-maxage=31536000, stale-while-revalidate=31536000',
     }),
   )
   expect(post1Route2.headers, 'a cache hit on the first invocation of a prerendered page').toEqual(
     expect.objectContaining({
-      'x-nextjs-cache': 'HIT',
+      'cache-status': expect.stringMatching(/"Next.js"; hit/),
       'netlify-cdn-cache-control': 's-maxage=31536000, stale-while-revalidate=31536000',
     }),
   )
@@ -80,13 +81,13 @@ test<FixtureTestContext>('should revalidate a route by path', async (ctx) => {
   expect(load(post2.body)('h1').text()).toBe('Hello, Statically fetched show 1')
   expect(post2.headers, 'a cache miss on the on demand revalidated path /1').toEqual(
     expect.objectContaining({
-      'x-nextjs-cache': 'MISS',
+      'cache-status': expect.stringMatching(/"Next.js"; miss/),
       'netlify-cdn-cache-control': 's-maxage=31536000, stale-while-revalidate=31536000',
     }),
   )
   expect(post2Route2.headers, 'a cache miss on the on demand revalidated path /2').toEqual(
     expect.objectContaining({
-      'x-nextjs-cache': 'MISS',
+      'cache-status': expect.stringMatching(/"Next.js"; miss/),
       'netlify-cdn-cache-control': 's-maxage=31536000, stale-while-revalidate=31536000',
     }),
   )

tests/integration/revalidate-tags.test.ts

@@ -39,7 +39,7 @@ test<FixtureTestContext>('should revalidate a route by tag', async (ctx) => {
   expect(load(post1.body)('h1').text()).toBe('Hello, Static Fetch 1')
   expect(post1.headers, 'a cache hit on the first invocation of a prerendered page').toEqual(
     expect.objectContaining({
-      'x-nextjs-cache': 'HIT',
+      'cache-status': expect.stringMatching(/"Next.js"; hit/),
       'netlify-cdn-cache-control': 's-maxage=31536000, stale-while-revalidate=31536000',
     }),
   )
@@ -57,7 +57,7 @@ test<FixtureTestContext>('should revalidate a route by tag', async (ctx) => {
   expect(load(post2.body)('h1').text()).toBe('Hello, Static Fetch 1')
   expect(post2.headers, 'a cache miss on the on demand revalidated page').toEqual(
     expect.objectContaining({
-      'x-nextjs-cache': 'MISS',
+      'cache-status': expect.stringMatching(/"Next.js"; miss/),
       'netlify-cdn-cache-control': 's-maxage=31536000, stale-while-revalidate=31536000',
     }),
   )
@@ -72,7 +72,7 @@ test<FixtureTestContext>('should revalidate a route by tag', async (ctx) => {
   expect(load(post3.body)('h1').text()).toBe('Hello, Static Fetch 1')
   expect(post3.headers, 'a cache hit on the revalidated and regenerated page').toEqual(
     expect.objectContaining({
-      'x-nextjs-cache': 'HIT',
+      'cache-status': expect.stringMatching(/"Next.js"; hit/),
       'netlify-cdn-cache-control': 's-maxage=31536000, stale-while-revalidate=31536000',
     }),
   )