From f0fd6752f7cfb3d5123f9b650b74ac7d25331337 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Fri, 6 Dec 2024 16:49:59 +0100 Subject: [PATCH] Dynamic IO: Improve error handling --- .../next/src/server/app-render/app-render.tsx | 143 ++++++++++++------ .../app-render/create-error-handler.tsx | 63 ++++---- .../app-render/prospective-render-utils.ts | 7 + .../src/server/use-cache/use-cache-wrapper.ts | 12 +- .../not-found/page.tsx | 1 - 5 files changed, 146 insertions(+), 80 deletions(-) rename test/e2e/app-dir/use-cache/app/{(suspense) => (no-suspense)}/not-found/page.tsx (67%) diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index 16243888ee226..ea7f07b62cfe6 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -80,6 +80,7 @@ import { createHTMLErrorHandler, type DigestedError, isUserLandError, + getDigestForWellKnownError, } from './create-error-handler' import { getShortDynamicParamType, @@ -152,6 +153,7 @@ import { getRevalidateReason } from '../instrumentation/utils' import { PAGE_SEGMENT_KEY } from '../../shared/lib/segment' import type { FallbackRouteParams } from '../request/fallback-params' import { DynamicServerError } from '../../client/components/hooks-server-context' +import { ServerPrerenderStreamResult } from './app-render-prerender-utils' import { type ReactServerPrerenderResult, ReactServerResult, @@ -570,7 +572,7 @@ async function generateDynamicFlightRenderResult( ctx.clientReferenceManifest, ctx.workStore.route, requestStore - ) + ).catch(resolveValidation) // avoid unhandled rejections and a forever hanging promise } // For app dir, use the bundled version of Flight server renderer (renderToReadableStream) @@ -1693,7 +1695,7 @@ async function renderToStream( clientReferenceManifest, workStore.route, requestStore - ) + ).catch(resolveValidation) // avoid unhandled rejections and a forever hanging promise reactServerResult = new ReactServerResult(reactServerStream) } else { @@ -2088,7 +2090,13 @@ async function spawnDynamicValidationInDev( firstAttemptRSCPayload, clientReferenceManifest.clientModules, { - onError: (err: unknown) => { + onError: (err) => { + const digest = getDigestForWellKnownError(err) + + if (digest) { + return digest + } + if ( initialServerPrerenderController.signal.aborted || initialServerRenderController.signal.aborted @@ -2146,7 +2154,13 @@ async function spawnDynamicValidationInDev( />, { signal: initialClientController.signal, - onError: (err: unknown, _errorInfo: ErrorInfo) => { + onError: (err) => { + const digest = getDigestForWellKnownError(err) + + if (digest) { + return digest + } + if (initialClientController.signal.aborted) { // These are expected errors that might error the prerender. we ignore them. } else if ( @@ -2239,12 +2253,15 @@ async function spawnDynamicValidationInDev( finalServerPayload, clientReferenceManifest.clientModules, { - onError: (err: unknown) => { - if (finalServerController.signal.aborted) { - if (isPrerenderInterruptedError(err)) { - return err.digest - } + onError: (err) => { + if ( + finalServerController.signal.aborted && + isPrerenderInterruptedError(err) + ) { + return err.digest } + + return getDigestForWellKnownError(err) }, signal: finalServerController.signal, } @@ -2272,15 +2289,14 @@ async function spawnDynamicValidationInDev( />, { signal: finalClientController.signal, - onError: (err: unknown, errorInfo: ErrorInfo) => { + onError: (err, errorInfo) => { if ( isPrerenderInterruptedError(err) || finalClientController.signal.aborted ) { requestStore.usedDynamic = true - const componentStack: string | undefined = (errorInfo as any) - .componentStack + const componentStack = errorInfo.componentStack if (typeof componentStack === 'string') { trackAllowedDynamicAccess( route, @@ -2292,6 +2308,8 @@ async function spawnDynamicValidationInDev( } return } + + return getDigestForWellKnownError(err) }, } ), @@ -2444,7 +2462,10 @@ async function prerenderToStream( onHTMLRenderSSRError ) - let reactServerPrerenderResult: null | ReactServerPrerenderResult = null + let reactServerPrerenderResult: + | null + | ReactServerPrerenderResult + | ServerPrerenderStreamResult = null const setHeader = (name: string, value: string | string[]) => { res.setHeader(name, value) @@ -2525,7 +2546,13 @@ async function prerenderToStream( initialServerPayload, clientReferenceManifest.clientModules, { - onError: (err: unknown) => { + onError: (err) => { + const digest = getDigestForWellKnownError(err) + + if (digest) { + return digest + } + if (initialServerPrerenderController.signal.aborted) { // The render aborted before this error was handled which indicates // the error is caused by unfinished components within the render @@ -2612,7 +2639,13 @@ async function prerenderToStream( />, { signal: initialClientController.signal, - onError: (err: unknown, _errorInfo: ErrorInfo) => { + onError: (err) => { + const digest = getDigestForWellKnownError(err) + + if (digest) { + return digest + } + if (initialClientController.signal.aborted) { // These are expected errors that might error the prerender. we ignore them. } else if ( @@ -2999,7 +3032,13 @@ async function prerenderToStream( firstAttemptRSCPayload, clientReferenceManifest.clientModules, { - onError: (err: unknown) => { + onError: (err) => { + const digest = getDigestForWellKnownError(err) + + if (digest) { + return digest + } + if ( initialServerPrerenderController.signal.aborted || initialServerRenderController.signal.aborted @@ -3057,7 +3096,13 @@ async function prerenderToStream( />, { signal: initialClientController.signal, - onError: (err: unknown, _errorInfo: ErrorInfo) => { + onError: (err) => { + const digest = getDigestForWellKnownError(err) + + if (digest) { + return digest + } + if (initialClientController.signal.aborted) { // These are expected errors that might error the prerender. we ignore them. } else if ( @@ -3155,33 +3200,34 @@ async function prerenderToStream( res.statusCode === 404 ) - const serverPrerenderStreamResult = await prerenderServerWithPhases( - finalServerController.signal, - () => - workUnitAsyncStorage.run( - finalServerPrerenderStore, - ComponentMod.renderToReadableStream, - finalServerPayload, - clientReferenceManifest.clientModules, - { - onError: (err: unknown) => { - if (finalServerController.signal.aborted) { - serverIsDynamic = true - if (isPrerenderInterruptedError(err)) { - return err.digest + const serverPrerenderStreamResult = (reactServerPrerenderResult = + await prerenderServerWithPhases( + finalServerController.signal, + () => + workUnitAsyncStorage.run( + finalServerPrerenderStore, + ComponentMod.renderToReadableStream, + finalServerPayload, + clientReferenceManifest.clientModules, + { + onError: (err: unknown) => { + if (finalServerController.signal.aborted) { + serverIsDynamic = true + if (isPrerenderInterruptedError(err)) { + return err.digest + } + return getDigestForWellKnownError(err) } - return - } - return serverComponentsErrorHandler(err) - }, - signal: finalServerController.signal, - } - ), - () => { - finalServerController.abort() - } - ) + return serverComponentsErrorHandler(err) + }, + signal: finalServerController.signal, + } + ), + () => { + finalServerController.abort() + } + )) let htmlStream const serverPhasedStream = serverPrerenderStreamResult.asPhasedStream() @@ -3764,6 +3810,14 @@ async function prerenderToStream( } const validateRootLayout = renderOpts.dev + + // This is intentionally using the readable datastream from the main + // render rather than the flight data from the error page render + const flightStream = + reactServerPrerenderResult instanceof ServerPrerenderStreamResult + ? reactServerPrerenderResult.asStream() + : reactServerPrerenderResult.consumeAsStream() + return { // Returning the error that was thrown so it can be used to handle // the response in the caller. @@ -3771,10 +3825,7 @@ async function prerenderToStream( ssrErrors: allCapturedErrors, stream: await continueFizzStream(fizzStream, { inlinedDataStream: createInlinedDataReadableStream( - // This is intentionally using the readable datastream from the - // main render rather than the flight data from the error page - // render - reactServerPrerenderResult.consumeAsStream(), + flightStream, ctx.nonce, formState ), diff --git a/packages/next/src/server/app-render/create-error-handler.tsx b/packages/next/src/server/app-render/create-error-handler.tsx index 646ac38bc39ef..eb337f466a05c 100644 --- a/packages/next/src/server/app-render/create-error-handler.tsx +++ b/packages/next/src/server/app-render/create-error-handler.tsx @@ -21,6 +21,27 @@ type SSRErrorHandler = ( export type DigestedError = Error & { digest: string } +/** + * Returns a digest for well-known Next.js errors, otherwise `undefined`. If a + * digest is returned this also means that the error does not need to be + * reported. + */ +export function getDigestForWellKnownError(error: unknown): string | undefined { + // If we're bailing out to CSR, we don't need to log the error. + if (isBailoutToCSRError(error)) return error.digest + + // If this is a navigation error, we don't need to log the error. + if (isNextRouterError(error)) return error.digest + + // If this error occurs, we know that we should be stopping the static + // render. This is only thrown in static generation when PPR is not enabled, + // which causes the whole page to be marked as dynamic. We don't need to + // tell the user about this error, as it's not actionable. + if (isDynamicServerError(error)) return error.digest + + return undefined +} + export function createFlightReactServerErrorHandler( shouldFormatError: boolean, onReactServerRenderError: (err: DigestedError) => void @@ -34,17 +55,11 @@ export function createFlightReactServerErrorHandler( // If the response was closed, we don't need to log the error. if (isAbortError(thrownValue)) return - // If we're bailing out to CSR, we don't need to log the error. - if (isBailoutToCSRError(thrownValue)) return thrownValue.digest + const digest = getDigestForWellKnownError(thrownValue) - // If this is a navigation error, we don't need to log the error. - if (isNextRouterError(thrownValue)) return thrownValue.digest - - // If this error occurs, we know that we should be stopping the static - // render. This is only thrown in static generation when PPR is not enabled, - // which causes the whole page to be marked as dynamic. We don't need to - // tell the user about this error, as it's not actionable. - if (isDynamicServerError(thrownValue)) return thrownValue.digest + if (digest) { + return digest + } const err = getProperError(thrownValue) as DigestedError @@ -92,17 +107,11 @@ export function createHTMLReactServerErrorHandler( // If the response was closed, we don't need to log the error. if (isAbortError(thrownValue)) return - // If we're bailing out to CSR, we don't need to log the error. - if (isBailoutToCSRError(thrownValue)) return thrownValue.digest + const digest = getDigestForWellKnownError(thrownValue) - // If this is a navigation error, we don't need to log the error. - if (isNextRouterError(thrownValue)) return thrownValue.digest - - // If this error occurs, we know that we should be stopping the static - // render. This is only thrown in static generation when PPR is not enabled, - // which causes the whole page to be marked as dynamic. We don't need to - // tell the user about this error, as it's not actionable. - if (isDynamicServerError(thrownValue)) return thrownValue.digest + if (digest) { + return digest + } const err = getProperError(thrownValue) as DigestedError @@ -168,17 +177,11 @@ export function createHTMLErrorHandler( // If the response was closed, we don't need to log the error. if (isAbortError(thrownValue)) return - // If we're bailing out to CSR, we don't need to log the error. - if (isBailoutToCSRError(thrownValue)) return thrownValue.digest + const digest = getDigestForWellKnownError(thrownValue) - // If this is a navigation error, we don't need to log the error. - if (isNextRouterError(thrownValue)) return thrownValue.digest - - // If this error occurs, we know that we should be stopping the static - // render. This is only thrown in static generation when PPR is not enabled, - // which causes the whole page to be marked as dynamic. We don't need to - // tell the user about this error, as it's not actionable. - if (isDynamicServerError(thrownValue)) return thrownValue.digest + if (digest) { + return digest + } const err = getProperError(thrownValue) as DigestedError // If the error already has a digest, respect the original digest, diff --git a/packages/next/src/server/app-render/prospective-render-utils.ts b/packages/next/src/server/app-render/prospective-render-utils.ts index 6f3c567872365..486d274bd9122 100644 --- a/packages/next/src/server/app-render/prospective-render-utils.ts +++ b/packages/next/src/server/app-render/prospective-render-utils.ts @@ -1,7 +1,14 @@ +import { getDigestForWellKnownError } from './create-error-handler' + export function printDebugThrownValueForProspectiveRender( thrownValue: unknown, route: string ) { + // We don't need to print well-known Next.js errors. + if (getDigestForWellKnownError(thrownValue)) { + return + } + let message: undefined | string if ( typeof thrownValue === 'object' && diff --git a/packages/next/src/server/use-cache/use-cache-wrapper.ts b/packages/next/src/server/use-cache/use-cache-wrapper.ts index 9c62ff637e268..e500bbb0c90aa 100644 --- a/packages/next/src/server/use-cache/use-cache-wrapper.ts +++ b/packages/next/src/server/use-cache/use-cache-wrapper.ts @@ -38,7 +38,7 @@ import type { CacheHandler, CacheEntry } from '../lib/cache-handlers/types' import type { CacheSignal } from '../app-render/cache-signal' import { decryptActionBoundArgs } from '../app-render/encryption' import { InvariantError } from '../../shared/lib/invariant-error' -import { createFlightReactServerErrorHandler } from '../app-render/create-error-handler' +import { getDigestForWellKnownError } from '../app-render/create-error-handler' const isEdgeRuntime = process.env.NEXT_RUNTIME === 'edge' @@ -335,13 +335,19 @@ async function generateCacheEntryImpl( // digests are handled correctly. Error formatting and reporting is not // necessary here; the errors are encoded in the stream, and will be // reported in the "Server" environment. - onError: createFlightReactServerErrorHandler(false, (error: unknown) => { + onError: (error) => { + const digest = getDigestForWellKnownError(error) + + if (digest) { + return digest + } + // TODO: For now we're also reporting the error here, because in // production, the "Server" environment will only get the obfuscated // error (created by the Flight Client in the cache wrapper). console.error(error) errors.push(error) - }), + }, } ) diff --git a/test/e2e/app-dir/use-cache/app/(suspense)/not-found/page.tsx b/test/e2e/app-dir/use-cache/app/(no-suspense)/not-found/page.tsx similarity index 67% rename from test/e2e/app-dir/use-cache/app/(suspense)/not-found/page.tsx rename to test/e2e/app-dir/use-cache/app/(no-suspense)/not-found/page.tsx index 6528ed28ee47b..33b72584a3c30 100644 --- a/test/e2e/app-dir/use-cache/app/(suspense)/not-found/page.tsx +++ b/test/e2e/app-dir/use-cache/app/(no-suspense)/not-found/page.tsx @@ -1,4 +1,3 @@ -// TODO: This should not need the suspense boundary in the root layout. 'use cache' import { notFound } from 'next/navigation'