diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index bcbe0bc045ab7..dd24e9f5afecb 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -60,7 +60,7 @@ import { getTracer } from '../lib/trace/tracer' import { FlightRenderResult } from './flight-render-result' import { createErrorHandler, - ErrorHandlerSource, + type DigestedError, type ErrorHandler, } from './create-error-handler' import { @@ -727,7 +727,7 @@ async function renderToHTMLOrFlightImpl( serverModuleMap, }) - const digestErrorsMap: Map = new Map() + const digestErrorsMap: Map = new Map() const allCapturedErrors: Error[] = [] const isNextExport = !!renderOpts.nextExport const { staticGenerationStore, requestStore } = baseCtx @@ -762,32 +762,61 @@ async function renderToHTMLOrFlightImpl( routeType: 'render', } as const - const onReactStreamRenderError = onInstrumentationRequestError - ? (err: Error) => onInstrumentationRequestError(err, req, errorContext) - : undefined + // Including RSC rendering and flight data rendering + function getRSCError(err: DigestedError) { + const digest = err.digest + if (!digestErrorsMap.has(digest)) { + digestErrorsMap.set(digest, err) + } + return err + } + + function getSSRError(err: DigestedError) { + // For SSR errors, if we have the existing digest in errors map, + // we should use the existing error object to avoid duplicate error logs. + if (digestErrorsMap.has(err.digest)) { + return digestErrorsMap.get(err.digest)! + } + return err + } + + function onFlightDataRenderError(err: DigestedError) { + return onInstrumentationRequestError?.(err, req, { + ...errorContext, + renderSource: 'react-server-components-payload', + }) + } + + function onServerRenderError(err: DigestedError) { + const renderSource = digestErrorsMap.has(err.digest) + ? 'react-server-components' + : 'server-rendering' + return onInstrumentationRequestError?.(err, req, { + ...errorContext, + renderSource, + }) + } const serverComponentsErrorHandler = createErrorHandler({ - source: ErrorHandlerSource.serverComponents, dev, isNextExport, - onReactStreamRenderError, - digestErrorsMap, + // RSC rendering error will report as SSR error + onReactStreamRenderError: undefined, + getErrorByRenderSource: getRSCError, silenceLogger: silenceStaticGenerationErrors, }) const flightDataRendererErrorHandler = createErrorHandler({ - source: ErrorHandlerSource.flightData, dev, isNextExport, - onReactStreamRenderError, - digestErrorsMap, + onReactStreamRenderError: onFlightDataRenderError, + getErrorByRenderSource: getRSCError, silenceLogger: silenceStaticGenerationErrors, }) const htmlRendererErrorHandler = createErrorHandler({ - source: ErrorHandlerSource.html, dev, isNextExport, - onReactStreamRenderError, - digestErrorsMap, + onReactStreamRenderError: onServerRenderError, + getErrorByRenderSource: getSSRError, allCapturedErrors, silenceLogger: silenceStaticGenerationErrors, }) 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 ecde9da6390e6..9f5d1deadacb9 100644 --- a/packages/next/src/server/app-render/create-error-handler.tsx +++ b/packages/next/src/server/app-render/create-error-handler.tsx @@ -15,11 +15,7 @@ export type ErrorHandler = ( errorInfo: unknown ) => string | undefined -export const ErrorHandlerSource = { - serverComponents: 'serverComponents', - flightData: 'flightData', - html: 'html', -} as const +export type DigestedError = Error & { digest: string } /** * Create error handler for renderers. @@ -27,22 +23,17 @@ export const ErrorHandlerSource = { * isn't spammed with unactionable errors */ export function createErrorHandler({ - /** - * Used for debugging - */ - source, dev, isNextExport, onReactStreamRenderError, - digestErrorsMap, + getErrorByRenderSource, allCapturedErrors, silenceLogger, }: { - source: (typeof ErrorHandlerSource)[keyof typeof ErrorHandlerSource] dev?: boolean isNextExport?: boolean onReactStreamRenderError?: (err: any) => void - digestErrorsMap: Map + getErrorByRenderSource: (err: DigestedError) => Error allCapturedErrors?: Error[] silenceLogger?: boolean }): ErrorHandler { @@ -55,7 +46,6 @@ export function createErrorHandler({ err.message + (errorInfo?.stack || err.stack || '') ).toString() } - const digest = err.digest if (allCapturedErrors) allCapturedErrors.push(err) @@ -68,13 +58,7 @@ export function createErrorHandler({ // If this is a navigation error, we don't need to log the error. if (isNextRouterError(err)) return err.digest - if (!digestErrorsMap.has(digest)) { - digestErrorsMap.set(digest, err) - } else if (source === ErrorHandlerSource.html) { - // For SSR errors, if we have the existing digest in errors map, - // we should use the existing error object to avoid duplicate error logs. - err = digestErrorsMap.get(digest) - } + err = getErrorByRenderSource(err) // 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, @@ -86,8 +70,7 @@ export function createErrorHandler({ if (dev) { formatServerError(err) } - // Used for debugging error source - // console.error(source, err) + // Don't log the suppressed error during export if ( !( @@ -107,17 +90,7 @@ export function createErrorHandler({ }) } - if ( - (!silenceLogger && - // Only log the error from SSR rendering errors and flight data render errors, - // as RSC renderer error will still be pipped into SSR renderer as well. - source === 'html') || - source === 'flightData' - ) { - // The error logger is currently not provided in the edge runtime. - // Use the exposed `__next_log_error__` instead. - // This will trace error traces to the original source code. - + if (!silenceLogger) { onReactStreamRenderError?.(err) } } diff --git a/packages/next/src/server/instrumentation/types.ts b/packages/next/src/server/instrumentation/types.ts index 83587942963e8..0872edbd4cb3a 100644 --- a/packages/next/src/server/instrumentation/types.ts +++ b/packages/next/src/server/instrumentation/types.ts @@ -2,6 +2,10 @@ type RequestErrorContext = { routerKind: 'Pages Router' | 'App Router' routePath: string // the route file path, e.g. /app/blog/[dynamic] routeType: 'render' | 'route' | 'action' | 'middleware' + renderSource?: + | 'react-server-components' + | 'react-server-components-payload' + | 'server-rendering' // TODO: other future instrumentation context } diff --git a/test/e2e/on-request-error/basic/basic.test.ts b/test/e2e/on-request-error/basic/basic.test.ts index eab6508cc6ed9..9178ca72b0492 100644 --- a/test/e2e/on-request-error/basic/basic.test.ts +++ b/test/e2e/on-request-error/basic/basic.test.ts @@ -24,11 +24,17 @@ describe('on-request-error - basic', () => { return JSON.parse(content) } - async function validateErrorRecord( - name: string, - url: string, - isMiddleware: boolean = false - ) { + async function validateErrorRecord({ + name, + url, + renderSource, + isMiddleware = false, + }: { + name: string + url: string + renderSource: string | undefined + isMiddleware?: boolean + }) { await retry(async () => { const recordLogs = next.cliOutput .split('\n') @@ -45,14 +51,26 @@ describe('on-request-error - basic', () => { expect(payload.message).toBe(name) expect(count).toBe(1) - validateRequestByName(payload.request, url, isMiddleware) + validateRequestByName({ + payload: payload, + url, + isMiddleware, + renderSource, + }) } - function validateRequestByName( - request: any, - url: string, - isMiddleware: boolean = false - ) { + function validateRequestByName({ + payload, + url, + renderSource, + isMiddleware = false, + }: { + payload: any + url: string + renderSource: string | undefined + isMiddleware: boolean + }) { + const { request } = payload if (isMiddleware) { // For middleware, the URL is absolute url with host expect(request.url).toMatch(/^http:\/\//) @@ -60,6 +78,9 @@ describe('on-request-error - basic', () => { } else { expect(request.url).toBe(url) } + + expect(payload.context.renderSource).toBe(renderSource) + expect(request.method).toBe('GET') expect(request.headers['accept']).toBe('*/*') } @@ -71,61 +92,106 @@ describe('on-request-error - basic', () => { describe('app router', () => { it('should catch server component page error in node runtime', async () => { await next.fetch('/server-page') - await validateErrorRecord('server-page-node-error', '/server-page') + await validateErrorRecord({ + name: 'server-page-node-error', + url: '/server-page', + renderSource: 'react-server-components', + }) }) it('should catch server component page error in edge runtime', async () => { await next.fetch('/server-page/edge') - await validateErrorRecord('server-page-edge-error', '/server-page/edge') + await validateErrorRecord({ + name: 'server-page-edge-error', + url: '/server-page/edge', + renderSource: 'react-server-components', + }) }) it('should catch client component page error in node runtime', async () => { await next.fetch('/client-page') - await validateErrorRecord('client-page-node-error', '/client-page') + await validateErrorRecord({ + name: 'client-page-node-error', + url: '/client-page', + renderSource: 'server-rendering', + }) }) it('should catch client component page error in edge runtime', async () => { await next.fetch('/client-page/edge') - await validateErrorRecord('client-page-edge-error', '/client-page/edge') + await validateErrorRecord({ + name: 'client-page-edge-error', + url: '/client-page/edge', + renderSource: 'server-rendering', + }) }) it('should catch app routes error in node runtime', async () => { await next.fetch('/app-route') - await validateErrorRecord('route-node-error', '/app-route') + await validateErrorRecord({ + name: 'route-node-error', + url: '/app-route', + renderSource: undefined, + }) }) it('should catch app routes error in edge runtime', async () => { await next.fetch('/app-route/edge') - await validateErrorRecord('route-edge-error', '/app-route/edge') + await validateErrorRecord({ + name: 'route-edge-error', + url: '/app-route/edge', + renderSource: undefined, + }) }) }) describe('pages router', () => { it('should catch pages router page error in node runtime', async () => { await next.fetch('/page') - await validateErrorRecord('pages-page-node-error', '/page') + await validateErrorRecord({ + name: 'pages-page-node-error', + url: '/page', + renderSource: undefined, + }) }) it('should catch pages router page error in edge runtime', async () => { await next.fetch('/page/edge') - await validateErrorRecord('pages-page-edge-error', '/page/edge') + await validateErrorRecord({ + name: 'pages-page-edge-error', + url: '/page/edge', + renderSource: undefined, + }) }) it('should catch pages router api error in node runtime', async () => { await next.fetch('/api/pages-route') - await validateErrorRecord('api-node-error', '/api/pages-route') + await validateErrorRecord({ + name: 'api-node-error', + url: '/api/pages-route', + renderSource: undefined, + }) }) it('should catch pages router api error in edge runtime', async () => { await next.fetch('/api/pages-route/edge') - await validateErrorRecord('api-edge-error', '/api/pages-route/edge') + await validateErrorRecord({ + name: 'api-edge-error', + url: '/api/pages-route/edge', + renderSource: undefined, + }) }) }) describe('middleware', () => { it('should catch middleware error', async () => { await next.fetch('/middleware-error') - await validateErrorRecord('middleware-error', '/middleware-error', true) + await validateErrorRecord({ + name: 'middleware-error', + url: '/middleware-error', + isMiddleware: true, + renderSource: undefined, + }) }) }) })