diff --git a/packages/next/src/server/web/sandbox/context.ts b/packages/next/src/server/web/sandbox/context.ts index 899758cb46f2a..eecd6b8f958cd 100644 --- a/packages/next/src/server/web/sandbox/context.ts +++ b/packages/next/src/server/web/sandbox/context.ts @@ -452,6 +452,10 @@ Learn More: https://nextjs.org/docs/messages/edge-dynamic-code-evaluation`), context.clearTimeout = (timeout: number) => timeoutsManager.remove(timeout) + if (process.env.__NEXT_TEST_MODE) { + context.__next_outer_globalThis__ = globalThis + } + return context }, }) diff --git a/test/e2e/app-dir/next-after-app/app/delay-deep/page.js b/test/e2e/app-dir/next-after-app/app/delay-deep/page.js new file mode 100644 index 0000000000000..9ff11d857dede --- /dev/null +++ b/test/e2e/app-dir/next-after-app/app/delay-deep/page.js @@ -0,0 +1,55 @@ +import { Suspense } from 'react' +import { unstable_after as after } from 'next/server' +import { cliLog } from '../../utils/log' +import { sleep } from '../../utils/sleep' + +// don't waste time prerendering, after() will bail out anyway +export const dynamic = 'force-dynamic' + +export default async function Page() { + cliLog({ source: '[page] /delay-deep (Page) - render' }) + return ( + + Delay + + ) +} + +async function Inner({ children }) { + cliLog({ + source: '[page] /delay-deep (Inner) - render, sleeping', + }) + + await sleep(1000) + + cliLog({ + source: '[page] /delay-deep (Inner) - render, done sleeping', + }) + + return ( +
+ + {children} + +
+ ) +} + +async function Inner2({ children }) { + cliLog({ + source: '[page] /delay-deep (Inner2) - render, sleeping', + }) + + await sleep(1000) + + cliLog({ + source: '[page] /delay-deep (Inner2) - render, done sleeping', + }) + + after(async () => { + await sleep(1000) + cliLog({ source: '[page] /delay-deep (Inner2) - after' }) + }) + + return <>{children} +} diff --git a/test/e2e/app-dir/next-after-app/app/layout.js b/test/e2e/app-dir/next-after-app/app/layout.js index bece157b43025..255bfe1aed4a0 100644 --- a/test/e2e/app-dir/next-after-app/app/layout.js +++ b/test/e2e/app-dir/next-after-app/app/layout.js @@ -1,7 +1,11 @@ +import { maybeInstallInvocationShutdownHook } from '../utils/simulated-invocation' + // (patched in tests) // export const runtime = 'REPLACE_ME' +// export const dynamic = 'REPLACE_ME' export default function AppLayout({ children }) { + maybeInstallInvocationShutdownHook() return ( diff --git a/test/e2e/app-dir/next-after-app/app/route-streaming/route.js b/test/e2e/app-dir/next-after-app/app/route-streaming/route.js new file mode 100644 index 0000000000000..cc3264ec050b6 --- /dev/null +++ b/test/e2e/app-dir/next-after-app/app/route-streaming/route.js @@ -0,0 +1,46 @@ +import { unstable_after as after } from 'next/server' +import { cliLog } from '../../utils/log' +import { sleep } from '../../utils/sleep' +import { maybeInstallInvocationShutdownHook } from '../../utils/simulated-invocation' + +export const dynamic = 'force-dynamic' + +// (patched in tests) +// export const runtime = 'REPLACE_ME' + +export async function GET() { + maybeInstallInvocationShutdownHook() + + /** @type {ReadableStream} */ + const result = new ReadableStream({ + async start(controller) { + cliLog({ + source: '[route handler] /route-streaming - body, sleeping', + }) + await sleep(500) + cliLog({ + source: '[route handler] /route-streaming - body, done sleeping', + }) + + const encoder = new TextEncoder() + for (const chunk of ['one', 'two', 'three']) { + await sleep(500) + controller.enqueue(encoder.encode(chunk + '\r\n')) + } + + after(async () => { + await sleep(1000) + cliLog({ + source: '[route handler] /route-streaming - after', + }) + }) + controller.close() + }, + }) + return new Response(result, { + headers: { + 'content-type': 'text/plain; charset=utf-8', + 'transfer-encoding': 'chunked', + }, + }) +} diff --git a/test/e2e/app-dir/next-after-app/index.test.ts b/test/e2e/app-dir/next-after-app/index.test.ts index 8e89682e82d9a..bdbed54b9e3ad 100644 --- a/test/e2e/app-dir/next-after-app/index.test.ts +++ b/test/e2e/app-dir/next-after-app/index.test.ts @@ -1,5 +1,5 @@ /* eslint-env jest */ -import { nextTestSetup } from 'e2e-utils' +import { NextInstance, nextTestSetup } from 'e2e-utils' import { retry } from 'next-test-utils' import { createProxyServer } from 'next/experimental/testmode/proxy' import { outdent } from 'outdent' @@ -22,25 +22,42 @@ describe.each(runtimes)('unstable_after() in %s runtime', (runtimeValue) => { }, }) + const filesToPatchRuntime = [ + 'app/layout.js', + 'app/route/route.js', + 'app/route-streaming/route.js', + ] + const replaceRuntime = (contents: string, file: string) => { + const placeholder = `// export const runtime = 'REPLACE_ME'` + + if (!contents.includes(placeholder)) { + throw new Error(`Placeholder "${placeholder}" not found in ${file}`) + } + + return contents.replace( + placeholder, + `export const runtime = '${runtimeValue}'` + ) + } + + const runtimePatches = new Map< + string, + string | ((contents: string) => string) + >( + filesToPatchRuntime.map( + (file) => + [file, (contents: string) => replaceRuntime(contents, file)] as const + ) + ) + { const originalContents: Record = {} beforeAll(async () => { - const placeholder = `// export const runtime = 'REPLACE_ME'` - - const filesToPatch = ['app/layout.js', 'app/route/route.js'] - - for (const file of filesToPatch) { + for (const file of filesToPatchRuntime) { await next.patchFile(file, (contents) => { - if (!contents.includes(placeholder)) { - throw new Error(`Placeholder "${placeholder}" not found in ${file}`) - } originalContents[file] = contents - - return contents.replace( - placeholder, - `export const runtime = '${runtimeValue}'` - ) + return replaceRuntime(contents, file) }) } }) @@ -282,6 +299,7 @@ describe.each(runtimes)('unstable_after() in %s runtime', (runtimeValue) => { const { cleanup } = await sandbox( next, new Map([ + ...runtimePatches, [ // this needs to be injected as early as possible, before the server tries to read the context // (which may be even before we load the page component in dev mode) @@ -312,6 +330,97 @@ describe.each(runtimes)('unstable_after() in %s runtime', (runtimeValue) => { } }) + if (!isNextDev) { + describe('keeps the invocation alive if after() is called late during streaming', () => { + const setup = async () => { + const { cleanup } = await patchSandbox( + next, + new Map string)>([ + ...runtimePatches, + [ + 'app/layout.js', + (contents) => { + contents = replaceRuntime(contents, 'app/layout.js') + + contents = contents.replace( + `// export const dynamic = 'REPLACE_ME'`, + `export const dynamic = 'force-dynamic'` + ) + + return contents + }, + ], + [ + 'utils/simulated-invocation.js', + (contents) => { + return contents.replace( + `const shouldInstallShutdownHook = false`, + `const shouldInstallShutdownHook = true` + ) + }, + ], + [ + // this needs to be injected as early as possible, before the server tries to read the context + // (which may be even before we load the page component in dev mode) + 'instrumentation.js', + outdent` + import { injectRequestContext } from './utils/simulated-invocation' + export function register() { + injectRequestContext(); + } + `, + ], + ]) + ) + + return cleanup + } + + /* eslint-disable jest/no-standalone-expect */ + const it_failingForEdge = runtimeValue === 'edge' ? it.failing : it + + it_failingForEdge('during render', async () => { + const cleanup = await setup() + try { + const response = await next.fetch('/delay-deep') + expect(response.status).toBe(200) + await response.text() + await retry(() => { + expect(getLogs()).toContainEqual('simulated-invocation :: end') + }, 10_000) + + expect(getLogs()).toContainEqual({ + source: '[page] /delay-deep (Inner2) - after', + }) + } finally { + await cleanup() + } + }) + + it_failingForEdge( + 'in a route handler that streams a response', + async () => { + const cleanup = await setup() + try { + const response = await next.fetch('/route-streaming') + expect(response.status).toBe(200) + await response.text() + await retry(() => { + expect(getLogs()).toContainEqual('simulated-invocation :: end') + }, 10_000) + + expect(getLogs()).toContainEqual({ + source: '[route handler] /route-streaming - after', + }) + } finally { + await cleanup() + } + } + ) + /* eslint-enable jest/no-standalone-expect */ + }) + } + if (isNextDev) { // TODO: these are at the end because they destroy the next server. // is there a cleaner way to do this without making the tests slower? @@ -323,12 +432,14 @@ describe.each(runtimes)('unstable_after() in %s runtime', (runtimeValue) => { const { session, cleanup } = await sandbox( next, new Map([ + ...runtimePatches, [ 'app/static/page.js', - (await next.readFile('app/static/page.js')).replace( - `// export const dynamic = 'REPLACE_ME'`, - `export const dynamic = '${dynamicValue}'` - ), + (contents) => + contents.replace( + `// export const dynamic = 'REPLACE_ME'`, + `export const dynamic = '${dynamicValue}'` + ), ], ]), '/static' @@ -350,12 +461,10 @@ describe.each(runtimes)('unstable_after() in %s runtime', (runtimeValue) => { const { session, cleanup } = await sandbox( next, new Map([ + ...runtimePatches, [ 'app/invalid-in-client/page.js', - (await next.readFile('app/invalid-in-client/page.js')).replace( - `// 'use client'`, - `'use client'` - ), + (contents) => contents.replace(`// 'use client'`, `'use client'`), ], ]), '/invalid-in-client' @@ -391,3 +500,24 @@ function timeoutPromise(duration: number, message = 'Timeout') { ) ) } + +async function patchSandbox( + next: NextInstance, + files: Map string)> +) { + await next.stop() + await next.clean() + + for (const [file, newContents] of files) { + await next.patchFile(file, newContents) + } + + await next.start() + + const cleanup = async () => { + await next.stop() + await next.clean() + } + + return { cleanup } +} diff --git a/test/e2e/app-dir/next-after-app/utils/awaiter.ts b/test/e2e/app-dir/next-after-app/utils/awaiter.ts new file mode 100644 index 0000000000000..f4d3214b57868 --- /dev/null +++ b/test/e2e/app-dir/next-after-app/utils/awaiter.ts @@ -0,0 +1,70 @@ +import { InvariantError } from 'next/dist/shared/lib/invariant-error' + +/** + * Provides a `waitUntil` implementation which gathers promises to be awaited later (via {@link AwaiterMulti.awaiting}). + * Unlike a simple `Promise.all`, {@link AwaiterMulti} works recursively -- + * if a promise passed to {@link AwaiterMulti.waitUntil} calls `waitUntil` again, + * that second promise will also be awaited. + */ +export class AwaiterMulti { + private promises: Set> = new Set() + private onError: (error: unknown) => void + + constructor({ onError }: { onError?: (error: unknown) => void } = {}) { + this.onError = onError ?? console.error + } + + public waitUntil = (promise: Promise): void => { + // if a promise settles before we await it, we can drop it. + const cleanup = () => { + this.promises.delete(promise) + } + + this.promises.add( + promise.then(cleanup, (err) => { + cleanup() + this.onError(err) + }) + ) + } + + public async awaiting(): Promise { + while (this.promises.size > 0) { + const promises = Array.from(this.promises) + this.promises.clear() + await Promise.all(promises) + } + } +} + +/** + * Like {@link AwaiterMulti}, but can only be awaited once. + * If {@link AwaiterOnce.waitUntil} is called after that, it will throw. + */ +export class AwaiterOnce { + private awaiter: AwaiterMulti + private done: boolean = false + private pending: Promise | undefined + + constructor(options: { onError?: (error: unknown) => void } = {}) { + this.awaiter = new AwaiterMulti(options) + } + + public waitUntil = (promise: Promise): void => { + if (this.done) { + throw new InvariantError( + 'Cannot call waitUntil() on an AwaiterOnce that was already awaited' + ) + } + return this.awaiter.waitUntil(promise) + } + + public async awaiting(): Promise { + if (!this.pending) { + this.pending = this.awaiter.awaiting().finally(() => { + this.done = true + }) + } + return this.pending + } +} diff --git a/test/e2e/app-dir/next-after-app/utils/simulated-invocation.js b/test/e2e/app-dir/next-after-app/utils/simulated-invocation.js new file mode 100644 index 0000000000000..02e242fb6d97f --- /dev/null +++ b/test/e2e/app-dir/next-after-app/utils/simulated-invocation.js @@ -0,0 +1,178 @@ +import { requestAsyncStorage } from 'next/dist/client/components/request-async-storage.external' +import { AwaiterOnce } from './awaiter' +import { cliLog } from './log' + +// replaced in tests +const shouldInstallShutdownHook = false + +/* +This module is meant to help simulate a serverless invocation, which will shut down when +- the response is finished +- all promises passed to waitUntil are settled +this turns out to be a bit tricky, so here's an explanation of how this works. + +We need two pieces: + +1. `injectRequestContext` - the "waitUntil" part +this injects a mock-ish '@next/request-context' that provides a `waitUntil` +that collects the promises passed to it in an `Awaiter`, so that we can await them later, before exiting. +(we need to call this in instrumentation.ts, because base-server accesses `waitUntil` it before render.) + +2. `installInvocationShutdownHook` - the "when the response is finished" part +registers an `onClose` callback that will await the promises passed to `waitUntil`, +and then shut down the process. +(this can only be called during render/[handing a request], because we need `onClose` to be available) + + +These two pieces are connected via `globalThis[Symbol.for("invocation-context")]`. +This leads to a tricky situation when we're running a handler with `runtime = "edge"`. +In tests/localhost, those handlers will be called in an EdgeRuntime sandbox (see `runEdgeFunction`), +and won't have direct access to the `globalThis` of the "outer" nodejs process. + +So for edge, the flow goes like this: + +1. The "outer" node server starts, and runs `instrumentation.ts` + (injecting our 'invocation-context' that contains the "outer" `waitUntil`) + +2. The outer server gets a request, and runs the edge handler + (wrapped with `server/web/adapter`) inside `runEdgeFunction`, i.e. in an EdgeRuntime sandbox. + + and then, within the sandbox: + + 3. `server/web/adapter` creates an "inner" `waitUntil` (see `NextFetchEvent.waitUntil`). + This is what `unstable_after` calls will use. + Notably, `adapter`'s `waitUntil` doesn't do much on its own -- + it only collects the promises, which `adapter` then returns as part of a FetchEventResult, + expecting its caller to pass them to a "real" `waitUntil`. + I'm not sure why this inversion exists, but that's what it does. + + 4. the edge handler creates an "inner" NextWebServer + - NOTE: this ALSO runs `instrumentation.ts`, so we need to make sure that + we don't create a second '@next/request-context' here + + 5. Rendering (or other request handling) happens within the sandbox. + + 6. During render, we install the shutdown hook, and will call it in onClose. + + - NOTE: as outlined above, `installInvocationShutdownHook` runs in the edge sandbox, + but it needs to access the "outer" globalThis. + - NOTE 2: we also need to be able to call `process.exit` from here, + which means that `invocationContext.shutdownHook` has to be passed in from the nodejs runtime -- + otherwise, edge compilation will replace it with a stub that calls `throwUnsupportedAPIError`. + + 7. the render hadnler returns a Response. `adapter` takes it and the promises passed to the inner `waitUntil` + and puts them on a FetchEventResult. + - NOTE: **no calls to the "outer" `waitUntil` occurred yet!** + all that happened is that `adapter`'s Awaiter collected them. + +8. The "outer" server gets back the FetchEventResult, and passes the promise from that to the "outer" waitUntil. + This finally puts a promise into the Awaiter we created in `injectRequestContext`. + +9. The response finishes, and `onClose` calls the shutdown hook (from inside the edge sandbox). + we await the single promise that got added to the awaiter, and finally shutdown the process. + +*/ + +function createInvocationContext() { + const awaiter = new AwaiterOnce() + + const waitUntil = (promise) => { + awaiter.waitUntil(promise) + } + + const shutdownHook = async () => { + cliLog(`Request finished, waiting for \`waitUntil\` promises`) + await awaiter.awaiting() + cliLog('simulated-invocation :: end') + process.exit(0) + } + + return { awaiter, waitUntil, shutdownHook } +} + +const INVOCATION_CONTEXT = Symbol.for('invocation-context') + +/** Install a '@next/request-context' that will collect promises passed to `waitUntil` */ +export function injectRequestContext() { + // if we're in a edge runtime sandbox, skip installing -- + // we already installed this "outside", in the nodejs runtime. + // (and process.exit won't work anyway) + if (process.env.NEXT_RUNTIME === 'edge' && getOuterGlobalThisInSandbox()) { + return + } + + const globalThis = resolveGlobalThis() + + if (globalThis[INVOCATION_CONTEXT]) { + throw new Error('Cannot call `injectRequestContext` twice') + } + + const invocationContext = createInvocationContext() + + /** @type {import('next/dist/server/after/builtin-request-context').BuiltinRequestContext} */ + globalThis[Symbol.for('@next/request-context')] = { + get() { + return { + waitUntil: invocationContext.waitUntil, + } + }, + } + globalThis[INVOCATION_CONTEXT] = invocationContext +} + +export function maybeInstallInvocationShutdownHook() { + if (!shouldInstallShutdownHook) { + return + } + installInvocationShutdownHook() +} + +/** Schedule a shutdown when the response is done and all `waitUntil` promises settled */ +export function installInvocationShutdownHook() { + const globalThis = resolveGlobalThis() + const context = globalThis[INVOCATION_CONTEXT] + + if (!context) { + throw new Error('Missing invocation context') + } + + onClose(() => { + context.shutdownHook() + }) +} + +function onClose(/** @type {() => void} */ callback) { + // this is a hack, but we don't want to do this with an after() + // because that'll call `waitUntil` and affect what we're trying to test here + const store = requestAsyncStorage.getStore() + if (!store) { + throw new Error('Could not access request store') + } + const ctx = store.afterContext + // AfterContextImpl has an `onClose` property, it's just not exposed on the interface + if (typeof ctx?.['onClose'] !== 'function') { + throw new Error('Could not access `onClose` from afterContext') + } + return ctx['onClose'](callback) +} + +/** Get the real `globalThis`, regardless if we're in the actual server or an edge sandbox. */ +const resolveGlobalThis = () => { + if (process.env.NEXT_RUNTIME === 'edge') { + const obj = getOuterGlobalThisInSandbox() + if (!obj) { + throw new Error('__next_outer_globalThis__ is not defined') + } + return obj + } + + // eslint-disable-next-line no-undef + const _globalThis = globalThis + return _globalThis +} + +const getOuterGlobalThisInSandbox = () => { + // eslint-disable-next-line no-undef + const _globalThis = globalThis + return _globalThis.__next_outer_globalThis__ +} diff --git a/test/e2e/app-dir/next-after-app/utils/sleep.js b/test/e2e/app-dir/next-after-app/utils/sleep.js new file mode 100644 index 0000000000000..283b4c2083c90 --- /dev/null +++ b/test/e2e/app-dir/next-after-app/utils/sleep.js @@ -0,0 +1,4 @@ +/** @returns {Promise} */ +export function sleep(/** @type {number} */ duration) { + return new Promise((resolve) => setTimeout(resolve, duration)) +} diff --git a/test/lib/development-sandbox.ts b/test/lib/development-sandbox.ts index d5a301a2588f8..e31e385300b76 100644 --- a/test/lib/development-sandbox.ts +++ b/test/lib/development-sandbox.ts @@ -32,7 +32,7 @@ export function waitForHydration(browser: BrowserInterface): Promise { export async function sandbox( next: NextInstance, - initialFiles?: Map, + initialFiles?: Map string)>, initialUrl: string = '/', webDriverOptions: any = undefined ) { diff --git a/test/lib/next-modes/base.ts b/test/lib/next-modes/base.ts index b767dfd628fe1..103075a3759ce 100644 --- a/test/lib/next-modes/base.ts +++ b/test/lib/next-modes/base.ts @@ -356,7 +356,14 @@ export class NextInstance { public async stop(): Promise { this.isStopping = true if (this.childProcess) { - const exitPromise = once(this.childProcess, 'exit') + const alreadyExited = + this.childProcess.exitCode !== null || + this.childProcess.signalCode !== null + + const exitPromise = alreadyExited + ? Promise.resolve() + : once(this.childProcess, 'exit') + await new Promise((resolve) => { treeKill(this.childProcess.pid, 'SIGKILL', (err) => { if (err) { @@ -370,6 +377,7 @@ export class NextInstance { this.childProcess = undefined require('console').log(`Stopped next server`) } + this.isStopping = false } public async destroy(): Promise { @@ -444,7 +452,7 @@ export class NextInstance { // This is a temporary workaround for turbopack starting watching too late. // So we delay file changes by 500ms to give it some time // to connect the WebSocket and start watching. - if (process.env.TURBOPACK) { + if (isNextDev && process.env.TURBOPACK) { require('console').log('fs dev delay before', filename) await waitFor(500) }