Skip to content

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

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

Merged
merged 18 commits into from
Mar 6, 2025
Merged
Show file tree
Hide file tree
Changes from 8 commits
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
5 changes: 5 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 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
47 changes: 34 additions & 13 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 || 31536000}, 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,14 +250,27 @@ 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`)
const cacheControl = headers.get('cache-control')
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 (
['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')
if (
cacheControl !== null &&
['GET', 'HEAD'].includes(request.method) &&
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/** @type {import('next').NextConfig} */
const nextConfig = {
output: 'standalone',
eslint: {
ignoreDuringBuilds: true,
},
generateBuildId: () => 'build-id',
}

module.exports = nextConfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"name": "page-router-404-get-static-props-with-revalidate",
"version": "0.1.0",
"private": true,
"scripts": {
"postinstall": "next build",
"dev": "next dev",
"build": "next build"
},
"dependencies": {
"next": "latest",
"react": "18.2.0",
"react-dom": "18.2.0"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
export default function NotFound({ timestamp }) {
return (
<p>
Custom 404 page with revalidate: <pre data-testid="timestamp">{timestamp}</pre>
</p>
)
}

/** @type {import('next').GetStaticProps} */
export const getStaticProps = ({ locale }) => {
return {
props: {
timestamp: Date.now(),
},
revalidate: 300,
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
const Product = ({ time, slug }) => (
<div>
<h1>Product {slug}</h1>
<p>
This page uses getStaticProps() and getStaticPaths() to pre-fetch a Product
<span data-testid="date-now">{time}</span>
</p>
</div>
)

/** @type {import('next').GetStaticProps} */
export async function getStaticProps({ params }) {
if (params.slug === 'not-found-no-revalidate') {
return {
notFound: true,
}
} else if (params.slug === 'not-found-with-revalidate') {
return {
notFound: true,
revalidate: 600,
}
}

return {
props: {
time: new Date().toISOString(),
slug: params.slug,
},
}
}

/** @type {import('next').GetStaticPaths} */
export const getStaticPaths = () => {
return {
paths: [],
fallback: 'blocking', // false or "blocking"
}
}

export default Product
12 changes: 12 additions & 0 deletions tests/fixtures/page-router-base-path-i18n/pages/products/[slug].js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,19 @@ const Product = ({ time, slug }) => (
</div>
)

/** @type {import('next').GetStaticProps} */
export async function getStaticProps({ params }) {
if (params.slug === 'not-found-no-revalidate') {
return {
notFound: true,
}
} else if (params.slug === 'not-found-with-revalidate') {
return {
notFound: true,
revalidate: 600,
}
}

return {
props: {
time: new Date().toISOString(),
Expand Down
3 changes: 3 additions & 0 deletions tests/fixtures/page-router/pages/404.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function NotFound() {
return <p>Custom 404 page</p>
}
7 changes: 7 additions & 0 deletions tests/fixtures/page-router/pages/products/[slug].js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ const Product = ({ time, slug }) => (
)

export async function getStaticProps({ params }) {
if (params.slug === 'not-found-with-revalidate') {
return {
notFound: true,
revalidate: 600,
}
}

return {
props: {
time: new Date().toISOString(),
Expand Down
123 changes: 122 additions & 1 deletion tests/integration/page-router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { HttpResponse, http, passthrough } from 'msw'
import { setupServer } from 'msw/node'
import { platform } from 'node:process'
import { v4 } from 'uuid'
import { afterAll, afterEach, beforeAll, beforeEach, expect, test, vi } from 'vitest'
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test, vi } from 'vitest'
import { type FixtureTestContext } from '../utils/contexts.js'
import { createFixture, invokeFunction, runPlugin } from '../utils/fixture.js'
import { encodeBlobKey, generateRandomObjectID, startMockBlobStore } from '../utils/helpers.js'
Expand Down Expand Up @@ -153,3 +153,124 @@ test<FixtureTestContext>('Should serve correct locale-aware custom 404 pages', a
'Served 404 page content should use non-default locale if non-default locale is explicitly used in pathname (after basePath)',
).toBe('fr')
})

describe.only('404 caching', () => {
describe('404 without getStaticProps', () => {
test<FixtureTestContext>('not matching dynamic paths should be cached permanently', async (ctx) => {
await createFixture('page-router', ctx)
await runPlugin(ctx)

const notExistingPage = await invokeFunction(ctx, {
url: '/not-existing-page',
})

expect(notExistingPage.statusCode).toBe(404)

expect(
notExistingPage.headers['netlify-cdn-cache-control'],
'should be cached permanently',
).toBe('s-maxage=31536000, stale-while-revalidate=31536000, durable')
})
test<FixtureTestContext>('matching dynamic path with revalidate should be cached permanently', async (ctx) => {
await createFixture('page-router', ctx)
await runPlugin(ctx)

const notExistingPage = await invokeFunction(ctx, {
url: '/products/not-found-with-revalidate',
})

expect(notExistingPage.statusCode).toBe(404)

expect(
notExistingPage.headers['netlify-cdn-cache-control'],
'should be cached permanently',
).toBe('s-maxage=31536000, stale-while-revalidate=31536000, durable')
})
})

describe('404 with getStaticProps without revalidate', () => {
test<FixtureTestContext>('not matching dynamic paths should be cached permanently', async (ctx) => {
console.log('[test] not matching dynamic paths')

await createFixture('page-router-base-path-i18n', ctx)
await runPlugin(ctx)

const notExistingPage = await invokeFunction(ctx, {
url: '/base/path/not-existing-page',
})

expect(notExistingPage.statusCode).toBe(404)

expect(
notExistingPage.headers['netlify-cdn-cache-control'],
'should be cached permanently',
).toBe('s-maxage=31536000, stale-while-revalidate=31536000, durable')
})
test<FixtureTestContext>('matching dynamic path with revalidate should be cached permanently', async (ctx) => {
console.log('[test] matching dynamic path with revalidate')

await createFixture('page-router-base-path-i18n', ctx)
await runPlugin(ctx)

const notExistingPage = await invokeFunction(ctx, {
url: '/base/path/products/not-found-with-revalidate',
})

expect(notExistingPage.statusCode).toBe(404)

expect(
notExistingPage.headers['netlify-cdn-cache-control'],
'should be cached permanently',
).toBe('s-maxage=31536000, stale-while-revalidate=31536000, durable')
})
})

describe('404 with getStaticProps with revalidate', () => {
test<FixtureTestContext>('not matching dynamic paths should be cached for 404 page revalidate', async (ctx) => {
await createFixture('page-router-404-get-static-props-with-revalidate', ctx)
await runPlugin(ctx)

// ignoring initial stale case
await invokeFunction(ctx, {
url: 'not-existing-page',
})

await new Promise((res) => setTimeout(res, 100))

const notExistingPage = await invokeFunction(ctx, {
url: 'not-existing-page',
})

expect(notExistingPage.statusCode).toBe(404)

expect(
notExistingPage.headers['netlify-cdn-cache-control'],
'should be cached for 404 page revalidate',
).toBe('s-maxage=300, stale-while-revalidate=31536000, durable')
})

test<FixtureTestContext>('matching dynamic path with revalidate should be cached for 404 page revalidate', async (ctx) => {
console.log('[test] matching dynamic path with revalidate')

await createFixture('page-router-404-get-static-props-with-revalidate', ctx)
await runPlugin(ctx)

// ignoring initial stale case
await invokeFunction(ctx, {
url: 'products/not-found-with-revalidate',
})
await new Promise((res) => setTimeout(res, 100))

const notExistingPage = await invokeFunction(ctx, {
url: 'products/not-found-with-revalidate',
})

expect(notExistingPage.statusCode).toBe(404)

expect(
notExistingPage.headers['netlify-cdn-cache-control'],
'should be cached for 404 page revalidate',
).toBe('s-maxage=300, stale-while-revalidate=31536000, durable')
})
})
})
Loading