From a5a618693917857e46f337ee9a1aba2cc339bfe6 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 17 Jul 2024 12:43:17 +0200 Subject: [PATCH 1/5] test: add dynamic routes and suspense test case for onRequestError --- .../app/app-page/dynamic/[id]/page.js | 6 ++ .../app/app-page/suspense/page.js | 18 ++++ .../app/app-route/dynamic/[id]/route.js | 6 ++ .../dynamic-routes/app/layout.js | 12 +++ .../dynamic-routes/app/write-log/route.js | 40 ++++++++ .../dynamic-routes/dynamic-routes.test.ts | 94 +++++++++++++++++++ .../dynamic-routes/instrumentation.js | 13 +++ .../dynamic-routes/next.config.js | 5 + 8 files changed, 194 insertions(+) create mode 100644 test/e2e/on-request-error/dynamic-routes/app/app-page/dynamic/[id]/page.js create mode 100644 test/e2e/on-request-error/dynamic-routes/app/app-page/suspense/page.js create mode 100644 test/e2e/on-request-error/dynamic-routes/app/app-route/dynamic/[id]/route.js create mode 100644 test/e2e/on-request-error/dynamic-routes/app/layout.js create mode 100644 test/e2e/on-request-error/dynamic-routes/app/write-log/route.js create mode 100644 test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts create mode 100644 test/e2e/on-request-error/dynamic-routes/instrumentation.js create mode 100644 test/e2e/on-request-error/dynamic-routes/next.config.js diff --git a/test/e2e/on-request-error/dynamic-routes/app/app-page/dynamic/[id]/page.js b/test/e2e/on-request-error/dynamic-routes/app/app-page/dynamic/[id]/page.js new file mode 100644 index 0000000000000..dc64183d30f62 --- /dev/null +++ b/test/e2e/on-request-error/dynamic-routes/app/app-page/dynamic/[id]/page.js @@ -0,0 +1,6 @@ +export default function Page({ searchParams }) { + const search = new URLSearchParams(searchParams).toString() + throw new Error( + 'server-dynamic-page-node-error' + (search ? `?${search}` : '') + ) +} diff --git a/test/e2e/on-request-error/dynamic-routes/app/app-page/suspense/page.js b/test/e2e/on-request-error/dynamic-routes/app/app-page/suspense/page.js new file mode 100644 index 0000000000000..7b1a46fe1650c --- /dev/null +++ b/test/e2e/on-request-error/dynamic-routes/app/app-page/suspense/page.js @@ -0,0 +1,18 @@ +import { Suspense } from 'react' + +export default function Page() { + return ( + + + + ) +} + +function Inner() { + if (typeof window === 'undefined') { + throw new Error('server-suspense-page-node-error') + } + return 'inner' +} + +export const dynamic = 'force-dynamic' diff --git a/test/e2e/on-request-error/dynamic-routes/app/app-route/dynamic/[id]/route.js b/test/e2e/on-request-error/dynamic-routes/app/app-route/dynamic/[id]/route.js new file mode 100644 index 0000000000000..e2b2e39c45b41 --- /dev/null +++ b/test/e2e/on-request-error/dynamic-routes/app/app-route/dynamic/[id]/route.js @@ -0,0 +1,6 @@ +export function GET(request) { + const search = request.url.split('?')[1] || '' + throw new Error( + 'server-dynamic-route-node-error' + (search ? `?${search}` : '') + ) +} diff --git a/test/e2e/on-request-error/dynamic-routes/app/layout.js b/test/e2e/on-request-error/dynamic-routes/app/layout.js new file mode 100644 index 0000000000000..8525f5f8c0b2a --- /dev/null +++ b/test/e2e/on-request-error/dynamic-routes/app/layout.js @@ -0,0 +1,12 @@ +export const metadata = { + title: 'Next.js', + description: 'Generated by Next.js', +} + +export default function RootLayout({ children }) { + return ( + + {children} + + ) +} diff --git a/test/e2e/on-request-error/dynamic-routes/app/write-log/route.js b/test/e2e/on-request-error/dynamic-routes/app/write-log/route.js new file mode 100644 index 0000000000000..62b11b6617cd8 --- /dev/null +++ b/test/e2e/on-request-error/dynamic-routes/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/dynamic-routes/dynamic-routes.test.ts b/test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts new file mode 100644 index 0000000000000..539eea5644ac1 --- /dev/null +++ b/test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts @@ -0,0 +1,94 @@ +import { nextTestSetup } from 'e2e-utils' +import { retry } from 'next-test-utils' + +describe('on-request-error - dynamic-routes', () => { + const { next, skipped } = nextTestSetup({ + files: __dirname, + skipDeployment: true, + env: { + __NEXT_EXPERIMENTAL_INSTRUMENTATION: '1', + }, + }) + + if (skipped) { + return + } + + const outputLogPath = 'output-log.json' + + async function getOutputLogJson() { + if (!(await next.hasFile(outputLogPath))) { + return {} + } + const content = await next.readFile(outputLogPath) + return JSON.parse(content) + } + + async function getErrorRecord({ errorMessage }: { errorMessage: string }) { + // Assert the instrumentation is called + await retry(async () => { + const recordLogs = next.cliOutput + .split('\n') + .filter((log) => log.includes('[instrumentation] write-log')) + const expectedLog = recordLogs.find((log) => log.includes(errorMessage)) + expect(expectedLog).toBeDefined() + }, 5000) + + const json = await getOutputLogJson() + const record = json[errorMessage] + + return record + } + + beforeAll(async () => { + await next.patchFile(outputLogPath, '{}') + }) + + describe('app router', () => { + it('should catch app router dynamic page error with search params', async () => { + await next.fetch('/app-page/dynamic/123?apple=dope') + const record = await getErrorRecord({ + errorMessage: 'server-dynamic-page-node-error?apple=dope', + }) + expect(record).toMatchObject({ + payload: { + message: 'server-dynamic-page-node-error?apple=dope', + context: { + routePath: '/app-page/dynamic/[id]', + }, + }, + }) + }) + + it('should catch app router dynamic routes error with search params', async () => { + await next.fetch('/app-route/dynamic/123?apple=dope') + const record = await getErrorRecord({ + errorMessage: 'server-dynamic-route-node-error?apple=dope', + }) + expect(record).toMatchObject({ + payload: { + message: 'server-dynamic-route-node-error?apple=dope', + context: { + routePath: '/app-route/dynamic/[id]', + }, + }, + }) + }) + + it('should catch client component page error in node runtime', async () => { + await next.fetch('/app-page/suspense') + const record = await getErrorRecord({ + errorMessage: 'server-suspense-page-node-error', + }) + + expect(record).toMatchObject({ + payload: { + message: 'server-suspense-page-node-error', + context: { + routePath: '/app-page/suspense', + }, + }, + }) + }) + }) +}) diff --git a/test/e2e/on-request-error/dynamic-routes/instrumentation.js b/test/e2e/on-request-error/dynamic-routes/instrumentation.js new file mode 100644 index 0000000000000..b9944b351236c --- /dev/null +++ b/test/e2e/on-request-error/dynamic-routes/instrumentation.js @@ -0,0 +1,13 @@ +export function onRequestError(err, request, context) { + fetch(`http://localhost:${process.env.PORT}/write-log`, { + method: 'POST', + body: JSON.stringify({ + message: err.message, + request, + context, + }), + headers: { + 'Content-Type': 'application/json', + }, + }) +} diff --git a/test/e2e/on-request-error/dynamic-routes/next.config.js b/test/e2e/on-request-error/dynamic-routes/next.config.js new file mode 100644 index 0000000000000..c4cf84a76553b --- /dev/null +++ b/test/e2e/on-request-error/dynamic-routes/next.config.js @@ -0,0 +1,5 @@ +module.exports = { + experimental: { + instrumentationHook: true, + }, +} From 51cd846f86b28ae7a1cb12e2b2a07286149f6512 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 17 Jul 2024 14:33:56 +0200 Subject: [PATCH 2/5] test: add tests for dynamic routes and suspense for onRequestError --- test/e2e/on-request-error/_testing/utils.js | 7 ++ test/e2e/on-request-error/basic/basic.test.ts | 18 ++--- .../app/app-page/dynamic/[id]/page.js | 7 +- .../app/app-route/dynamic/[id]/route.js | 7 +- .../dynamic-routes/dynamic-routes.test.ts | 75 +++++++++++++++---- .../dynamic-routes/pages/api/dynamic/[id].js | 3 + .../pages/pages-page/dynamic/[id].js | 9 +++ .../server-action-error.test.ts | 11 +-- 8 files changed, 90 insertions(+), 47 deletions(-) create mode 100644 test/e2e/on-request-error/_testing/utils.js create mode 100644 test/e2e/on-request-error/dynamic-routes/pages/api/dynamic/[id].js create mode 100644 test/e2e/on-request-error/dynamic-routes/pages/pages-page/dynamic/[id].js diff --git a/test/e2e/on-request-error/_testing/utils.js b/test/e2e/on-request-error/_testing/utils.js new file mode 100644 index 0000000000000..00c17414ee643 --- /dev/null +++ b/test/e2e/on-request-error/_testing/utils.js @@ -0,0 +1,7 @@ +export async function getOutputLogJson(next, outputLogPath) { + if (!(await next.hasFile(outputLogPath))) { + return {} + } + const content = await next.readFile(outputLogPath) + return JSON.parse(content) +} diff --git a/test/e2e/on-request-error/basic/basic.test.ts b/test/e2e/on-request-error/basic/basic.test.ts index bcb350c7f37c2..3e018fbf84037 100644 --- a/test/e2e/on-request-error/basic/basic.test.ts +++ b/test/e2e/on-request-error/basic/basic.test.ts @@ -1,5 +1,6 @@ import { nextTestSetup } from 'e2e-utils' import { retry } from 'next-test-utils' +import { getOutputLogJson } from '../_testing/utils' describe('on-request-error - basic', () => { const { next, skipped } = nextTestSetup({ @@ -16,14 +17,6 @@ describe('on-request-error - basic', () => { const outputLogPath = 'output-log.json' - async function getOutputLogJson() { - if (!(await next.hasFile(outputLogPath))) { - return {} - } - const content = await next.readFile(outputLogPath) - return JSON.parse(content) - } - async function validateErrorRecord({ errorMessage, url, @@ -37,14 +30,15 @@ describe('on-request-error - basic', () => { }) { // Assert the instrumentation is called await retry(async () => { - const recordLogs = next.cliOutput + const recordLogLines = next.cliOutput .split('\n') .filter((log) => log.includes('[instrumentation] write-log')) - const expectedLog = recordLogs.find((log) => log.includes(errorMessage)) - expect(expectedLog).toBeDefined() + expect(recordLogLines).toEqual( + expect.arrayContaining([expect.stringContaining(errorMessage)]) + ) }, 5000) - const json = await getOutputLogJson() + const json = await getOutputLogJson(next, outputLogPath) const record = json[errorMessage] const { payload } = record diff --git a/test/e2e/on-request-error/dynamic-routes/app/app-page/dynamic/[id]/page.js b/test/e2e/on-request-error/dynamic-routes/app/app-page/dynamic/[id]/page.js index dc64183d30f62..6c5bd83380e69 100644 --- a/test/e2e/on-request-error/dynamic-routes/app/app-page/dynamic/[id]/page.js +++ b/test/e2e/on-request-error/dynamic-routes/app/app-page/dynamic/[id]/page.js @@ -1,6 +1,3 @@ -export default function Page({ searchParams }) { - const search = new URLSearchParams(searchParams).toString() - throw new Error( - 'server-dynamic-page-node-error' + (search ? `?${search}` : '') - ) +export default function Page() { + throw new Error('server-dynamic-page-node-error') } diff --git a/test/e2e/on-request-error/dynamic-routes/app/app-route/dynamic/[id]/route.js b/test/e2e/on-request-error/dynamic-routes/app/app-route/dynamic/[id]/route.js index e2b2e39c45b41..cf8c4cbcdc740 100644 --- a/test/e2e/on-request-error/dynamic-routes/app/app-route/dynamic/[id]/route.js +++ b/test/e2e/on-request-error/dynamic-routes/app/app-route/dynamic/[id]/route.js @@ -1,6 +1,3 @@ -export function GET(request) { - const search = request.url.split('?')[1] || '' - throw new Error( - 'server-dynamic-route-node-error' + (search ? `?${search}` : '') - ) +export function GET() { + throw new Error('server-dynamic-route-node-error') } diff --git a/test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts b/test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts index 539eea5644ac1..56f664a46f4e7 100644 --- a/test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts +++ b/test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts @@ -1,5 +1,6 @@ import { nextTestSetup } from 'e2e-utils' import { retry } from 'next-test-utils' +import { getOutputLogJson } from '../_testing/utils' describe('on-request-error - dynamic-routes', () => { const { next, skipped } = nextTestSetup({ @@ -16,25 +17,18 @@ describe('on-request-error - dynamic-routes', () => { const outputLogPath = 'output-log.json' - async function getOutputLogJson() { - if (!(await next.hasFile(outputLogPath))) { - return {} - } - const content = await next.readFile(outputLogPath) - return JSON.parse(content) - } - async function getErrorRecord({ errorMessage }: { errorMessage: string }) { // Assert the instrumentation is called await retry(async () => { - const recordLogs = next.cliOutput + const recordLogLines = next.cliOutput .split('\n') .filter((log) => log.includes('[instrumentation] write-log')) - const expectedLog = recordLogs.find((log) => log.includes(errorMessage)) - expect(expectedLog).toBeDefined() + expect(recordLogLines).toEqual( + expect.arrayContaining([expect.stringContaining(errorMessage)]) + ) }, 5000) - const json = await getOutputLogJson() + const json = await getOutputLogJson(next, outputLogPath) const record = json[errorMessage] return record @@ -48,11 +42,14 @@ describe('on-request-error - dynamic-routes', () => { it('should catch app router dynamic page error with search params', async () => { await next.fetch('/app-page/dynamic/123?apple=dope') const record = await getErrorRecord({ - errorMessage: 'server-dynamic-page-node-error?apple=dope', + errorMessage: 'server-dynamic-page-node-error', }) expect(record).toMatchObject({ payload: { - message: 'server-dynamic-page-node-error?apple=dope', + message: 'server-dynamic-page-node-error', + request: { + url: '/app-page/dynamic/123?apple=dope', + }, context: { routePath: '/app-page/dynamic/[id]', }, @@ -63,11 +60,14 @@ describe('on-request-error - dynamic-routes', () => { it('should catch app router dynamic routes error with search params', async () => { await next.fetch('/app-route/dynamic/123?apple=dope') const record = await getErrorRecord({ - errorMessage: 'server-dynamic-route-node-error?apple=dope', + errorMessage: 'server-dynamic-route-node-error', }) expect(record).toMatchObject({ payload: { - message: 'server-dynamic-route-node-error?apple=dope', + message: 'server-dynamic-route-node-error', + request: { + url: '/app-route/dynamic/123?apple=dope', + }, context: { routePath: '/app-route/dynamic/[id]', }, @@ -84,6 +84,9 @@ describe('on-request-error - dynamic-routes', () => { expect(record).toMatchObject({ payload: { message: 'server-suspense-page-node-error', + request: { + url: '/app-page/suspense', + }, context: { routePath: '/app-page/suspense', }, @@ -91,4 +94,44 @@ describe('on-request-error - dynamic-routes', () => { }) }) }) + + describe('pages router', () => { + it('should catch pages router dynamic page error with search params', async () => { + await next.fetch('/pages-page/dynamic/123?apple=dope') + const record = await getErrorRecord({ + errorMessage: 'pages-page-node-error', + }) + + expect(record).toMatchObject({ + payload: { + message: 'pages-page-node-error', + request: { + url: '/pages-page/dynamic/123?apple=dope', + }, + context: { + routePath: '/pages-page/dynamic/[id]', + }, + }, + }) + }) + + it('should catch pages router dynamic API route error with search params', async () => { + await next.fetch('/api/dynamic/123?apple=dope') + const record = await getErrorRecord({ + errorMessage: 'pages-api-node-error', + }) + + expect(record).toMatchObject({ + payload: { + message: 'pages-api-node-error', + request: { + url: '/api/dynamic/123?apple=dope', + }, + context: { + routePath: '/api/dynamic/[id]', + }, + }, + }) + }) + }) }) diff --git a/test/e2e/on-request-error/dynamic-routes/pages/api/dynamic/[id].js b/test/e2e/on-request-error/dynamic-routes/pages/api/dynamic/[id].js new file mode 100644 index 0000000000000..7c25dccfe8f2f --- /dev/null +++ b/test/e2e/on-request-error/dynamic-routes/pages/api/dynamic/[id].js @@ -0,0 +1,3 @@ +export default function handler() { + throw new Error('pages-api-node-error') +} diff --git a/test/e2e/on-request-error/dynamic-routes/pages/pages-page/dynamic/[id].js b/test/e2e/on-request-error/dynamic-routes/pages/pages-page/dynamic/[id].js new file mode 100644 index 0000000000000..95eedf546498b --- /dev/null +++ b/test/e2e/on-request-error/dynamic-routes/pages/pages-page/dynamic/[id].js @@ -0,0 +1,9 @@ +export default function Page() { + throw new Error('pages-page-node-error') +} + +export function getServerSideProps() { + return { + props: {}, + } +} diff --git a/test/e2e/on-request-error/server-action-error/server-action-error.test.ts b/test/e2e/on-request-error/server-action-error/server-action-error.test.ts index 492759a3a0cc0..b092494fcfc7a 100644 --- a/test/e2e/on-request-error/server-action-error/server-action-error.test.ts +++ b/test/e2e/on-request-error/server-action-error/server-action-error.test.ts @@ -1,5 +1,6 @@ import { nextTestSetup } from 'e2e-utils' import { retry } from 'next-test-utils' +import { getOutputLogJson } from '../_testing/utils' describe('on-request-error - server-action-error', () => { const { next, skipped } = nextTestSetup({ @@ -16,14 +17,6 @@ describe('on-request-error - server-action-error', () => { const outputLogPath = 'output-log.json' - async function getOutputLogJson() { - if (!(await next.hasFile(outputLogPath))) { - return {} - } - const content = await next.readFile(outputLogPath) - return JSON.parse(content) - } - async function validateErrorRecord({ errorMessage, url, @@ -42,7 +35,7 @@ describe('on-request-error - server-action-error', () => { ) }, 5000) - const json = await getOutputLogJson() + const json = await getOutputLogJson(next, outputLogPath) const record = json[errorMessage] // Assert error is recorded in the output log From af3b329d8ef8f27ef86ca1ff09f6dab04e35e634 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 17 Jul 2024 15:13:36 +0200 Subject: [PATCH 3/5] set page to dynamic --- .../dynamic-routes/app/app-page/dynamic/[id]/page.js | 2 ++ .../dynamic-routes/app/app-route/dynamic/[id]/route.js | 2 ++ test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/test/e2e/on-request-error/dynamic-routes/app/app-page/dynamic/[id]/page.js b/test/e2e/on-request-error/dynamic-routes/app/app-page/dynamic/[id]/page.js index 6c5bd83380e69..72230748dac61 100644 --- a/test/e2e/on-request-error/dynamic-routes/app/app-page/dynamic/[id]/page.js +++ b/test/e2e/on-request-error/dynamic-routes/app/app-page/dynamic/[id]/page.js @@ -1,3 +1,5 @@ export default function Page() { throw new Error('server-dynamic-page-node-error') } + +export const dynamic = 'force-dynamic' diff --git a/test/e2e/on-request-error/dynamic-routes/app/app-route/dynamic/[id]/route.js b/test/e2e/on-request-error/dynamic-routes/app/app-route/dynamic/[id]/route.js index cf8c4cbcdc740..8a0aa34f4d3c3 100644 --- a/test/e2e/on-request-error/dynamic-routes/app/app-route/dynamic/[id]/route.js +++ b/test/e2e/on-request-error/dynamic-routes/app/app-route/dynamic/[id]/route.js @@ -1,3 +1,5 @@ export function GET() { throw new Error('server-dynamic-route-node-error') } + +export const dynamic = 'force-dynamic' diff --git a/test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts b/test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts index 56f664a46f4e7..456ef02fc1345 100644 --- a/test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts +++ b/test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts @@ -75,7 +75,7 @@ describe('on-request-error - dynamic-routes', () => { }) }) - it('should catch client component page error in node runtime', async () => { + it('should catch suspense rendering page error in node runtime', async () => { await next.fetch('/app-page/suspense') const record = await getErrorRecord({ errorMessage: 'server-suspense-page-node-error', From 2013224f50f50d07a3d5a059cdf5164cb7e3eefa Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 17 Jul 2024 17:35:47 +0200 Subject: [PATCH 4/5] drop the experimental env var --- packages/next/src/server/base-server.ts | 5 +---- packages/next/src/server/web/globals.ts | 7 +------ test/e2e/on-request-error/basic/basic.test.ts | 3 --- .../on-request-error/dynamic-routes/dynamic-routes.test.ts | 3 --- .../server-action-error/server-action-error.test.ts | 3 --- .../skip-next-internal-error.test.ts | 3 --- 6 files changed, 2 insertions(+), 22 deletions(-) diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index d8601e571aa94..40afd0988804c 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -836,10 +836,7 @@ export default abstract class Server< ) { const [err, req, ctx] = args - if ( - process.env.__NEXT_EXPERIMENTAL_INSTRUMENTATION && - this.instrumentation - ) { + if (this.instrumentation) { try { await this.instrumentation.onRequestError?.( err, diff --git a/packages/next/src/server/web/globals.ts b/packages/next/src/server/web/globals.ts index 5e5ce7c8e56cc..6c4bf22ef2e8c 100644 --- a/packages/next/src/server/web/globals.ts +++ b/packages/next/src/server/web/globals.ts @@ -37,12 +37,7 @@ export async function edgeInstrumentationOnRequestError( ) { const instrumentation = await getEdgeInstrumentationModule() try { - if ( - process.env.__NEXT_EXPERIMENTAL_INSTRUMENTATION && - instrumentation?.onRequestError - ) { - await instrumentation.onRequestError(...args) - } + await instrumentation?.onRequestError?.(...args) } catch (err) { // Log the soft error and continue, since the original error has already been thrown console.error('Error in instrumentation.onRequestError:', err) diff --git a/test/e2e/on-request-error/basic/basic.test.ts b/test/e2e/on-request-error/basic/basic.test.ts index 3e018fbf84037..0af57ce7077a9 100644 --- a/test/e2e/on-request-error/basic/basic.test.ts +++ b/test/e2e/on-request-error/basic/basic.test.ts @@ -6,9 +6,6 @@ describe('on-request-error - basic', () => { const { next, skipped } = nextTestSetup({ files: __dirname, skipDeployment: true, - env: { - __NEXT_EXPERIMENTAL_INSTRUMENTATION: '1', - }, }) if (skipped) { diff --git a/test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts b/test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts index 456ef02fc1345..66c3d447bbfee 100644 --- a/test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts +++ b/test/e2e/on-request-error/dynamic-routes/dynamic-routes.test.ts @@ -6,9 +6,6 @@ describe('on-request-error - dynamic-routes', () => { const { next, skipped } = nextTestSetup({ files: __dirname, skipDeployment: true, - env: { - __NEXT_EXPERIMENTAL_INSTRUMENTATION: '1', - }, }) if (skipped) { diff --git a/test/e2e/on-request-error/server-action-error/server-action-error.test.ts b/test/e2e/on-request-error/server-action-error/server-action-error.test.ts index b092494fcfc7a..33f45afa9e652 100644 --- a/test/e2e/on-request-error/server-action-error/server-action-error.test.ts +++ b/test/e2e/on-request-error/server-action-error/server-action-error.test.ts @@ -6,9 +6,6 @@ describe('on-request-error - server-action-error', () => { const { next, skipped } = nextTestSetup({ files: __dirname, skipDeployment: true, - env: { - __NEXT_EXPERIMENTAL_INSTRUMENTATION: '1', - }, }) if (skipped) { diff --git a/test/e2e/on-request-error/skip-next-internal-error/skip-next-internal-error.test.ts b/test/e2e/on-request-error/skip-next-internal-error/skip-next-internal-error.test.ts index de8016c020e39..b34f2e4a1c7c5 100644 --- a/test/e2e/on-request-error/skip-next-internal-error/skip-next-internal-error.test.ts +++ b/test/e2e/on-request-error/skip-next-internal-error/skip-next-internal-error.test.ts @@ -4,9 +4,6 @@ describe('on-request-error - skip-next-internal-error', () => { const { next, skipped } = nextTestSetup({ files: __dirname, skipDeployment: true, - env: { - __NEXT_EXPERIMENTAL_INSTRUMENTATION: '1', - }, }) if (skipped) { From 517f0e0eaebc255456d469ee8a26208b2b4aeb4d Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 17 Jul 2024 22:26:58 +0200 Subject: [PATCH 5/5] Expose TS type for onRequestError (#67859) ## What Expose the TS types for onRequestError API ### API ```ts type RequestErrorContext = { routerKind: 'Pages Router' | 'App Router'; routePath: string; routeType: 'render' | 'route' | 'action' | 'middleware'; renderSource?: 'react-server-components' | 'react-server-components-payload' | 'server-rendering'; }; export declare namespace Instrumentation { type onRequestError = (error: unknown, errorRequest: Readonly<{ method: string; url: string; headers: NodeJS.Dict; }>, errorContext: Readonly) => void | Promise; ``` ### Usage ```ts // instrumentation.ts import { type Instrumentation } from 'next' export const onRequestError: Instrumentation.onRequestError = ( err, request, context ) => { //... } ``` --- packages/next/src/server/instrumentation/types.ts | 4 ++++ packages/next/src/types.ts | 2 ++ .../basic/{instrumentation.js => instrumentation.ts} | 10 ++++++++-- 3 files changed, 14 insertions(+), 2 deletions(-) rename test/e2e/on-request-error/basic/{instrumentation.js => instrumentation.ts} (54%) diff --git a/packages/next/src/server/instrumentation/types.ts b/packages/next/src/server/instrumentation/types.ts index 55ee90632932c..1c260cf79fb4e 100644 --- a/packages/next/src/server/instrumentation/types.ts +++ b/packages/next/src/server/instrumentation/types.ts @@ -23,3 +23,7 @@ export type InstrumentationModule = { register?(): void onRequestError?: InstrumentationOnRequestError } + +export namespace Instrumentation { + export type onRequestError = InstrumentationOnRequestError +} diff --git a/packages/next/src/types.ts b/packages/next/src/types.ts index fcb4c47b73cef..396abc74fdb73 100644 --- a/packages/next/src/types.ts +++ b/packages/next/src/types.ts @@ -38,6 +38,8 @@ export type { ResolvedViewport, } from './lib/metadata/types/metadata-interface' +export type { Instrumentation } from './server/instrumentation/types' + /** * Stub route type for typedRoutes before `next dev` or `next build` is run * @link https://nextjs.org/docs/app/building-your-application/configuring/typescript#statically-typed-links diff --git a/test/e2e/on-request-error/basic/instrumentation.js b/test/e2e/on-request-error/basic/instrumentation.ts similarity index 54% rename from test/e2e/on-request-error/basic/instrumentation.js rename to test/e2e/on-request-error/basic/instrumentation.ts index b9944b351236c..6310bffac27fc 100644 --- a/test/e2e/on-request-error/basic/instrumentation.js +++ b/test/e2e/on-request-error/basic/instrumentation.ts @@ -1,8 +1,14 @@ -export function onRequestError(err, request, context) { +import { type Instrumentation } from 'next' + +export const onRequestError: Instrumentation.onRequestError = ( + err, + request, + context +) => { fetch(`http://localhost:${process.env.PORT}/write-log`, { method: 'POST', body: JSON.stringify({ - message: err.message, + message: (err as Error).message, request, context, }),