fix: ensure onRequestError is invoked when otel enabled (#83343)

yiminghe · huozhi · huozhi · commit 39c3cd625d62 · 2025-09-12T15:48:15.000+02:00
Co-authored-by: Jiachi Liu &lt;inbox@huozhi.im&gt;
diff --git a/packages/next/src/build/templates/app-page.ts b/packages/next/src/build/templates/app-page.ts
@@ -1244,8 +1244,7 @@ export async function handler(
       )
     }
   } catch (err) {
-    // if we aren't wrapped by base-server handle here
-    if (!activeSpan && !(err instanceof NoFallbackError)) {
+    if (!(err instanceof NoFallbackError)) {
       await routeModule.onRequestError(
         req,
         err,
diff --git a/packages/next/src/build/templates/app-route.ts b/packages/next/src/build/templates/app-route.ts
@@ -448,8 +448,7 @@ export async function handler(
       )
     }
   } catch (err) {
-    // if we aren't wrapped by base-server handle here
-    if (!activeSpan && !(err instanceof NoFallbackError)) {
+    if (!(err instanceof NoFallbackError)) {
       await routeModule.onRequestError(req, err, {
         routerKind: 'App Router',
         routePath: normalizedSrcPage,
diff --git a/test/e2e/on-request-error/otel/app/app-route/route.js b/test/e2e/on-request-error/otel/app/app-route/route.js
@@ -0,0 +1,6 @@
+import { connection } from 'next/server'
+
+export async function GET() {
+  await connection()
+  throw new Error('route-node-error')
+}
diff --git a/test/e2e/on-request-error/otel/app/layout.js b/test/e2e/on-request-error/otel/app/layout.js
@@ -0,0 +1,11 @@
+import { Suspense } from 'react'
+
+export default function Layout({ children }) {
+  return (
+    <html>
+      <body>
+        <Suspense>{children}</Suspense>
+      </body>
+    </html>
+  )
+}
diff --git a/test/e2e/on-request-error/otel/app/server-page/page.js b/test/e2e/on-request-error/otel/app/server-page/page.js
@@ -0,0 +1,6 @@
+import { connection } from 'next/server'
+
+export default async function Page() {
+  await connection()
+  throw new Error('server-page-node-error')
+}
diff --git a/test/e2e/on-request-error/otel/app/write-log/route.js b/test/e2e/on-request-error/otel/app/write-log/route.js
@@ -0,0 +1,40 @@
+import fs from 'fs'
+import fsp from 'fs/promises'
+import path from 'path'
+
+const dir = path.join(path.dirname(new URL(import.meta.url).pathname), '../..')
+const logPath = path.join(dir, 'output-log.json')
+
+export async function POST(req) {
+  let payloadString = ''
+  const decoder = new TextDecoder()
+  const reader = req.clone().body.getReader()
+  while (true) {
+    const { done, value } = await reader.read()
+    if (done) {
+      break
+    }
+
+    payloadString += decoder.decode(value)
+  }
+
+  const payload = JSON.parse(payloadString)
+
+  const json = fs.existsSync(logPath)
+    ? JSON.parse(await fsp.readFile(logPath, 'utf8'))
+    : {}
+
+  if (!json[payload.message]) {
+    json[payload.message] = {
+      payload,
+      count: 1,
+    }
+  } else {
+    json[payload.message].count++
+  }
+
+  await fsp.writeFile(logPath, JSON.stringify(json, null, 2), 'utf8')
+
+  console.log(`[instrumentation] write-log:${payload.message}`)
+  return new Response(null, { status: 204 })
+}
diff --git a/test/e2e/on-request-error/otel/instrumentation.js b/test/e2e/on-request-error/otel/instrumentation.js
@@ -0,0 +1,21 @@
+import { registerOTel } from '@vercel/otel'
+
+export const onRequestError = async (err, request, context) => {
+  await fetch(`http://localhost:${process.env.PORT}/write-log`, {
+    method: 'POST',
+    body: JSON.stringify({
+      message: err.message,
+      request,
+      context,
+    }),
+    headers: {
+      'Content-Type': 'application/json',
+    },
+  })
+}
+
+export function register() {
+  registerOTel({
+    serviceName: 'test',
+  })
+}
diff --git a/test/e2e/on-request-error/otel/otel.test.ts b/test/e2e/on-request-error/otel/otel.test.ts
@@ -0,0 +1,83 @@
+import { nextTestSetup } from 'e2e-utils'
+import { retry } from 'next-test-utils'
+import { getOutputLogJson } from '../_testing/utils'
+
+describe('on-request-error - otel', () => {
+  const { next, skipped } = nextTestSetup({
+    files: __dirname,
+    skipDeployment: true,
+    packageJson: {
+      dependencies: {
+        '@vercel/otel': '^1.13.0',
+      },
+    },
+  })
+
+  if (skipped) {
+    return
+  }
+
+  const outputLogPath = 'output-log.json'
+
+  async function validateErrorRecord({
+    errorMessage,
+    url,
+    renderSource,
+  }: {
+    errorMessage: string
+    url: string
+    renderSource: string | undefined
+  }) {
+    // Assert the instrumentation is called
+    await retry(async () => {
+      const recordLogLines = next.cliOutput
+        .split('\n')
+        .filter((log) => log.includes('[instrumentation] write-log'))
+      expect(recordLogLines).toEqual(
+        expect.arrayContaining([expect.stringContaining(errorMessage)])
+      )
+      // TODO: remove custom duration in case we increase the default.
+    }, 5000)
+
+    const json = await getOutputLogJson(next, outputLogPath)
+    const record = json[errorMessage]
+
+    const { payload } = record
+    const { request } = payload
+
+    expect(request.path).toBe(url)
+    expect(record).toMatchObject({
+      count: 1,
+      payload: {
+        message: errorMessage,
+        request: { method: 'GET', headers: { accept: '*/*' } },
+        ...(renderSource ? { context: { renderSource } } : undefined),
+      },
+    })
+  }
+
+  beforeAll(async () => {
+    await next.patchFile(outputLogPath, '{}')
+  })
+
+  describe('app router', () => {
+    it('should catch server component page error in node runtime', async () => {
+      await next.fetch('/server-page')
+      await validateErrorRecord({
+        errorMessage: 'server-page-node-error',
+        url: '/server-page',
+        renderSource: 'react-server-components',
+      })
+    })
+
+    it('should catch app routes error in node runtime', async () => {
+      await next.fetch('/app-route')
+
+      await validateErrorRecord({
+        errorMessage: 'route-node-error',
+        url: '/app-route',
+        renderSource: undefined,
+      })
+    })
+  })
+})

Original file line number	Diff line number	Diff line change
`@@ -1244,8 +1244,7 @@ export async function handler(`
`1244`	`1244`	`)`
`1245`	`1245`	`}`
`1246`	`1246`	`} catch (err) {`
`1247`		`- // if we aren't wrapped by base-server handle here`
`1248`		`- if (!activeSpan && !(err instanceof NoFallbackError)) {`
	`1247`	`+ if (!(err instanceof NoFallbackError)) {`
`1249`	`1248`	`await routeModule.onRequestError(`
`1250`	`1249`	`req,`
`1251`	`1250`	`err,`
Original file line number	Diff line number	Diff line change
`@@ -448,8 +448,7 @@ export async function handler(`
`448`	`448`	`)`
`449`	`449`	`}`
`450`	`450`	`} catch (err) {`
`451`		`- // if we aren't wrapped by base-server handle here`
`452`		`- if (!activeSpan && !(err instanceof NoFallbackError)) {`
	`451`	`+ if (!(err instanceof NoFallbackError)) {`
`453`	`452`	`await routeModule.onRequestError(req, err, {`
`454`	`453`	`routerKind: 'App Router',`
`455`	`454`	`routePath: normalizedSrcPage,`