Skip to content

Commit

Permalink
[Segment Cache] Respond with 204 on cache miss
Browse files Browse the repository at this point in the history
Currently, when the client prefetches a segment, the server responds
with a 404 if it cannot fulfill the request. This updates it to respond
with a 204 No Content instead, since it's not an error for the client
to request a segment whose prefetch hasn't been generated.

When responding with 204, the server also sends the
'x-nextjs-postponed' header, but only if PPR is enabled for the route.
The client can use the absence of this header to distinguish between a
regular cache miss and a miss due to PPR being disabled. In a later PR,
I will update the client to use this information to fall back to the
non-PPR behavior.
  • Loading branch information
acdlite committed Dec 10, 2024
1 parent 3eb1d03 commit dcb5292
Show file tree
Hide file tree
Showing 11 changed files with 152 additions and 46 deletions.
21 changes: 18 additions & 3 deletions packages/next/src/client/components/segment-cache/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {
} from '../../../server/app-render/collect-segment-data'
import type { LoadingModuleData } from '../../../shared/lib/app-router-context.shared-runtime'
import {
NEXT_DID_POSTPONE_HEADER,
NEXT_ROUTER_PREFETCH_HEADER,
NEXT_ROUTER_SEGMENT_PREFETCH_HEADER,
NEXT_URL,
Expand Down Expand Up @@ -505,6 +506,15 @@ async function fetchRouteOnCacheMiss(
rejectRouteCacheEntry(entry, Date.now() + 10 * 1000)
return
}
if (response.status === 204) {
const routePPRIsEnabled = response.headers.get(NEXT_DID_POSTPONE_HEADER)
if (!routePPRIsEnabled) {
// TODO: PPR is disabled for this route. We should do a legacy-style
// prefetch request to populate the cache.
}
rejectRouteCacheEntry(entry, Date.now() + 10 * 1000)
return
}
const prefetchStream = createPrefetchResponseStream(
response.body,
routeCacheLru,
Expand Down Expand Up @@ -594,9 +604,14 @@ async function fetchSegmentEntryOnCacheMiss(
accessToken === '' ? segmentPath : `${segmentPath}.${accessToken}`,
routeKey.nextUrl
)
if (!response || !response.ok || !response.body) {
// Server responded with an error. We should still cache the response, but
// we can try again after 10 seconds.
if (
!response ||
!response.ok ||
response.status === 204 || // Cache miss
!response.body
) {
// Server responded with an error, or with a miss. We should still cache
// the response, but we can try again after 10 seconds.
rejectSegmentCacheEntry(segmentCacheEntry, Date.now() + 10 * 1000)
return
}
Expand Down
88 changes: 47 additions & 41 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3044,49 +3044,55 @@ export default abstract class Server<
}
)

if (
isRoutePPREnabled &&
isPrefetchRSCRequest &&
typeof segmentPrefetchHeader === 'string'
) {
if (cacheEntry?.value?.kind === CachedRouteKind.APP_PAGE) {
// This is a prefetch request for an individual segment's static data.
// Unless the segment is fully dynamic, the data should have already been
// loaded into the cache, when the page itself was generated. So we should
// always either return the cache entry. If no cache entry is available,
// it's a 404 — either the segment is fully dynamic, or an invalid segment
// path was requested.
if (cacheEntry.value.segmentData) {
const matchedSegment = cacheEntry.value.segmentData.get(
segmentPrefetchHeader
)
if (matchedSegment !== undefined) {
return {
type: 'rsc',
body: RenderResult.fromStatic(matchedSegment),
// TODO: Eventually this should use revalidate time of the
// individual segment, not the whole page.
revalidate: cacheEntry.revalidate,
}
if (isPrefetchRSCRequest && typeof segmentPrefetchHeader === 'string') {
// This is a prefetch request issued by the client Segment Cache. These
// should never reach the application layer (lambda). We should either
// respond from the cache (HIT) or respond with 204 No Content (MISS).
if (
cacheEntry !== null &&
// This is always true at runtime but is needed to refine the type
// of cacheEntry.value to CachedAppPageValue, because the outer
// ResponseCacheEntry is not a discriminated union.
cacheEntry.value?.kind === CachedRouteKind.APP_PAGE &&
cacheEntry.value.segmentData
) {
const matchedSegment = cacheEntry.value.segmentData.get(
segmentPrefetchHeader
)
if (matchedSegment !== undefined) {
// Cache hit
return {
type: 'rsc',
body: RenderResult.fromStatic(matchedSegment),
// TODO: Eventually this should use revalidate time of the
// individual segment, not the whole page.
revalidate: cacheEntry.revalidate,
}
}
// If the segment is not found, return a 404. Since this is an RSC
// request, there's no reason to render a 404 page; just return an
// empty response.
res.statusCode = 404
return {
type: 'rsc',
body: RenderResult.fromStatic(''),
revalidate: cacheEntry.revalidate,
}
} else {
// Segment prefetches should never reach the application layer. If
// there's no cache entry for this page, it's a 404.
res.statusCode = 404
return {
type: 'rsc',
body: RenderResult.fromStatic(''),
}
}

// Cache miss. Either a cache entry for this route has not been generated,
// or there's no match for the requested segment. Regardless, respond with
// a 204 No Content. We don't bother to respond with 404 in cases where
// the segment does not exist, because these requests are only issued by
// the client cache.
// TODO: If this is a request for the route tree (the special /_tree
// segment), we should *always* respond with a tree, even if PPR
// is disabled.
res.statusCode = 204
if (isRoutePPREnabled) {
// Set a header to indicate that PPR is enabled for this route. This
// lets the client distinguish between a regular cache miss and a cache
// miss due to PPR being disabled.
// NOTE: Theoretically, when PPR is enabled, there should *never* be
// a cache miss because we should generate a fallback route. So this
// is mostly defensive.
res.setHeader(NEXT_DID_POSTPONE_HEADER, '1')
}
return {
type: 'rsc',
body: RenderResult.fromStatic(''),
revalidate: cacheEntry?.revalidate,
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ describe('per segment prefetching', () => {
expect(childResponseText).toInclude('"rsc"')
})

it('respond with 404 if the segment does not have prefetch data', async () => {
it('respond with 204 if the segment does not have prefetch data', async () => {
const response = await prefetch('/en', '/does-not-exist')
expect(response.status).toBe(404)
expect(response.status).toBe(204)
const responseText = await response.text()
expect(responseText.trim()).toBe('')
})
Expand Down
11 changes: 11 additions & 0 deletions test/e2e/app-dir/segment-cache/incremental-opt-in/app/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export default function RootLayout({
children,
}: {
children: React.ReactNode
}) {
return (
<html lang="en">
<body>{children}</body>
</html>
)
}
19 changes: 19 additions & 0 deletions test/e2e/app-dir/segment-cache/incremental-opt-in/app/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import Link from 'next/link'

export default function Page() {
return (
<ul>
<li>
<Link href="/ppr-enabled">Page with PPR enabled</Link>
</li>
<li>
<Link href="/ppr-enabled/dynamic-param">
Page with PPR enabled but has dynamic param
</Link>
</li>
<li>
<Link href="/ppr-disabled">Page with PPR disabled</Link>
</li>
</ul>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function PPRDisabled() {
return '(intentionally empty)'
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return '(intentionally empty)'
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export const experimental_ppr = true

export default function RootLayout({
children,
}: {
children: React.ReactNode
}) {
return children
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function PPREnabled() {
return '(intentionally empty)'
}
12 changes: 12 additions & 0 deletions test/e2e/app-dir/segment-cache/incremental-opt-in/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* @type {import('next').NextConfig}
*/
const nextConfig = {
experimental: {
ppr: 'incremental',
dynamicIO: true,
clientSegmentCache: true,
},
}

module.exports = nextConfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { nextTestSetup } from 'e2e-utils'

describe('segment cache (incremental opt in)', () => {
const { next, isNextDev, skipped } = nextTestSetup({
files: __dirname,
skipDeployment: true,
})
if (isNextDev || skipped) {
test('ppr is disabled', () => {})
return
}

// TODO: Replace with e2e test once the client part is implemented
it('prefetch responds with 204 if PPR is disabled for a route', async () => {
await next.browser('/')
const response = await next.fetch('/ppr-disabled', {
headers: {
RSC: '1',
'Next-Router-Prefetch': '1',
'Next-Router-Segment-Prefetch': '/_tree',
},
})
expect(response.status).toBe(204)
})
})

0 comments on commit dcb5292

Please sign in to comment.