From 70c6f94d95c85d8f32e1d3c057316b03d432ccad Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Wed, 3 May 2023 13:42:21 +0200 Subject: [PATCH] Revert "Revert "Fix cross-worker revalidate API" (#49138)" (#49141) Revert #49138 and split the server-ipc file into smaller modules. --- packages/next/src/server/api-utils/node.ts | 57 ++++++++--- .../next/src/server/dev/next-dev-server.ts | 2 +- .../{server-ipc.ts => server-ipc/index.ts} | 99 ++----------------- .../server/lib/server-ipc/invoke-request.ts | 59 +++++++++++ .../next/src/server/lib/server-ipc/utils.ts | 23 +++++ packages/next/src/server/next-server.ts | 41 ++++++-- .../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 ++++ 11 files changed, 215 insertions(+), 110 deletions(-) rename packages/next/src/server/lib/{server-ipc.ts => server-ipc/index.ts} (60%) create mode 100644 packages/next/src/server/lib/server-ipc/invoke-request.ts create mode 100644 packages/next/src/server/lib/server-ipc/utils.ts 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 fb8a87e879983..9fe5330a8ef41 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/invoke-request' 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/dev/next-dev-server.ts b/packages/next/src/server/dev/next-dev-server.ts index a317b920bcd28..e1a3ebb70b271 100644 --- a/packages/next/src/server/dev/next-dev-server.ts +++ b/packages/next/src/server/dev/next-dev-server.ts @@ -105,7 +105,7 @@ import { IncrementalCache } from '../lib/incremental-cache' import LRUCache from 'next/dist/compiled/lru-cache' import { NextUrlWithParsedQuery } from '../request-meta' import { deserializeErr, errorToJSON } from '../render' -import { invokeRequest } from '../lib/server-ipc' +import { invokeRequest } from '../lib/server-ipc/invoke-request' import { generateInterceptionRoutesRewrites } from '../../lib/generate-interception-routes-rewrites' // Load ReactDevOverlay only when needed diff --git a/packages/next/src/server/lib/server-ipc.ts b/packages/next/src/server/lib/server-ipc/index.ts similarity index 60% rename from packages/next/src/server/lib/server-ipc.ts rename to packages/next/src/server/lib/server-ipc/index.ts index e711ecaea81c1..a0dfe1b313e75 100644 --- a/packages/next/src/server/lib/server-ipc.ts +++ b/packages/next/src/server/lib/server-ipc/index.ts @@ -1,9 +1,9 @@ -import type NextServer from '../next-server' -import { genExecArgv, getNodeOptionsWithoutInspect } from './utils' -import { deserializeErr, errorToJSON } from '../render' -import { IncomingMessage } from 'http' +import type NextServer from '../../next-server' + +import { genExecArgv, getNodeOptionsWithoutInspect } from '../utils' +import { deserializeErr, errorToJSON } from '../../render' import crypto from 'crypto' -import isError from '../../lib/is-error' +import isError from '../../../lib/is-error' // we can't use process.send as jest-worker relies on // it already and can cause unexpected message errors @@ -88,7 +88,7 @@ export const createWorker = ( ) => { const { initialEnv } = require('@next/env') as typeof import('@next/env') const { Worker } = require('next/dist/compiled/jest-worker') - const worker = new Worker(require.resolve('./render-server'), { + const worker = new Worker(require.resolve('../render-server'), { numWorkers: 1, // TODO: do we want to allow more than 10 OOM restarts? maxRetries: 10, @@ -125,9 +125,9 @@ export const createWorker = ( 'clearModuleContext', ], }) as any as InstanceType & { - initialize: typeof import('./render-server').initialize - deleteCache: typeof import('./render-server').deleteCache - deleteAppClientCache: typeof import('./render-server').deleteAppClientCache + initialize: typeof import('../render-server').initialize + deleteCache: typeof import('../render-server').deleteCache + deleteAppClientCache: typeof import('../render-server').deleteAppClientCache } worker.getStderr().pipe(process.stderr) @@ -135,84 +135,3 @@ export const createWorker = ( return worker } - -const forbiddenHeaders = [ - 'accept-encoding', - 'content-length', - 'keepalive', - 'content-encoding', - 'transfer-encoding', - // https://github.com/nodejs/undici/issues/1470 - 'connection', -] - -export const filterReqHeaders = ( - headers: Record -) => { - for (const [key, value] of Object.entries(headers)) { - if ( - forbiddenHeaders.includes(key) || - !(Array.isArray(value) || typeof value === 'string') - ) { - delete headers[key] - } - } - return headers -} - -export const invokeRequest = async ( - targetUrl: string, - requestInit: { - headers: IncomingMessage['headers'] - method: IncomingMessage['method'] - }, - readableBody?: import('stream').Readable -) => { - const parsedUrl = new URL(targetUrl) - - // force localhost to IPv4 as some DNS may - // resolve to IPv6 instead - if (parsedUrl.hostname === 'localhost') { - parsedUrl.hostname = '127.0.0.1' - } - const invokeHeaders = filterReqHeaders({ - ...requestInit.headers, - }) as IncomingMessage['headers'] - - const invokeRes = await new Promise( - (resolveInvoke, rejectInvoke) => { - const http = require('http') as typeof import('http') - - try { - const invokeReq = http.request( - targetUrl, - { - headers: invokeHeaders, - method: requestInit.method, - }, - (res) => { - resolveInvoke(res) - } - ) - invokeReq.on('error', (err) => { - rejectInvoke(err) - }) - - if (requestInit.method !== 'GET' && requestInit.method !== 'HEAD') { - if (readableBody) { - readableBody.pipe(invokeReq) - readableBody.on('close', () => { - invokeReq.end() - }) - } - } else { - invokeReq.end() - } - } catch (err) { - rejectInvoke(err) - } - } - ) - - return invokeRes -} diff --git a/packages/next/src/server/lib/server-ipc/invoke-request.ts b/packages/next/src/server/lib/server-ipc/invoke-request.ts new file mode 100644 index 0000000000000..056e552bdac62 --- /dev/null +++ b/packages/next/src/server/lib/server-ipc/invoke-request.ts @@ -0,0 +1,59 @@ +import { IncomingMessage } from 'http' +import { filterReqHeaders } from './utils' + +export const invokeRequest = async ( + targetUrl: string, + requestInit: { + headers: IncomingMessage['headers'] + method: IncomingMessage['method'] + }, + readableBody?: import('stream').Readable +) => { + const parsedUrl = new URL(targetUrl) + + // force localhost to IPv4 as some DNS may + // resolve to IPv6 instead + if (parsedUrl.hostname === 'localhost') { + parsedUrl.hostname = '127.0.0.1' + } + const invokeHeaders = filterReqHeaders({ + ...requestInit.headers, + }) as IncomingMessage['headers'] + + const invokeRes = await new Promise( + (resolveInvoke, rejectInvoke) => { + const http = require('http') as typeof import('http') + + try { + const invokeReq = http.request( + targetUrl, + { + headers: invokeHeaders, + method: requestInit.method, + }, + (res) => { + resolveInvoke(res) + } + ) + invokeReq.on('error', (err) => { + rejectInvoke(err) + }) + + if (requestInit.method !== 'GET' && requestInit.method !== 'HEAD') { + if (readableBody) { + readableBody.pipe(invokeReq) + readableBody.on('close', () => { + invokeReq.end() + }) + } + } else { + invokeReq.end() + } + } catch (err) { + rejectInvoke(err) + } + } + ) + + return invokeRes +} diff --git a/packages/next/src/server/lib/server-ipc/utils.ts b/packages/next/src/server/lib/server-ipc/utils.ts new file mode 100644 index 0000000000000..2676772b7f3f7 --- /dev/null +++ b/packages/next/src/server/lib/server-ipc/utils.ts @@ -0,0 +1,23 @@ +const forbiddenHeaders = [ + 'accept-encoding', + 'content-length', + 'keepalive', + 'content-encoding', + 'transfer-encoding', + // https://github.com/nodejs/undici/issues/1470 + 'connection', +] + +export const filterReqHeaders = ( + headers: Record +) => { + for (const [key, value] of Object.entries(headers)) { + if ( + forbiddenHeaders.includes(key) || + !(Array.isArray(value) || typeof value === 'string') + ) { + delete headers[key] + } + } + return headers +} diff --git a/packages/next/src/server/next-server.ts b/packages/next/src/server/next-server.ts index 0ebce2758ae79..2b0b9382d0e2d 100644 --- a/packages/next/src/server/next-server.ts +++ b/packages/next/src/server/next-server.ts @@ -106,7 +106,9 @@ import { getRouteRegex } from '../shared/lib/router/utils/route-regex' 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 { invokeRequest } from './lib/server-ipc/invoke-request' +import { filterReqHeaders } from './lib/server-ipc/utils' +import { createRequestResponseMocks } from './lib/mock-request' export * from './base-server' @@ -906,16 +908,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 +1671,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 0000000000000..a3a86a5ca1e12 --- /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 0000000000000..6cc12f77ff9cd --- /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 0000000000000..cfa3ac3d7aa94 --- /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 0000000000000..a3e4482a2bcbc --- /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 0000000000000..1922eb6c3bf31 --- /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) + }) + } +)