archive/rsnext
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix stale revalidate stream handling (#55978)

Browse source 
This ensures we don't block sending the stream down when handling stale revalidates, in edge runtime we leverage the existing `waitUntil` handling and in node.js runtime we couple this into our pipe readable handling to wait to close the writable until after the promise resolves. 

x-ref: https://github.com/vercel/next.js/issues/54193
This commit is contained in:
JJ Kasper 2023-09-25 13:50:00 -07:00 committed by GitHub
parent df3c1b822a
commit 57bb52d37d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 158 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
packages/next/src/server/app-render/app-render.tsx
View file

@ -1765,12 +1765,12 @@ export const renderToHTMLOrFlight: AppPageRender = (
asNotFound: pagePath === '/404',
tree: loaderTree,
}),
{ ...extraRenderResultMeta }
{
...extraRenderResultMeta,
waitUntil: Promise.all(staticGenerationStore.pendingRevalidates || []),
}
)
if (staticGenerationStore.pendingRevalidates) {
await Promise.all(staticGenerationStore.pendingRevalidates)
}
addImplicitTags(staticGenerationStore)
extraRenderResultMeta.fetchTags = staticGenerationStore.tags?.join(',')
renderResult.extendMetadata({

1
packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts
View file

@ -16,6 +16,7 @@ export type StaticGenerationContext = {
fetchCache?: StaticGenerationStore['fetchCache']
isDraftMode?: boolean
isServerAction?: boolean
waitUntil?: Promise<any>
/**
* A hack around accessing the store value outside the context of the

8
packages/next/src/server/base-server.ts
View file

@ -2080,8 +2080,12 @@ export default abstract class Server<ServerOptions extends Options = Options> {
}
// Send the response now that we have copied it into the cache.
await sendResponse(req, res, response)
await sendResponse(
req,
res,
response,
context.staticGenerationContext.waitUntil
)
return null
} catch (err) {
// If this is during static generation, throw the error again.

3
packages/next/src/server/future/route-modules/app-route/module.ts
View file

@ -371,9 +371,10 @@ export class AppRouteRouteModule extends RouteModule<
;(context.staticGenerationContext as any).fetchMetrics =
staticGenerationStore.fetchMetrics
await Promise.all(
context.staticGenerationContext.waitUntil = Promise.all(
staticGenerationStore.pendingRevalidates || []
)
addImplicitTags(staticGenerationStore)
;(context.staticGenerationContext as any).fetchTags =
staticGenerationStore.tags?.join(',')

9
packages/next/src/server/pipe-readable.ts
View file

@ -36,11 +36,14 @@ export interface PipeTarget<R = any> {
* Allows us to cleanup our onClose listener.
*/
off: (event: 'close', cb: () => void) => void
closed?: boolean
}
export async function pipeReadable(
readable: ReadableStream<Uint8Array>,
writable: PipeTarget<Uint8Array>
writable: PipeTarget<Uint8Array>,
waitUntilForEnd?: Promise<void>
) {
const reader = readable.getReader()
let readerDone = false
@ -93,6 +96,10 @@ export async function pipeReadable(
// If the client hasn't disconnected yet, end the writable so that the
// response sends the final bytes.
if (waitUntilForEnd) {
await waitUntilForEnd
}
if (!writableClosed) {
writable.end()
}

7
packages/next/src/server/render-result.ts
View file

@ -12,6 +12,7 @@ export type RenderResultMetadata = {
isRedirect?: boolean
fetchMetrics?: StaticGenerationStore['fetchMetrics']
fetchTags?: string
waitUntil?: Promise<any>
}
type RenderResultResponse = ReadableStream<Uint8Array> | string | null
@ -47,10 +48,13 @@ export default class RenderResult {
return new RenderResult(value)
}
private waitUntil?: Promise<void>
constructor(
response: RenderResultResponse,
{
contentType,
waitUntil,
...metadata
}: {
contentType?: ContentTypeOption
@ -59,6 +63,7 @@ export default class RenderResult {
this.response = response
this.contentType = contentType
this.metadata = metadata
this.waitUntil = waitUntil
}
public extendMetadata(metadata: RenderResultMetadata) {
@ -107,6 +112,6 @@ export default class RenderResult {
)
}
return await pipeReadable(this.response, res)
return await pipeReadable(this.response, res, this.waitUntil)
}
}

5
packages/next/src/server/send-response.ts
View file

@ -13,7 +13,8 @@ import { splitCookiesString } from './web/utils'
export async function sendResponse(
req: BaseNextRequest,
res: BaseNextResponse,
response: Response
response: Response,
waitUntil?: Promise<any>
): Promise<void> {
// Don't use in edge runtime
if (process.env.NEXT_RUNTIME !== 'edge') {
@ -45,7 +46,7 @@ export async function sendResponse(
// A response body must not be sent for HEAD requests. See https://httpwg.org/specs/rfc9110.html#HEAD
if (response.body && req.method !== 'HEAD') {
await pipeReadable(response.body, originalResponse)
await pipeReadable(response.body, originalResponse, waitUntil)
} else {
originalResponse.end()
}

13
packages/next/src/server/web/edge-route-module-wrapper.ts
View file

@ -12,6 +12,7 @@ import { IncrementalCache } from '../lib/incremental-cache'
import { RouteMatcher } from '../future/route-matchers/route-matcher'
import { removeTrailingSlash } from '../../shared/lib/router/utils/remove-trailing-slash'
import { removePathPrefix } from '../../shared/lib/router/utils/remove-path-prefix'
import { NextFetchEvent } from './spec-extension/fetch-event'
type WrapOptions = Partial<Pick<AdapterOptions, 'page'>>
@ -62,7 +63,10 @@ export class EdgeRouteModuleWrapper {
}
}
private async handler(request: NextRequest): Promise<Response> {
private async handler(
request: NextRequest,
evt: NextFetchEvent
): Promise<Response> {
// Get the pathname for the matcher. Pathnames should not have trailing
// slashes for matching.
let pathname = removeTrailingSlash(new URL(request.url).pathname)
@ -108,6 +112,11 @@ export class EdgeRouteModuleWrapper {
}
// Get the response from the handler.
return await this.routeModule.handle(request, context)
const res = await this.routeModule.handle(request, context)
if (context.staticGenerationContext.waitUntil) {
evt.waitUntil(context.staticGenerationContext.waitUntil)
}
return res
}
}

45
test/e2e/app-dir/app-static/app-static.test.ts
View file

@ -502,6 +502,13 @@ createNextDescribe(
'force-cache.html',
'ssg-draft-mode.rsc',
'ssr-forced/page.js',
'stale-cache-serving-edge/app-page/page.js',
'stale-cache-serving-edge/app-page/page_client-reference-manifest.js',
'stale-cache-serving-edge/route-handler/route.js',
'stale-cache-serving/app-page.prefetch.rsc',
'stale-cache-serving/app-page/page.js',
'stale-cache-serving/app-page/page_client-reference-manifest.js',
'stale-cache-serving/route-handler/route.js',
'custom.prefetch.rsc',
'force-cache/page.js',
'ssg-draft-mode.html',
@ -1627,6 +1634,44 @@ createNextDescribe(
})
}
it.each([
{ path: '/stale-cache-serving/app-page' },
{ path: '/stale-cache-serving/route-handler' },
{ path: '/stale-cache-serving-edge/app-page' },
{ path: '/stale-cache-serving-edge/route-handler' },
])('should stream properly for $path', async ({ path }) => {
// prime cache initially
await next.fetch(path)
for (let i = 0; i < 6; i++) {
await waitFor(1000)
const start = Date.now()
let streamStart = 0
const res = await next.fetch(path)
const chunks: any[] = []
await new Promise<void>((bodyResolve) => {
res.body.on('data', (chunk) => {
if (!streamStart) {
streamStart = Date.now()
}
chunks.push(chunk)
})
res.body.on('end', () => {
bodyResolve()
})
})
require('console').log({
start,
duration: Date.now() - start,
streamStart,
startDuration: streamStart - start,
})
expect(streamStart - start).toBeLessThan(3000)
}
})
it('should correctly handle statusCode with notFound + ISR', async () => {
for (let i = 0; i < 5; i++) {
const res = await next.fetch('/articles/non-existent')

20
test/e2e/app-dir/app-static/app/stale-cache-serving-edge/app-page/page.tsx Normal file
View file

@ -0,0 +1,20 @@
export const runtime = 'edge'
const delay = 3000
export default async function Page(props) {
const start = Date.now()
const data = await fetch(
`https://next-data-api-endpoint.vercel.app/api/delay?delay=${delay}`,
{ next: { revalidate: 3 } }
).then((res) => res.json())
const fetchDuration = Date.now() - start
return (
<>
<p id="data">
{JSON.stringify({ fetchDuration, data, now: Date.now() })}
</p>
</>
)
}

16
test/e2e/app-dir/app-static/app/stale-cache-serving-edge/route-handler/route.ts Normal file
View file

@ -0,0 +1,16 @@
import { NextRequest, NextResponse } from 'next/server'
export const runtime = 'edge'
const delay = 3000
export async function GET(req: NextRequest) {
const start = Date.now()
const data = await fetch(
`https://next-data-api-endpoint.vercel.app/api/delay?delay=${delay}`,
{ next: { revalidate: 3 } }
).then((res) => res.json())
const fetchDuration = Date.now() - start
return NextResponse.json({ fetchDuration, data, now: Date.now() })
}

20
test/e2e/app-dir/app-static/app/stale-cache-serving/app-page/page.tsx Normal file
View file

@ -0,0 +1,20 @@
export const dynamic = 'force-dynamic'
const delay = 3000
export default async function Page(props) {
const start = Date.now()
const data = await fetch(
`https://next-data-api-endpoint.vercel.app/api/delay?delay=${delay}`,
{ next: { revalidate: 3 } }
).then((res) => res.json())
const fetchDuration = Date.now() - start
return (
<>
<p id="data">
{JSON.stringify({ fetchDuration, data, now: Date.now() })}
</p>
</>
)
}

16
test/e2e/app-dir/app-static/app/stale-cache-serving/route-handler/route.ts Normal file
View file

@ -0,0 +1,16 @@
import { NextRequest, NextResponse } from 'next/server'
export const dynamic = 'force-dynamic'
const delay = 3000
export async function GET(req: NextRequest) {
const start = Date.now()
const data = await fetch(
`https://next-data-api-endpoint.vercel.app/api/delay?delay=${delay}`,
{ next: { revalidate: 3 } }
).then((res) => res.json())
const fetchDuration = Date.now() - start
return NextResponse.json({ fetchDuration, data, now: Date.now() })
}