From b0d1bd3a7cebe9c46cce61b1d8cca4997ca15498 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 3 Oct 2024 20:16:22 -0600 Subject: [PATCH 1/2] feat: decouple cookies and action state from redirect error --- .../next/src/client/components/redirect.ts | 50 +- packages/next/src/server/api-utils/index.ts | 2 +- .../src/server/app-render/action-handler.ts | 18 +- .../next/src/server/app-render/app-render.tsx | 86 +-- packages/next/src/server/base-server.ts | 10 +- packages/next/src/server/lib/trace/tracer.ts | 8 + packages/next/src/server/render.tsx | 2 +- .../helpers/auto-implement-methods.ts | 5 +- .../helpers/resolve-handler-error.ts | 32 - .../server/route-modules/app-route/module.ts | 615 +++++++++--------- .../helpers/response-handlers.ts | 30 - 11 files changed, 385 insertions(+), 473 deletions(-) delete mode 100644 packages/next/src/server/route-modules/app-route/helpers/resolve-handler-error.ts delete mode 100644 packages/next/src/server/route-modules/helpers/response-handlers.ts diff --git a/packages/next/src/client/components/redirect.ts b/packages/next/src/client/components/redirect.ts index fe787f5e94bfb..280ed1d2a763c 100644 --- a/packages/next/src/client/components/redirect.ts +++ b/packages/next/src/client/components/redirect.ts @@ -1,5 +1,3 @@ -import { requestAsyncStorage } from './request-async-storage.external' -import type { ResponseCookies } from '../../server/web/spec-extension/cookies' import { actionAsyncStorage } from './action-async-storage.external' import { RedirectStatusCode } from './redirect-status-code' @@ -10,22 +8,17 @@ export enum RedirectType { replace = 'replace', } -export type RedirectError = Error & { - digest: `${typeof REDIRECT_ERROR_CODE};${RedirectType};${U};${RedirectStatusCode};` - mutableCookies: ResponseCookies +export type RedirectError = Error & { + digest: `${typeof REDIRECT_ERROR_CODE};${RedirectType};${string};${RedirectStatusCode};` } export function getRedirectError( url: string, type: RedirectType, statusCode: RedirectStatusCode = RedirectStatusCode.TemporaryRedirect -): RedirectError { - const error = new Error(REDIRECT_ERROR_CODE) as RedirectError +): RedirectError { + const error = new Error(REDIRECT_ERROR_CODE) as RedirectError error.digest = `${REDIRECT_ERROR_CODE};${type};${url};${statusCode};` - const requestStore = requestAsyncStorage.getStore() - if (requestStore) { - error.mutableCookies = requestStore.mutableCookies - } return error } @@ -52,12 +45,7 @@ export function redirect( throw getRedirectError( url, redirectType, - // If we're in an action, we want to use a 303 redirect - // as we don't want the POST request to follow the redirect, - // as it could result in erroneous re-submissions. - actionStore?.isAction - ? RedirectStatusCode.SeeOther - : RedirectStatusCode.TemporaryRedirect + RedirectStatusCode.TemporaryRedirect ) } @@ -77,17 +65,7 @@ export function permanentRedirect( url: string, type: RedirectType = RedirectType.replace ): never { - const actionStore = actionAsyncStorage.getStore() - throw getRedirectError( - url, - type, - // If we're in an action, we want to use a 303 redirect - // as we don't want the POST request to follow the redirect, - // as it could result in erroneous re-submissions. - actionStore?.isAction - ? RedirectStatusCode.SeeOther - : RedirectStatusCode.PermanentRedirect - ) + throw getRedirectError(url, type, RedirectStatusCode.PermanentRedirect) } /** @@ -97,9 +75,7 @@ export function permanentRedirect( * @param error the error that may reference a redirect error * @returns true if the error is a redirect error */ -export function isRedirectError( - error: unknown -): error is RedirectError { +export function isRedirectError(error: unknown): error is RedirectError { if ( typeof error !== 'object' || error === null || @@ -132,9 +108,7 @@ export function isRedirectError( * @param error the error that may be a redirect error * @return the url if the error was a redirect error */ -export function getURLFromRedirectError( - error: RedirectError -): U +export function getURLFromRedirectError(error: RedirectError): string export function getURLFromRedirectError(error: unknown): string | null { if (!isRedirectError(error)) return null @@ -143,9 +117,7 @@ export function getURLFromRedirectError(error: unknown): string | null { return error.digest.split(';').slice(2, -2).join(';') } -export function getRedirectTypeFromError( - error: RedirectError -): RedirectType { +export function getRedirectTypeFromError(error: RedirectError): RedirectType { if (!isRedirectError(error)) { throw new Error('Not a redirect error') } @@ -153,9 +125,7 @@ export function getRedirectTypeFromError( return error.digest.split(';', 2)[1] as RedirectType } -export function getRedirectStatusCodeFromError( - error: RedirectError -): number { +export function getRedirectStatusCodeFromError(error: RedirectError): number { if (!isRedirectError(error)) { throw new Error('Not a redirect error') } diff --git a/packages/next/src/server/api-utils/index.ts b/packages/next/src/server/api-utils/index.ts index f4819ef100850..b28ac1cc498a4 100644 --- a/packages/next/src/server/api-utils/index.ts +++ b/packages/next/src/server/api-utils/index.ts @@ -25,7 +25,7 @@ export function wrapApiHandler any>( handler: T ): T { return ((...args) => { - getTracer().getRootSpanAttributes()?.set('next.route', page) + getTracer().setRootSpanAttribute('next.route', page) // Call API route method return getTracer().trace( NodeSpan.runHandler, diff --git a/packages/next/src/server/app-render/action-handler.ts b/packages/next/src/server/app-render/action-handler.ts index 4b38a37251cb9..80ffd3bfa9038 100644 --- a/packages/next/src/server/app-render/action-handler.ts +++ b/packages/next/src/server/app-render/action-handler.ts @@ -13,7 +13,6 @@ import { } from '../../client/components/app-router-headers' import { isNotFoundError } from '../../client/components/not-found' import { - getRedirectStatusCodeFromError, getRedirectTypeFromError, getURLFromRedirectError, isRedirectError, @@ -43,6 +42,7 @@ import { HeadersAdapter } from '../web/spec-extension/adapters/headers' import { fromNodeOutgoingHttpHeaders } from '../web/utils' import { selectWorkerForForwarding } from './action-utils' import { isNodeNextRequest, isWebNextRequest } from '../base-http/helpers' +import { RedirectStatusCode } from '../../client/components/redirect-status-code' function formDataFromSearchQueryString(query: string) { const searchParams = new URLSearchParams(query) @@ -855,7 +855,6 @@ export async function handleAction({ } catch (err) { if (isRedirectError(err)) { const redirectUrl = getURLFromRedirectError(err) - const statusCode = getRedirectStatusCodeFromError(err) const redirectType = getRedirectTypeFromError(err) await addRevalidationHeader(res, { @@ -865,7 +864,7 @@ export async function handleAction({ // if it's a fetch action, we'll set the status code for logging/debugging purposes // but we won't set a Location header, as the redirect will be handled by the client router - res.statusCode = statusCode + res.statusCode = RedirectStatusCode.SeeOther if (isFetchAction) { return { @@ -882,14 +881,11 @@ export async function handleAction({ } } - if (err.mutableCookies) { - const headers = new Headers() - - // If there were mutable cookies set, we need to set them on the - // response. - if (appendMutableCookies(headers, err.mutableCookies)) { - res.setHeader('set-cookie', Array.from(headers.values())) - } + // If there were mutable cookies set, we need to set them on the + // response. + const headers = new Headers() + if (appendMutableCookies(headers, requestStore.mutableCookies)) { + res.setHeader('set-cookie', Array.from(headers.values())) } res.setHeader('Location', redirectUrl) diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index 47578c41893d6..dc6858e3ce64c 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -1043,7 +1043,7 @@ async function renderToHTMLOrFlightImpl( res, } - getTracer().getRootSpanAttributes()?.set('next.route', pagePath) + getTracer().setRootSpanAttribute('next.route', pagePath) if (isStaticGeneration) { // We're either building or revalidating. In either case we need to @@ -1576,40 +1576,32 @@ async function renderToStream( throw err } + let errorType: 'not-found' | 'redirect' | undefined + if (isNotFoundError(err)) { + errorType = 'not-found' res.statusCode = 404 - } - let hasRedirectError = false - if (isRedirectError(err)) { - hasRedirectError = true + } else if (isRedirectError(err)) { + errorType = 'redirect' res.statusCode = getRedirectStatusCodeFromError(err) - if (err.mutableCookies) { - const headers = new Headers() - // If there were mutable cookies set, we need to set them on the - // response. - if (appendMutableCookies(headers, err.mutableCookies)) { - setHeader('set-cookie', Array.from(headers.values())) - } - } const redirectUrl = addPathPrefix( getURLFromRedirectError(err), renderOpts.basePath ) - setHeader('Location', redirectUrl) - } - const is404 = res.statusCode === 404 - if (!is404 && !hasRedirectError && !shouldBailoutToCSR) { + // If there were mutable cookies set, we need to set them on the + // response. + const headers = new Headers() + if (appendMutableCookies(headers, ctx.requestStore.mutableCookies)) { + setHeader('set-cookie', Array.from(headers.values())) + } + + setHeader('location', redirectUrl) + } else if (!shouldBailoutToCSR) { res.statusCode = 500 } - const errorType = is404 - ? 'not-found' - : hasRedirectError - ? 'redirect' - : undefined - const [errorPreinitScripts, errorBootstrapScript] = getRequiredScripts( renderOpts.buildManifest, ctx.assetPrefix, @@ -2724,46 +2716,38 @@ async function prerenderToStream( throw err } + // If we errored when we did not have an RSC stream to read from. This is + // not just a render error, we need to throw early. + if (reactServerPrerenderResult === null) { + throw err + } + + let errorType: 'not-found' | 'redirect' | undefined + if (isNotFoundError(err)) { + errorType = 'not-found' res.statusCode = 404 - } - let hasRedirectError = false - if (isRedirectError(err)) { - hasRedirectError = true + } else if (isRedirectError(err)) { + errorType = 'redirect' res.statusCode = getRedirectStatusCodeFromError(err) - if (err.mutableCookies) { - const headers = new Headers() - // If there were mutable cookies set, we need to set them on the - // response. - if (appendMutableCookies(headers, err.mutableCookies)) { - setHeader('set-cookie', Array.from(headers.values())) - } - } const redirectUrl = addPathPrefix( getURLFromRedirectError(err), renderOpts.basePath ) - setHeader('Location', redirectUrl) - } - const is404 = res.statusCode === 404 - if (!is404 && !hasRedirectError && !shouldBailoutToCSR) { - res.statusCode = 500 - } + // If there were mutable cookies set, we need to set them on the + // response. + const headers = new Headers() + if (appendMutableCookies(headers, ctx.requestStore.mutableCookies)) { + setHeader('set-cookie', Array.from(headers.values())) + } - if (reactServerPrerenderResult === null) { - // We errored when we did not have an RSC stream to read from. This is not just a render - // error, we need to throw early - throw err + setHeader('location', redirectUrl) + } else if (!shouldBailoutToCSR) { + res.statusCode = 500 } - const errorType = is404 - ? 'not-found' - : hasRedirectError - ? 'redirect' - : undefined - const [errorPreinitScripts, errorBootstrapScript] = getRequiredScripts( renderOpts.buildManifest, ctx.assetPrefix, diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index b0b71b119cf6b..3412ad5d69cc1 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -123,10 +123,6 @@ import { getTracer, isBubbledError, SpanKind } from './lib/trace/tracer' import { BaseServerSpan } from './lib/trace/constants' import { I18NProvider } from './lib/i18n-provider' import { sendResponse } from './send-response' -import { - handleBadRequestResponse, - handleInternalServerErrorResponse, -} from './route-modules/helpers/response-handlers' import { fromNodeOutgoingHttpHeaders, normalizeNextQueryParam, @@ -2587,7 +2583,7 @@ export default abstract class Server< Log.error(err) // Otherwise, send a 500 response. - await sendResponse(req, res, handleInternalServerErrorResponse()) + await sendResponse(req, res, new Response(null, { status: 500 })) return null } @@ -2597,7 +2593,7 @@ export default abstract class Server< ) { // An OPTIONS request to a page handler is invalid. if (req.method === 'OPTIONS' && !is404Page) { - await sendResponse(req, res, handleBadRequestResponse()) + await sendResponse(req, res, new Response(null, { status: 400 })) return null } @@ -3575,7 +3571,7 @@ export default abstract class Server< shouldEnsure: false, }) if (result) { - getTracer().getRootSpanAttributes()?.set('next.route', pathname) + getTracer().setRootSpanAttribute('next.route', pathname) try { return await this.renderToResponseWithComponents(ctx, result) } catch (err) { diff --git a/packages/next/src/server/lib/trace/tracer.ts b/packages/next/src/server/lib/trace/tracer.ts index 78c544209395c..cd2502d9742c7 100644 --- a/packages/next/src/server/lib/trace/tracer.ts +++ b/packages/next/src/server/lib/trace/tracer.ts @@ -450,6 +450,14 @@ class NextTracerImpl implements NextTracer { const spanId = context.active().getValue(rootSpanIdKey) as number return rootSpanAttributesStore.get(spanId) } + + public setRootSpanAttribute(key: AttributeNames, value: AttributeValue) { + const spanId = context.active().getValue(rootSpanIdKey) as number + const attributes = rootSpanAttributesStore.get(spanId) + if (attributes) { + attributes.set(key, value) + } + } } const getTracer = (() => { diff --git a/packages/next/src/server/render.tsx b/packages/next/src/server/render.tsx index ae913fb6c3a64..b7e4b3158ab91 100644 --- a/packages/next/src/server/render.tsx +++ b/packages/next/src/server/render.tsx @@ -1391,7 +1391,7 @@ export async function renderToHTMLImpl( } } - getTracer().getRootSpanAttributes()?.set('next.route', renderOpts.page) + getTracer().setRootSpanAttribute('next.route', renderOpts.page) const documentResult = await getTracer().trace( RenderSpan.renderDocument, { diff --git a/packages/next/src/server/route-modules/app-route/helpers/auto-implement-methods.ts b/packages/next/src/server/route-modules/app-route/helpers/auto-implement-methods.ts index 05f5dc96cf965..3cb4f2d0f118d 100644 --- a/packages/next/src/server/route-modules/app-route/helpers/auto-implement-methods.ts +++ b/packages/next/src/server/route-modules/app-route/helpers/auto-implement-methods.ts @@ -1,10 +1,13 @@ import type { AppRouteHandlerFn, AppRouteHandlers } from '../module' import { HTTP_METHODS, type HTTP_METHOD } from '../../../web/http' -import { handleMethodNotAllowedResponse } from '../../helpers/response-handlers' const AUTOMATIC_ROUTE_METHODS = ['HEAD', 'OPTIONS'] as const +function handleMethodNotAllowedResponse(): Response { + return new Response(null, { status: 405 }) +} + export function autoImplementMethods( handlers: AppRouteHandlers ): Record { diff --git a/packages/next/src/server/route-modules/app-route/helpers/resolve-handler-error.ts b/packages/next/src/server/route-modules/app-route/helpers/resolve-handler-error.ts deleted file mode 100644 index 611ba66065c52..0000000000000 --- a/packages/next/src/server/route-modules/app-route/helpers/resolve-handler-error.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { isNotFoundError } from '../../../../client/components/not-found' -import { - getURLFromRedirectError, - isRedirectError, - getRedirectStatusCodeFromError, -} from '../../../../client/components/redirect' -import { - handleNotFoundResponse, - handleRedirectResponse, -} from '../../helpers/response-handlers' - -export function resolveHandlerError(err: any): Response | false { - if (isRedirectError(err)) { - const redirect = getURLFromRedirectError(err) - if (!redirect) { - throw new Error('Invariant: Unexpected redirect url format') - } - - const status = getRedirectStatusCodeFromError(err) - - // This is a redirect error! Send the redirect response. - return handleRedirectResponse(redirect, err.mutableCookies, status) - } - - if (isNotFoundError(err)) { - // This is a not found error! Send the not found response. - return handleNotFoundResponse() - } - - // Return false to indicate that this is not a handled error. - return false -} diff --git a/packages/next/src/server/route-modules/app-route/module.ts b/packages/next/src/server/route-modules/app-route/module.ts index 48e73f521fb41..69ca602d81e35 100644 --- a/packages/next/src/server/route-modules/app-route/module.ts +++ b/packages/next/src/server/route-modules/app-route/module.ts @@ -19,16 +19,11 @@ import { withWorkStore, type WorkStoreContext, } from '../../async-storage/with-work-store' -import { - handleBadRequestResponse, - handleInternalServerErrorResponse, -} from '../helpers/response-handlers' import { type HTTP_METHOD, HTTP_METHODS, isHTTPMethod } from '../../web/http' import { addImplicitTags, patchFetch } from '../../lib/patch-fetch' import { getTracer } from '../../lib/trace/tracer' import { AppRouteRouteHandlersSpan } from '../../lib/trace/constants' import { getPathnameFromAbsolutePath } from './helpers/get-pathname-from-absolute-path' -import { resolveHandlerError } from './helpers/resolve-handler-error' import * as Log from '../../../build/output/log' import { autoImplementMethods } from './helpers/auto-implement-methods' import { @@ -42,7 +37,10 @@ import { parsedUrlQueryToParams } from './helpers/parsed-url-query-to-params' import * as serverHooks from '../../../client/components/hooks-server-context' import { DynamicServerError } from '../../../client/components/hooks-server-context' -import { requestAsyncStorage } from '../../../client/components/request-async-storage.external' +import { + requestAsyncStorage, + type RequestStore, +} from '../../../client/components/request-async-storage.external' import { workAsyncStorage, type WorkStore, @@ -51,7 +49,10 @@ import { prerenderAsyncStorage, type PrerenderStore, } from '../../app-render/prerender-async-storage.external' -import { actionAsyncStorage } from '../../../client/components/action-async-storage.external' +import { + actionAsyncStorage, + type ActionStore, +} from '../../../client/components/action-async-storage.external' import { cacheAsyncStorage } from '../../../server/app-render/cache-async-storage.external' import * as sharedModules from './shared-modules' import { getIsServerAction } from '../../lib/server-action-request-meta' @@ -71,6 +72,21 @@ import { CacheSignal } from '../../app-render/cache-signal' import { scheduleImmediate } from '../../../lib/scheduler' import { createServerParamsForRoute } from '../../request/params' import type { AppSegment } from '../../../build/app-segments/collect-app-segments' +import { + getRedirectStatusCodeFromError, + getURLFromRedirectError, + isRedirectError, + type RedirectError, +} from '../../../client/components/redirect' +import { isNotFoundError } from '../../../client/components/not-found' +import { RedirectStatusCode } from '../../../client/components/redirect-status-code' + +export class WrappedNextRouterError { + constructor( + public readonly error: RedirectError, + public readonly headers?: Headers + ) {} +} /** * The AppRouteModule is the type of the module exported by the bundled App @@ -261,27 +277,281 @@ export class AppRouteRouteModule extends RouteModule< */ private resolve(method: string): AppRouteHandlerFn { // Ensure that the requested method is a valid method (to prevent RCE's). - if (!isHTTPMethod(method)) return handleBadRequestResponse + if (!isHTTPMethod(method)) return () => new Response(null, { status: 400 }) // Return the handler. return this.methods[method] } - /** - * Executes the route handler. - */ - private async execute( - rawRequest: NextRequest, + private async do( + handler: AppRouteHandlerFn, + actionStore: ActionStore, + workStore: WorkStore, + requestStore: RequestStore, + request: NextRequest, + context: AppRouteRouteHandlerContext + ) { + const isStaticGeneration = workStore.isStaticGeneration + const dynamicIOEnabled = !!context.renderOpts.experimental?.dynamicIO + + // Patch the global fetch. + patchFetch({ + workAsyncStorage: this.workAsyncStorage, + requestAsyncStorage: this.requestAsyncStorage, + prerenderAsyncStorage: this.prerenderAsyncStorage, + }) + + const handlerContext: AppRouteHandlerFnContext = { + params: context.params + ? createServerParamsForRoute( + parsedUrlQueryToParams(context.params), + workStore + ) + : undefined, + } + + let res: unknown + try { + if (isStaticGeneration && dynamicIOEnabled) { + /** + * When we are attempting to statically prerender the GET handler of a route.ts module + * and dynamicIO is on we follow a similar pattern to rendering. + * + * We first run the handler letting caches fill. If something synchronously dynamic occurs + * during this prospective render then we can infer it will happen on every render and we + * just bail out of prerendering. + * + * Next we run the handler again and we check if we get a result back in a microtask. + * Next.js expects the return value to be a Response or a Thenable that resolves to a Response. + * Unfortunately Response's do not allow for acessing the response body synchronously or in + * a microtask so we need to allow one more task to unwrap the response body. This is a slightly + * different semantic than what we have when we render and it means that certain tasks can still + * execute before a prerender completes such as a carefully timed setImmediate. + * + * Functionally though IO should still take longer than the time it takes to unwrap the response body + * so our heuristic of excluding any IO should be preserved. + */ + let prospectiveRenderIsDynamic = false + const cacheSignal = new CacheSignal() + let dynamicTracking = createDynamicTrackingState(undefined) + const prospectiveRoutePrerenderStore: PrerenderStore = { + type: 'prerender', + cacheSignal, + // During prospective render we don't use a controller + // because we need to let all caches fill. + controller: null, + dynamicTracking, + } + let prospectiveResult + try { + prospectiveResult = this.prerenderAsyncStorage.run( + prospectiveRoutePrerenderStore, + handler, + request, + handlerContext + ) + } catch (err) { + if (isPrerenderInterruptedError(err)) { + // the route handler called an API which is always dynamic + // there is no need to try again + prospectiveRenderIsDynamic = true + } + } + if ( + typeof prospectiveResult === 'object' && + prospectiveResult !== null && + typeof (prospectiveResult as any).then === 'function' + ) { + // The handler returned a Thenable. We'll listen for rejections to determine + // if the route is erroring for dynamic reasons. + ;(prospectiveResult as any as Promise).then( + () => {}, + (err) => { + if (isPrerenderInterruptedError(err)) { + // the route handler called an API which is always dynamic + // there is no need to try again + prospectiveRenderIsDynamic = true + } + } + ) + } + await cacheSignal.cacheReady() + + if (prospectiveRenderIsDynamic) { + // the route handler called an API which is always dynamic + // there is no need to try again + const dynamicReason = getFirstDynamicReason(dynamicTracking) + if (dynamicReason) { + throw new DynamicServerError( + `Route ${workStore.route} couldn't be rendered statically because it used \`${dynamicReason}\`. See more info here: https://nextjs.org/docs/messages/dynamic-server-error` + ) + } else { + console.error( + 'Expected Next.js to keep track of reason for opting out of static rendering but one was not found. This is a bug in Next.js' + ) + throw new DynamicServerError( + `Route ${workStore.route} couldn't be rendered statically because it used a dynamic API. See more info here: https://nextjs.org/docs/messages/dynamic-server-error` + ) + } + } + + // TODO start passing this controller to the route handler. We should expose + // it so the handler to abort inflight requests and other operations if we abort + // the prerender. + const controller = new AbortController() + dynamicTracking = createDynamicTrackingState(undefined) + + const finalRoutePrerenderStore: PrerenderStore = { + type: 'prerender', + cacheSignal: null, + controller, + dynamicTracking, + } + + let responseHandled = false + res = await new Promise((resolve, reject) => { + scheduleImmediate(async () => { + try { + const result = await (this.prerenderAsyncStorage.run( + finalRoutePrerenderStore, + handler, + request, + handlerContext + ) as Promise) + if (responseHandled) { + // we already rejected in the followup task + return + } else if (!(result instanceof Response)) { + // This is going to error but we let that happen below + resolve(result) + return + } + + responseHandled = true + + let bodyHandled = false + result.arrayBuffer().then((body) => { + if (!bodyHandled) { + bodyHandled = true + + resolve( + new Response(body, { + headers: result.headers, + status: result.status, + statusText: result.statusText, + }) + ) + } + }, reject) + scheduleImmediate(() => { + if (!bodyHandled) { + bodyHandled = true + controller.abort() + reject(createDynamicIOError(workStore.route)) + } + }) + } catch (err) { + reject(err) + } + }) + scheduleImmediate(() => { + if (!responseHandled) { + responseHandled = true + controller.abort() + reject(createDynamicIOError(workStore.route)) + } + }) + }) + } else if (isStaticGeneration) { + res = await prerenderAsyncStorage.run( + { + type: 'prerender-legacy', + }, + handler, + request, + handlerContext + ) + } else { + res = await handler(request, handlerContext) + } + } catch (err) { + if (isRedirectError(err)) { + const url = getURLFromRedirectError(err) + if (!url) { + throw new Error('Invariant: Unexpected redirect url format') + } + + // We need to capture any headers that should be sent on + // the response. + const headers = new Headers() + + // Let's append any cookies that were added by the + // cookie API. + appendMutableCookies(headers, requestStore.mutableCookies) + + // Return the redirect response. + return new Response(null, { + // If we're in an action, we want to use a 303 redirect as we don't + // want the POST request to follow the redirect, as it could result in + // erroneous re-submissions. + status: actionStore.isAction + ? RedirectStatusCode.SeeOther + : getRedirectStatusCodeFromError(err), + headers, + }) + } else if (isNotFoundError(err)) { + return new Response(null, { status: 404 }) + } + + throw err + } + + // Validate that the response is a valid response object. + if (!(res instanceof Response)) { + throw new Error( + `No response is returned from route handler '${this.resolvedPagePath}'. Ensure you return a \`Response\` or a \`NextResponse\` in all branches of your handler.` + ) + } + + context.renderOpts.fetchMetrics = workStore.fetchMetrics + + context.renderOpts.pendingWaitUntil = Promise.all([ + workStore.incrementalCache?.revalidateTag( + workStore.revalidatedTags || [] + ), + ...Object.values(workStore.pendingRevalidates || {}), + ]) + + addImplicitTags(workStore, requestStore, undefined) + ;(context.renderOpts as any).fetchTags = workStore.tags?.join(',') + + // It's possible cookies were set in the handler, so we need + // to merge the modified cookies and the returned response + // here. + const headers = new Headers(res.headers) + if (appendMutableCookies(headers, requestStore.mutableCookies)) { + return new Response(res.body, { + status: res.status, + statusText: res.statusText, + headers, + }) + } + + return res + } + + public async handle( + req: NextRequest, context: AppRouteRouteHandlerContext ): Promise { // Get the handler function for the given method. - const handler = this.resolve(rawRequest.method) + const handler = this.resolve(req.method) // Get the context for the request. const requestContext: RequestContext = { - req: rawRequest, + req, res: undefined, - url: rawRequest.nextUrl, + url: req.nextUrl, renderOpts: { previewProps: context.prerenderManifest.preview, waitUntil: context.renderOpts.waitUntil, @@ -290,8 +560,6 @@ export class AppRouteRouteModule extends RouteModule< }, } - const dynamicIOEnabled = !!context.renderOpts.experimental?.dynamicIO - // Get the context for the static generation. const staticGenerationContext: WorkStoreContext = { // App Routes don't support unknown route params. @@ -303,14 +571,16 @@ export class AppRouteRouteModule extends RouteModule< // Add the fetchCache option to the renderOpts. staticGenerationContext.renderOpts.fetchCache = this.userland.fetchCache + const actionStore: ActionStore = { + isAppRoute: true, + isAction: getIsServerAction(req), + } + // Run the handler with the request AsyncLocalStorage to inject the helper // support. We set this to `unknown` because the type is not known until // runtime when we do a instanceof check below. const response: unknown = await this.actionAsyncStorage.run( - { - isAppRoute: true, - isAction: getIsServerAction(rawRequest), - }, + actionStore, () => withRequestStore( this.requestAsyncStorage, @@ -319,13 +589,11 @@ export class AppRouteRouteModule extends RouteModule< withWorkStore( this.workAsyncStorage, staticGenerationContext, - (workStore) => { + async (workStore) => { // Check to see if we should bail out of static generation based on // having non-static methods. - const isStaticGeneration = workStore.isStaticGeneration - if (this.hasNonStaticMethods) { - if (isStaticGeneration) { + if (workStore.isStaticGeneration) { const err = new DynamicServerError( 'Route is configured with methods that cannot be statically generated.' ) @@ -344,7 +612,7 @@ export class AppRouteRouteModule extends RouteModule< // We assume we can pass the original request through however we may end up // proxying it in certain circumstances based on execution type and configuration - let request = rawRequest + let request = req // Update the static generation store based on the dynamic property. switch (this.dynamic) { @@ -359,21 +627,18 @@ export class AppRouteRouteModule extends RouteModule< workStore.forceStatic = true // We also Proxy the request to replace dynamic data on the request // with empty stubs to allow for safely executing as static - request = new Proxy(rawRequest, forceStaticRequestHandlers) + request = new Proxy(req, forceStaticRequestHandlers) break case 'error': // The dynamic property is set to error, so we should throw an // error if the page is being statically generated. workStore.dynamicShouldError = true - if (isStaticGeneration) - request = new Proxy( - rawRequest, - requireStaticRequestHandlers - ) + if (workStore.isStaticGeneration) + request = new Proxy(req, requireStaticRequestHandlers) break default: // We proxy `NextRequest` to track dynamic access, and potentially bail out of static generation - request = proxyNextRequest(rawRequest, workStore) + request = proxyNextRequest(req, workStore) } // If the static generation store does not have a revalidate value @@ -383,8 +648,13 @@ export class AppRouteRouteModule extends RouteModule< // TODO: propagate this pathname from route matcher const route = getPathnameFromAbsolutePath(this.resolvedPagePath) - getTracer().getRootSpanAttributes()?.set('next.route', route) - return getTracer().trace( + + const tracer = getTracer() + + // Update the root span attribute for the route. + tracer.setRootSpanAttribute('next.route', route) + + return tracer.trace( AppRouteRouteHandlersSpan.runHandler, { spanName: `executing api route (app) ${route}`, @@ -392,228 +662,15 @@ export class AppRouteRouteModule extends RouteModule< 'next.route': route, }, }, - async () => { - // Patch the global fetch. - patchFetch({ - workAsyncStorage: this.workAsyncStorage, - requestAsyncStorage: this.requestAsyncStorage, - prerenderAsyncStorage: this.prerenderAsyncStorage, - }) - - const handlerContext: AppRouteHandlerFnContext = { - params: context.params - ? createServerParamsForRoute( - parsedUrlQueryToParams(context.params), - workStore - ) - : undefined, - } - - let res - if (isStaticGeneration && dynamicIOEnabled) { - /** - * When we are attempting to statically prerender the GET handler of a route.ts module - * and dynamicIO is on we follow a similar pattern to rendering. - * - * We first run the handler letting caches fill. If something synchronously dynamic occurs - * during this prospective render then we can infer it will happen on every render and we - * just bail out of prerendering. - * - * Next we run the handler again and we check if we get a result back in a microtask. - * Next.js expects the return value to be a Response or a Thenable that resolves to a Response. - * Unfortunately Response's do not allow for acessing the response body synchronously or in - * a microtask so we need to allow one more task to unwrap the response body. This is a slightly - * different semantic than what we have when we render and it means that certain tasks can still - * execute before a prerender completes such as a carefully timed setImmediate. - * - * Functionally though IO should still take longer than the time it takes to unwrap the response body - * so our heuristic of excluding any IO should be preserved. - */ - let prospectiveRenderIsDynamic = false - const cacheSignal = new CacheSignal() - let dynamicTracking = - createDynamicTrackingState(undefined) - const prospectiveRoutePrerenderStore: PrerenderStore = { - type: 'prerender', - cacheSignal, - // During prospective render we don't use a controller - // because we need to let all caches fill. - controller: null, - dynamicTracking, - } - let prospectiveResult - try { - prospectiveResult = this.prerenderAsyncStorage.run( - prospectiveRoutePrerenderStore, - handler, - request, - handlerContext - ) - } catch (err) { - if (isPrerenderInterruptedError(err)) { - // the route handler called an API which is always dynamic - // there is no need to try again - prospectiveRenderIsDynamic = true - } - } - if ( - typeof prospectiveResult === 'object' && - prospectiveResult !== null && - typeof (prospectiveResult as any).then === 'function' - ) { - // The handler returned a Thenable. We'll listen for rejections to determine - // if the route is erroring for dynamic reasons. - ;(prospectiveResult as any as Promise).then( - () => {}, - (err) => { - if (isPrerenderInterruptedError(err)) { - // the route handler called an API which is always dynamic - // there is no need to try again - prospectiveRenderIsDynamic = true - } - } - ) - } - await cacheSignal.cacheReady() - - if (prospectiveRenderIsDynamic) { - // the route handler called an API which is always dynamic - // there is no need to try again - const dynamicReason = - getFirstDynamicReason(dynamicTracking) - if (dynamicReason) { - throw new DynamicServerError( - `Route ${workStore.route} couldn't be rendered statically because it used \`${dynamicReason}\`. See more info here: https://nextjs.org/docs/messages/dynamic-server-error` - ) - } else { - console.error( - 'Expected Next.js to keep track of reason for opting out of static rendering but one was not found. This is a bug in Next.js' - ) - throw new DynamicServerError( - `Route ${workStore.route} couldn't be rendered statically because it used a dynamic API. See more info here: https://nextjs.org/docs/messages/dynamic-server-error` - ) - } - } - - // TODO start passing this controller to the route handler. We should expose - // it so the handler to abort inflight requests and other operations if we abort - // the prerender. - const controller = new AbortController() - dynamicTracking = createDynamicTrackingState(undefined) - - const finalRoutePrerenderStore: PrerenderStore = { - type: 'prerender', - cacheSignal: null, - controller, - dynamicTracking, - } - - let responseHandled = false - res = await new Promise((resolve, reject) => { - scheduleImmediate(async () => { - try { - const result = - await (this.prerenderAsyncStorage.run( - finalRoutePrerenderStore, - handler, - request, - handlerContext - ) as Promise) - if (responseHandled) { - // we already rejected in the followup task - return - } else if (!(result instanceof Response)) { - // This is going to error but we let that happen below - resolve(result) - return - } - - responseHandled = true - - let bodyHandled = false - result.arrayBuffer().then((body) => { - if (!bodyHandled) { - bodyHandled = true - - resolve( - new Response(body, { - headers: result.headers, - status: result.status, - statusText: result.statusText, - }) - ) - } - }, reject) - scheduleImmediate(() => { - if (!bodyHandled) { - bodyHandled = true - controller.abort() - reject(createDynamicIOError(workStore.route)) - } - }) - } catch (err) { - reject(err) - } - }) - scheduleImmediate(() => { - if (!responseHandled) { - responseHandled = true - controller.abort() - reject(createDynamicIOError(workStore.route)) - } - }) - }) - } else if (isStaticGeneration) { - res = await prerenderAsyncStorage.run( - { - type: 'prerender-legacy', - }, - handler, - request, - handlerContext - ) - } else { - res = await handler(request, handlerContext) - } - if (!(res instanceof Response)) { - throw new Error( - `No response is returned from route handler '${this.resolvedPagePath}'. Ensure you return a \`Response\` or a \`NextResponse\` in all branches of your handler.` - ) - } - context.renderOpts.fetchMetrics = workStore.fetchMetrics - - context.renderOpts.pendingWaitUntil = Promise.all([ - workStore.incrementalCache?.revalidateTag( - workStore.revalidatedTags || [] - ), - ...Object.values(workStore.pendingRevalidates || {}), - ]) - - addImplicitTags(workStore, requestStore, undefined) - ;(context.renderOpts as any).fetchTags = - workStore.tags?.join(',') - - // It's possible cookies were set in the handler, so we need - // to merge the modified cookies and the returned response - // here. - if (requestStore && requestStore.mutableCookies) { - const headers = new Headers(res.headers) - if ( - appendMutableCookies( - headers, - requestStore.mutableCookies - ) - ) { - return new Response(res.body, { - status: res.status, - statusText: res.statusText, - headers, - }) - } - } - - return res - } + async () => + this.do( + handler, + actionStore, + workStore, + requestStore, + request, + context + ) ) } ) @@ -624,33 +681,13 @@ export class AppRouteRouteModule extends RouteModule< // error response. if (!(response instanceof Response)) { // TODO: validate the correct handling behavior, maybe log something? - return handleInternalServerErrorResponse() + return new Response(null, { status: 500 }) } if (response.headers.has('x-middleware-rewrite')) { - // TODO: move this error into the `NextResponse.rewrite()` function. - // TODO-APP: re-enable support below when we can proxy these type of requests throw new Error( 'NextResponse.rewrite() was used in a app route handler, this is not currently supported. Please remove the invocation to continue.' ) - - // // This is a rewrite created via `NextResponse.rewrite()`. We need to send - // // the response up so it can be handled by the backing server. - - // // If the server is running in minimal mode, we just want to forward the - // // response (including the rewrite headers) upstream so it can perform the - // // redirect for us, otherwise return with the special condition so this - // // server can perform a rewrite. - // if (!minimalMode) { - // return { response, condition: 'rewrite' } - // } - - // // Relativize the url so it's relative to the base url. This is so the - // // outgoing headers upstream can be relative. - // const rewritePath = response.headers.get('x-middleware-rewrite')! - // const initUrl = getRequestMeta(req, 'initURL')! - // const { pathname } = parseUrl(relativizeURL(rewritePath, initUrl)) - // response.headers.set('x-middleware-rewrite', pathname) } if (response.headers.get('x-middleware-next') === '1') { @@ -662,26 +699,6 @@ export class AppRouteRouteModule extends RouteModule< return response } - - public async handle( - request: NextRequest, - context: AppRouteRouteHandlerContext - ): Promise { - try { - // Execute the route to get the response. - const response = await this.execute(request, context) - - // The response was handled, return it. - return response - } catch (err) { - // Try to resolve the error to a response, else throw it again. - const response = resolveHandlerError(err) - if (!response) throw err - - // The response was resolved, return it. - return response - } - } } export default AppRouteRouteModule diff --git a/packages/next/src/server/route-modules/helpers/response-handlers.ts b/packages/next/src/server/route-modules/helpers/response-handlers.ts deleted file mode 100644 index f993b12068b54..0000000000000 --- a/packages/next/src/server/route-modules/helpers/response-handlers.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { appendMutableCookies } from '../../web/spec-extension/adapters/request-cookies' -import type { ResponseCookies } from '../../web/spec-extension/cookies' - -export function handleRedirectResponse( - url: string, - mutableCookies: ResponseCookies, - status: number -): Response { - const headers = new Headers({ location: url }) - - appendMutableCookies(headers, mutableCookies) - - return new Response(null, { status, headers }) -} - -export function handleBadRequestResponse(): Response { - return new Response(null, { status: 400 }) -} - -export function handleNotFoundResponse(): Response { - return new Response(null, { status: 404 }) -} - -export function handleMethodNotAllowedResponse(): Response { - return new Response(null, { status: 405 }) -} - -export function handleInternalServerErrorResponse(): Response { - return new Response(null, { status: 500 }) -} From 68b729ba5da1afcdb4b120c00df1f7d99fbe4b5d Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 3 Oct 2024 21:43:33 -0600 Subject: [PATCH 2/2] fix: add location header to actually redirect --- packages/next/src/server/route-modules/app-route/module.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/src/server/route-modules/app-route/module.ts b/packages/next/src/server/route-modules/app-route/module.ts index 69ca602d81e35..33bf9cfb3aafd 100644 --- a/packages/next/src/server/route-modules/app-route/module.ts +++ b/packages/next/src/server/route-modules/app-route/module.ts @@ -483,7 +483,7 @@ export class AppRouteRouteModule extends RouteModule< // We need to capture any headers that should be sent on // the response. - const headers = new Headers() + const headers = new Headers({ Location: url }) // Let's append any cookies that were added by the // cookie API.