Add render source to onRequestError context (#67703)

### What

Provide `renderSource` for app router pages rendering errors in
instrumentation.js `onRequestError`.

Discussed with @gnoff that we refactored the error handlers a bit to
decouple from the Error render source since the handlers act differently
in different render phase.

### Why

This provides an easy way to determine wether the renderign error is
actually from SSR or RSC rendering. Since the RSC error is embedded in
the flight data and the final errors is only reported through React SSR
rendering.

Previously you can use the `digest` property sent to browser to
associate the actual error logged with your o11y provider, but it still
takes some effort, this would be the easy way to capture the original
source

Closes NDX-24

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<string, Error> = new Map()
+  const digestErrorsMap: Map<string, DigestedError> = 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,
   })

packages/next/src/server/app-render/create-error-handler.tsx

@@ -15,34 +15,25 @@ 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.
  * Tolerate dynamic server errors during prerendering so console
  * 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<string, Error>
+  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)
       }
     }

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
 }
 

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,21 +51,36 @@ 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:\/\//)
       expect(request.url).toMatch(url)
     } 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,
+      })
     })
   })
 })