From 931c10129913a847cf6c4ba5f561b2ebf5b8edcc Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Wed, 3 May 2023 10:35:34 +0200 Subject: [PATCH] Fix cross-worker revalidate API (#49101) Currently we invoke the revalidate request directly in the current server when `res.revalidate()` is called. However app needs to be rendered in a separate worker so this results in an error of React. This PR fixes it by sending the request via IPC so the main process will delegate that to the correct render worker. Closes #48948. --------- Co-authored-by: JJ Kasper Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- packages/next/src/server/api-utils/node.ts | 57 ++++++++++++++----- packages/next/src/server/next-server.ts | 38 +++++++++++-- .../app-dir/revalidate/app/layout.js | 7 +++ .../production/app-dir/revalidate/app/page.js | 4 ++ .../app-dir/revalidate/next.config.js | 5 ++ .../revalidate/pages/api/revalidate.js | 8 +++ .../app-dir/revalidate/revalidate.test.ts | 20 +++++++ 7 files changed, 121 insertions(+), 18 deletions(-) create mode 100644 test/production/app-dir/revalidate/app/layout.js create mode 100644 test/production/app-dir/revalidate/app/page.js create mode 100644 test/production/app-dir/revalidate/next.config.js create mode 100644 test/production/app-dir/revalidate/pages/api/revalidate.js create mode 100644 test/production/app-dir/revalidate/revalidate.test.ts diff --git a/packages/next/src/server/api-utils/node.ts b/packages/next/src/server/api-utils/node.ts index fb8a87e87998..2eb8ba4e2dcf 100644 --- a/packages/next/src/server/api-utils/node.ts +++ b/packages/next/src/server/api-utils/node.ts @@ -27,7 +27,6 @@ import { SYMBOL_PREVIEW_DATA, RESPONSE_LIMIT_DEFAULT, } from './index' -import { createRequestResponseMocks } from '../lib/mock-request' import { getTracer } from '../lib/trace/tracer' import { NodeSpan } from '../lib/trace/constants' import { RequestCookies } from '../web/spec-extension/cookies' @@ -36,6 +35,7 @@ import { PRERENDER_REVALIDATE_HEADER, PRERENDER_REVALIDATE_ONLY_GENERATED_HEADER, } from '../../lib/constants' +import { invokeRequest } from '../lib/server-ipc' export function tryGetPreviewData( req: IncomingMessage | BaseNextRequest | Request, @@ -194,7 +194,14 @@ export async function parseBody( type ApiContext = __ApiPreviewProps & { trustHostHeader?: boolean allowedRevalidateHeaderKeys?: string[] - revalidate?: (_req: IncomingMessage, _res: ServerResponse) => Promise + hostname?: string + revalidate?: (config: { + urlPath: string + revalidateHeaders: { [key: string]: string | string[] } + opts: { unstable_onlyGenerated?: boolean } + }) => Promise + + // (_req: IncomingMessage, _res: ServerResponse) => Promise } function getMaxContentLength(responseLimit?: ResponseLimit) { @@ -453,20 +460,44 @@ async function revalidate( throw new Error(`Invalid response ${res.status}`) } } else if (context.revalidate) { - const mocked = createRequestResponseMocks({ - url: urlPath, - headers: revalidateHeaders, - }) + // We prefer to use the IPC call if running under the workers mode. + const ipcPort = process.env.__NEXT_PRIVATE_ROUTER_IPC_PORT + if (ipcPort) { + const ipcKey = process.env.__NEXT_PRIVATE_ROUTER_IPC_KEY + const res = await invokeRequest( + `http://${ + context.hostname + }:${ipcPort}?key=${ipcKey}&method=revalidate&args=${encodeURIComponent( + JSON.stringify([{ urlPath, revalidateHeaders }]) + )}`, + { + method: 'GET', + headers: {}, + } + ) - await context.revalidate(mocked.req, mocked.res) - await mocked.res.hasStreamed + const chunks = [] - if ( - mocked.res.getHeader('x-nextjs-cache') !== 'REVALIDATED' && - !(mocked.res.statusCode === 404 && opts.unstable_onlyGenerated) - ) { - throw new Error(`Invalid response ${mocked.res.statusCode}`) + for await (const chunk of res) { + if (chunk) { + chunks.push(chunk) + } + } + const body = Buffer.concat(chunks).toString() + const result = JSON.parse(body) + + if (result.err) { + throw new Error(result.err.message) + } + + return } + + await context.revalidate({ + urlPath, + revalidateHeaders, + opts, + }) } else { throw new Error( `Invariant: required internal revalidate method not passed to api-utils` diff --git a/packages/next/src/server/next-server.ts b/packages/next/src/server/next-server.ts index 0ebce2758ae7..4dfa50e77b5d 100644 --- a/packages/next/src/server/next-server.ts +++ b/packages/next/src/server/next-server.ts @@ -107,6 +107,7 @@ import { removePathPrefix } from '../shared/lib/router/utils/remove-path-prefix' import { addPathPrefix } from '../shared/lib/router/utils/add-path-prefix' import { pathHasPrefix } from '../shared/lib/router/utils/path-has-prefix' import { filterReqHeaders, invokeRequest } from './lib/server-ipc' +import { createRequestResponseMocks } from './lib/mock-request' export * from './base-server' @@ -906,16 +907,13 @@ export default class NextNodeServer extends BaseServer { pageModule, { ...this.renderOpts.previewProps, - revalidate: (newReq: IncomingMessage, newRes: ServerResponse) => - this.getRequestHandler()( - new NodeNextRequest(newReq), - new NodeNextResponse(newRes) - ), + revalidate: this.revalidate.bind(this), // internal config so is not typed trustHostHeader: (this.nextConfig.experimental as Record) .trustHostHeader, allowedRevalidateHeaderKeys: this.nextConfig.experimental.allowedRevalidateHeaderKeys, + hostname: this.hostname, }, this.minimalMode, this.renderOpts.dev, @@ -1672,6 +1670,36 @@ export default class NextNodeServer extends BaseServer { } } + public async revalidate({ + urlPath, + revalidateHeaders, + opts, + }: { + urlPath: string + revalidateHeaders: { [key: string]: string | string[] } + opts: { unstable_onlyGenerated?: boolean } + }) { + const mocked = createRequestResponseMocks({ + url: urlPath, + headers: revalidateHeaders, + }) + + const handler = this.getRequestHandler() + await handler( + new NodeNextRequest(mocked.req), + new NodeNextResponse(mocked.res) + ) + await mocked.res.hasStreamed + + if ( + mocked.res.getHeader('x-nextjs-cache') !== 'REVALIDATED' && + !(mocked.res.statusCode === 404 && opts.unstable_onlyGenerated) + ) { + throw new Error(`Invalid response ${mocked.res.statusCode}`) + } + return {} + } + public async render( req: BaseNextRequest | IncomingMessage, res: BaseNextResponse | ServerResponse, diff --git a/test/production/app-dir/revalidate/app/layout.js b/test/production/app-dir/revalidate/app/layout.js new file mode 100644 index 000000000000..a3a86a5ca1e1 --- /dev/null +++ b/test/production/app-dir/revalidate/app/layout.js @@ -0,0 +1,7 @@ +export default function Root({ children }) { + return ( + + {children} + + ) +} diff --git a/test/production/app-dir/revalidate/app/page.js b/test/production/app-dir/revalidate/app/page.js new file mode 100644 index 000000000000..6cc12f77ff9c --- /dev/null +++ b/test/production/app-dir/revalidate/app/page.js @@ -0,0 +1,4 @@ +export default async function Page() { + const data = Math.random() + return

{data}

+} diff --git a/test/production/app-dir/revalidate/next.config.js b/test/production/app-dir/revalidate/next.config.js new file mode 100644 index 000000000000..cfa3ac3d7aa9 --- /dev/null +++ b/test/production/app-dir/revalidate/next.config.js @@ -0,0 +1,5 @@ +module.exports = { + experimental: { + appDir: true, + }, +} diff --git a/test/production/app-dir/revalidate/pages/api/revalidate.js b/test/production/app-dir/revalidate/pages/api/revalidate.js new file mode 100644 index 000000000000..a3e4482a2bcb --- /dev/null +++ b/test/production/app-dir/revalidate/pages/api/revalidate.js @@ -0,0 +1,8 @@ +export default async function (_req, res) { + try { + await res.revalidate('/') + return res.json({ revalidated: true }) + } catch (err) { + return res.status(500).send('Error') + } +} diff --git a/test/production/app-dir/revalidate/revalidate.test.ts b/test/production/app-dir/revalidate/revalidate.test.ts new file mode 100644 index 000000000000..1922eb6c3bf3 --- /dev/null +++ b/test/production/app-dir/revalidate/revalidate.test.ts @@ -0,0 +1,20 @@ +import { createNextDescribe } from 'e2e-utils' + +createNextDescribe( + 'app-dir revalidate', + { + files: __dirname, + skipDeployment: true, + }, + ({ next }) => { + it('should be able to revalidate the cache via pages/api', async () => { + const $ = await next.render$('/') + const id = $('h1').text() + const res = await next.fetch('/api/revalidate') + expect(res.status).toBe(200) + const $2 = await next.render$('/') + const id2 = $2('h1').text() + expect(id).not.toBe(id2) + }) + } +)