Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: apply caching headers to pages router 404 with getStaticProps #2764

Merged
merged 18 commits into from
Mar 6, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/build/content/prerendered.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const routeToFilePath = (path: string) => {

const buildPagesCacheValue = async (
path: string,
initialRevalidateSeconds: number | false | undefined,
shouldUseEnumKind: boolean,
shouldSkipJson = false,
): Promise<NetlifyCachedPageValue> => ({
Expand All @@ -65,6 +66,7 @@ const buildPagesCacheValue = async (
pageData: shouldSkipJson ? {} : JSON.parse(await readFile(`${path}.json`, 'utf-8')),
headers: undefined,
status: undefined,
revalidate: initialRevalidateSeconds,
})

const buildAppCacheValue = async (
Expand Down Expand Up @@ -178,6 +180,7 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise<void>
}
value = await buildPagesCacheValue(
join(ctx.publishDir, 'server/pages', key),
meta.initialRevalidateSeconds,
shouldUseEnumKind,
)
break
Expand Down Expand Up @@ -210,6 +213,7 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise<void>
const key = routeToFilePath(route)
const value = await buildPagesCacheValue(
join(ctx.publishDir, 'server/pages', key),
undefined,
shouldUseEnumKind,
true, // there is no corresponding json file for fallback, so we are skipping it for this entry
)
Expand Down
10 changes: 10 additions & 0 deletions src/run/handlers/cache.cts
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,11 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions {

const { revalidate, ...restOfPageValue } = blob.value

const requestContext = getRequestContext()
if (requestContext) {
requestContext.pageHandlerRevalidate = revalidate
}

await this.injectEntryToPrerenderManifest(key, revalidate)

return {
Expand All @@ -272,6 +277,11 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions {

const { revalidate, rscData, ...restOfPageValue } = blob.value

const requestContext = getRequestContext()
if (requestContext) {
requestContext.pageHandlerRevalidate = revalidate
}

await this.injectEntryToPrerenderManifest(key, revalidate)

return {
Expand Down
1 change: 1 addition & 0 deletions src/run/handlers/request-context.cts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export type RequestContext = {
didPagesRouterOnDemandRevalidate?: boolean
serverTiming?: string
routeHandlerRevalidate?: NetlifyCachedRouteValue['revalidate']
pageHandlerRevalidate?: NetlifyCachedRouteValue['revalidate']
/**
* Track promise running in the background and need to be waited for.
* Uses `context.waitUntil` if available, otherwise stores promises to
Expand Down
46 changes: 34 additions & 12 deletions src/run/headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { Span } from '@opentelemetry/api'
import type { NextConfigComplete } from 'next/dist/server/config-shared.js'

import { encodeBlobKey } from '../shared/blobkey.js'
import type { NetlifyCachedRouteValue } from '../shared/cache-types.cjs'

import { getLogger, RequestContext } from './handlers/request-context.cjs'
import type { RuntimeTracer } from './handlers/tracer.cjs'
Expand Down Expand Up @@ -208,6 +209,19 @@ export const adjustDateHeader = async ({
headers.set('date', lastModifiedDate.toUTCString())
}

function setCacheControlFromRequestContext(
headers: Headers,
revalidate: NetlifyCachedRouteValue['revalidate'],
) {
const cdnCacheControl =
// if we are serving already stale response, instruct edge to not attempt to cache that response
headers.get('x-nextjs-cache') === 'STALE'
? 'public, max-age=0, must-revalidate, durable'
: `s-maxage=${revalidate === false ? 31536000 : revalidate}, stale-while-revalidate=31536000, durable`

headers.set('netlify-cdn-cache-control', cdnCacheControl)
}

/**
* 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
Expand All @@ -225,13 +239,7 @@ export const setCacheControlHeaders = (
!headers.has('netlify-cdn-cache-control')
) {
// handle CDN Cache Control on Route Handler responses
const cdnCacheControl =
// if we are serving already stale response, instruct edge to not attempt to cache that response
headers.get('x-nextjs-cache') === 'STALE'
? 'public, max-age=0, must-revalidate, durable'
: `s-maxage=${requestContext.routeHandlerRevalidate === false ? 31536000 : requestContext.routeHandlerRevalidate}, stale-while-revalidate=31536000, durable`

headers.set('netlify-cdn-cache-control', cdnCacheControl)
setCacheControlFromRequestContext(headers, requestContext.routeHandlerRevalidate)
return
}

Expand All @@ -242,11 +250,25 @@ export const setCacheControlHeaders = (
.log('NetlifyHeadersHandler.trailingSlashRedirect')
}

if (status === 404 && request.url.endsWith('.php')) {
// temporary CDN Cache Control handling for bot probes on PHP files
// https://linear.app/netlify/issue/FRB-1344/prevent-excessive-ssr-invocations-due-to-404-routes
headers.set('cache-control', 'public, max-age=0, must-revalidate')
headers.set('netlify-cdn-cache-control', `max-age=31536000, durable`)
if (status === 404) {
if (request.url.endsWith('.php')) {
// temporary CDN Cache Control handling for bot probes on PHP files
// https://linear.app/netlify/issue/FRB-1344/prevent-excessive-ssr-invocations-due-to-404-routes
headers.set('cache-control', 'public, max-age=0, must-revalidate')
headers.set('netlify-cdn-cache-control', `max-age=31536000, durable`)
return
}

if (
typeof requestContext.pageHandlerRevalidate !== 'undefined' &&
['GET', 'HEAD'].includes(request.method) &&
!headers.has('cdn-cache-control') &&
!headers.has('netlify-cdn-cache-control')
) {
// handle CDN Cache Control on 404 Page responses
setCacheControlFromRequestContext(headers, requestContext.pageHandlerRevalidate)
return
}
}

const cacheControl = headers.get('cache-control')
Expand Down
12 changes: 12 additions & 0 deletions tests/integration/page-router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ test<FixtureTestContext>('Should serve correct locale-aware custom 404 pages', a
load(responseImplicitDefaultLocale.body)('[data-testid="locale"]').text(),
'Served 404 page content should use default locale if locale is not explicitly used in pathname (after basePath)',
).toBe('en')
expect(
responseImplicitDefaultLocale.headers['netlify-cdn-cache-control'],
'Response for not existing route if locale is not explicitly used in pathname (after basePath) should have 404 status',
).toBe('s-maxage=31536000, stale-while-revalidate=31536000, durable')

const responseExplicitDefaultLocale = await invokeFunction(ctx, {
url: '/base/path/en/not-existing-page',
Expand All @@ -139,6 +143,10 @@ test<FixtureTestContext>('Should serve correct locale-aware custom 404 pages', a
load(responseExplicitDefaultLocale.body)('[data-testid="locale"]').text(),
'Served 404 page content should use default locale if default locale is explicitly used in pathname (after basePath)',
).toBe('en')
expect(
responseExplicitDefaultLocale.headers['netlify-cdn-cache-control'],
'Response for not existing route if locale is not explicitly used in pathname (after basePath) should have 404 status',
).toBe('s-maxage=31536000, stale-while-revalidate=31536000, durable')

const responseNonDefaultLocale = await invokeFunction(ctx, {
url: '/base/path/fr/not-existing-page',
Expand All @@ -152,4 +160,8 @@ test<FixtureTestContext>('Should serve correct locale-aware custom 404 pages', a
load(responseNonDefaultLocale.body)('[data-testid="locale"]').text(),
'Served 404 page content should use non-default locale if non-default locale is explicitly used in pathname (after basePath)',
).toBe('fr')
expect(
responseNonDefaultLocale.headers['netlify-cdn-cache-control'],
'Response for not existing route if locale is not explicitly used in pathname (after basePath) should have 404 status',
).toBe('s-maxage=31536000, stale-while-revalidate=31536000, durable')
})
Loading