fix: set correct date header for cached objects (#124)

* fix: set correct date header for cached objects

* Document the header handling changes

* Remove test.only

* Mock blob store in headers tests

* Fix test

* Instantiate blob store at request time

* Instantiate store per-request

* Update src/run/handlers/server.ts

Co-authored-by: Lukas Holzer <lukas.holzer@typeflow.cc>

* Update src/run/headers.test.ts

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

* chore: lint

---------

Co-authored-by: Lukas Holzer <lukas.holzer@typeflow.cc>
Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

Author: 4 people

src/run/handlers/server.ts

@@ -4,7 +4,12 @@ import type { NextConfigComplete } from 'next/dist/server/config-shared.js'
 import type { WorkerRequestHandler } from 'next/dist/server/lib/types.js'
 
 import { TagsManifest, getTagsManifest } from '../config.js'
-import { setCacheControlHeaders, setCacheTagsHeaders, setVaryHeaders } from '../headers.js'
+import {
+  adjustDateHeader,
+  setCacheControlHeaders,
+  setCacheTagsHeaders,
+  setVaryHeaders,
+} from '../headers.js'
 import { nextResponseProxy } from '../revalidate.js'
 
 let nextHandler: WorkerRequestHandler, nextConfig: NextConfigComplete, tagsManifest: TagsManifest
@@ -30,15 +35,6 @@ export default async (request: Request) => {
 
   const resProxy = nextResponseProxy(res)
 
-  resProxy.prependListener('_headersSent', (event: HeadersSentEvent) => {
-    const headers = new Headers(event.headers)
-    setCacheControlHeaders(headers)
-    setCacheTagsHeaders(headers, request, tagsManifest)
-    setVaryHeaders(headers, request, nextConfig)
-    event.headers = Object.fromEntries(headers.entries())
-    // console.log('Modified response headers:', JSON.stringify(event.headers, null, 2))
-  })
-
   // temporary workaround for https://linear.app/netlify/issue/ADN-111/
   delete req.headers['accept-encoding']
 
@@ -51,13 +47,16 @@ export default async (request: Request) => {
     resProxy.end('Internal Server Error')
   }
 
-  // log the response from Next.js
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars
-  const response = {
-    headers: resProxy.getHeaders(),
-    statusCode: resProxy.statusCode,
-  }
-  // console.log('Next server response:', JSON.stringify(response, null, 2))
+  // Contrary to the docs, this resolves when the headers are available, not when the stream closes.
+  // See https://github.com/fastly/http-compute-js/blob/main/src/http-compute-js/http-server.ts#L168-L173
+  const response = await toComputeResponse(resProxy)
+
+  await adjustDateHeader(response.headers, request)
+
+  setCacheControlHeaders(response.headers)
+  setCacheTagsHeaders(response.headers, request, tagsManifest)
+  setVaryHeaders(response.headers, request, nextConfig)
+
 
-  return toComputeResponse(resProxy)
+  return response
 }

src/run/headers.test.ts

@@ -1,7 +1,20 @@
 import type { NextConfigComplete } from 'next/dist/server/config-shared.js'
-import { afterEach, describe, expect, test, vi } from 'vitest'
+import { v4 } from 'uuid'
+import { afterEach, describe, expect, test, vi, beforeEach } from 'vitest'
 
-import { setCacheControlHeaders, setVaryHeaders } from './headers.js'
+import { FixtureTestContext } from '../../tests/utils/fixture.js'
+import { generateRandomObjectID, startMockBlobStore } from '../../tests/utils/helpers.js'
+
+import { setVaryHeaders, setCacheControlHeaders } from './headers.js'
+
+beforeEach<FixtureTestContext>(async (ctx) => {
+  // set for each test a new deployID and siteID
+  ctx.deployID = generateRandomObjectID()
+  ctx.siteID = v4()
+  vi.stubEnv('DEPLOY_ID', ctx.deployID)
+
+  await startMockBlobStore(ctx)
+})
 
 describe('headers', () => {
   afterEach(() => {

src/run/headers.ts

@@ -1,3 +1,6 @@
+import { Buffer } from 'node:buffer'
+
+import { getDeployStore } from '@netlify/blobs'
 import type { NextConfigComplete } from 'next/dist/server/config-shared.js'
 
 import type { TagsManifest } from './config.js'
@@ -71,6 +74,37 @@ export const setVaryHeaders = (
   headers.set(`netlify-vary`, generateNetlifyVaryValues(netlifyVaryValues))
 }
 
+function encodeBlobKey(key: string) {
+  return Buffer.from(key.replace(/^\//, '')).toString('base64')
+}
+
+/**
+ * Change the date header to be the last-modified date of the blob. This means the CDN
+ * will use the correct expiry time for the response. e.g. if the blob was last modified
+ * 5 seconds ago and is sent with a maxage of 10, the CDN will cache it for 5 seconds.
+ * By default, Next.js sets the date header to the current time, even if it's served
+ * from the cache, meaning that the CDN will cache it for 10 seconds, which is incorrect.
+ */
+export const adjustDateHeader = async (headers: Headers, request: Request) => {
+  if (headers.get('x-nextjs-cache') !== 'HIT') {
+    return
+  }
+  const path = new URL(request.url).pathname
+  const key = encodeBlobKey(path)
+  const blobStore = getDeployStore()
+  // TODO: use metadata for this
+  const { lastModified } = (await blobStore.get(key, { type: 'json' })) ?? {}
+
+  if (!lastModified) {
+    return
+  }
+  const lastModifiedDate = new Date(lastModified)
+  // Show actual date of the function call in the date header
+  headers.set('x-nextjs-date', headers.get('date') ?? lastModifiedDate.toUTCString())
+  // Setting Age also would work, but we already have the lastModified time so will use that.
+  headers.set('date', lastModifiedDate.toUTCString())
+}
+
 /**
  * Ensure stale-while-revalidate and s-maxage don't leak to the client, but
  * assume the user knows what they are doing if CDN cache controls are set

src/run/next.cts

@@ -1,21 +1,20 @@
-import { getDeployStore } from '@netlify/blobs'
 import fs from 'fs/promises'
 import { Buffer } from 'node:buffer'
 import { relative, resolve } from 'path'
 
-import type { getRequestHandlers } from 'next/dist/server/lib/start-server.js'
-
+import { getDeployStore } from '@netlify/blobs'
 // @ts-expect-error no types installed
 import { patchFs } from 'fs-monkey'
+import type { getRequestHandlers } from 'next/dist/server/lib/start-server.js'
 
 type FS = typeof import('fs')
 
 export async function getMockedRequestHandlers(...args: Parameters<typeof getRequestHandlers>) {
   const store = getDeployStore()
   const ofs = { ...fs }
 
-  async function readFileFallbackBlobStore(...args: Parameters<FS['promises']['readFile']>) {
-    const [path, options] = args
+  async function readFileFallbackBlobStore(...fsargs: Parameters<FS['promises']['readFile']>) {
+    const [path, options] = fsargs
     try {
       // Attempt to read from the disk
       // important to use the `import * as fs from 'fs'` here to not end up in a endless loop
@@ -40,9 +39,11 @@ export async function getMockedRequestHandlers(...args: Parameters<typeof getReq
     {
       readFile: readFileFallbackBlobStore,
     },
+    // eslint-disable-next-line n/global-require
     require('fs').promises,
   )
 
+  // eslint-disable-next-line no-shadow
   const { getRequestHandlers } = await import('next/dist/server/lib/start-server.js')
   return getRequestHandlers(...args)
 }

tests/fixtures/page-router/pages/static/revalidate-slow.js

@@ -0,0 +1,26 @@
+const Show = ({ show, time }) => (
+  <div>
+    <p>
+      This page uses getStaticProps() to pre-fetch a TV show at
+      <span data-testid="date-now">{time}</span>
+    </p>
+    <hr />
+    <h1>Show #{show.id}</h1>
+    <p>{show.name}</p>
+  </div>
+)
+
+export async function getStaticProps(context) {
+  const res = await fetch(`https://tvproxy.netlify.app/shows/71`)
+  const data = await res.json()
+
+  return {
+    props: {
+      show: data,
+      time: new Date().toISOString(),
+    },
+    revalidate: +process.env.REVALIDATE_SECONDS || 10, // In seconds
+  }
+}
+
+export default Show

tests/integration/cache-handler.test.ts

@@ -45,6 +45,7 @@ describe('page router', () => {
       '500.html',
       'static/revalidate-automatic',
       'static/revalidate-manual',
+      'static/revalidate-slow',
     ])
 
     // test the function call
@@ -59,6 +60,9 @@ describe('page router', () => {
       }),
     )
 
+    // Ping this now so we can wait in parallel
+    const callLater = await invokeFunction(ctx, { url: 'static/revalidate-slow' })
+
     // wait to have a stale page
     await new Promise<void>((resolve) => setTimeout(resolve, 3_000))
 
@@ -73,6 +77,18 @@ describe('page router', () => {
     )
     expect(call2Date, 'the date was cached and is matching the initial one').not.toBe(call1Date)
 
+    // Slow revalidate should still be a hit, but the maxage should be updated
+    const callLater2 = await invokeFunction(ctx, { url: 'static/revalidate-slow' })
+
+    expect(callLater2.statusCode).toBe(200)
+
+    expect(callLater2.headers, 'date header matches the cached value').toEqual(
+      expect.objectContaining({
+        'x-nextjs-cache': 'HIT',
+        date: callLater.headers['date'],
+      }),
+    )
+
     // it does not wait for the cache.set so we have to manually wait here until the blob storage got populated
     await new Promise<void>((resolve) => setTimeout(resolve, 100))
 

tests/integration/static.test.ts

@@ -39,6 +39,7 @@ test<FixtureTestContext>('requesting a non existing page route that needs to be
     '500.html',
     'static/revalidate-automatic',
     'static/revalidate-manual',
+    'static/revalidate-slow',
   ])
 
   // test that it should request the 404.html file