From af79bd937c54566954ea5d68d0395b1963e94bb9 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 28 Feb 2023 08:48:09 +0000 Subject: [PATCH 01/16] feat(nextjs): Automatically resolve errors happening in dev mode --- packages/nextjs/package.json | 1 + packages/nextjs/rollup.npm.config.js | 4 +- packages/nextjs/src/client/index.ts | 5 + .../devErrorSymbolicationEventProcessor.ts | 160 ++++++++++++++++++ packages/nextjs/src/server/index.ts | 5 + yarn.lock | 2 +- 6 files changed, 175 insertions(+), 2 deletions(-) create mode 100644 packages/nextjs/src/common/devErrorSymbolicationEventProcessor.ts diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 34f551012a64..ef792a3c622d 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -32,6 +32,7 @@ }, "devDependencies": { "@types/webpack": "^4.41.31", + "stacktrace-parser": "^0.1.10", "eslint-plugin-react": "^7.31.11", "next": "10.1.3" }, diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index 5b23aa2fa0a4..dcd956e3a79b 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -16,7 +16,9 @@ export default [ // prevent this internal nextjs code from ending up in our built package (this doesn't happen automatially because // the name doesn't match an SDK dependency) - packageSpecificConfig: { external: ['next/router', 'next/constants', 'next/headers'] }, + packageSpecificConfig: { + external: ['next/router', 'next/constants', 'next/headers', 'next/dist/compiled/stacktrace-parser'], + }, }), ), ...makeNPMConfigVariants( diff --git a/packages/nextjs/src/client/index.ts b/packages/nextjs/src/client/index.ts index 9894067d69e2..550daf53cf55 100644 --- a/packages/nextjs/src/client/index.ts +++ b/packages/nextjs/src/client/index.ts @@ -5,6 +5,7 @@ import { configureScope, init as reactInit, Integrations } from '@sentry/react'; import { BrowserTracing, defaultRequestInstrumentationOptions } from '@sentry/tracing'; import type { EventProcessor } from '@sentry/types'; +import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor'; import { getVercelEnv } from '../common/getVercelEnv'; import { buildMetadata } from '../common/metadata'; import { addOrUpdateIntegration } from '../common/userIntegrations'; @@ -53,6 +54,10 @@ export function init(options: BrowserOptions): void { event.type === 'transaction' && event.transaction === '/404' ? null : event; filterTransactions.id = 'NextClient404Filter'; scope.addEventProcessor(filterTransactions); + + if (process.env.NODE_ENV === 'development') { + scope.addEventProcessor(devErrorSymbolicationEventProcessor); + } }); } diff --git a/packages/nextjs/src/common/devErrorSymbolicationEventProcessor.ts b/packages/nextjs/src/common/devErrorSymbolicationEventProcessor.ts new file mode 100644 index 000000000000..317ef9404b46 --- /dev/null +++ b/packages/nextjs/src/common/devErrorSymbolicationEventProcessor.ts @@ -0,0 +1,160 @@ +import type { Event, EventHint } from '@sentry/types'; +import type { parse, StackFrame } from 'stacktrace-parser'; + +type OriginalStackFrameResponse = { + originalStackFrame: StackFrame; + originalCodeFrame: string | null; + sourcePackage?: string; +}; + +async function resolveStackFrame( + frame: StackFrame, + error: Error, +): Promise<{ originalCodeFrame: string | null; originalStackFrame: StackFrame | null } | null> { + try { + if (!(frame.file?.startsWith('webpack-internal:') || frame.file?.startsWith('file:'))) { + return null; + } + + const params = new URLSearchParams(); + params.append('isServer', String(false)); // doesn't matter since it is overwritten by isAppDirectory + params.append('isEdgeServer', String(false)); // doesn't matter since it is overwritten by isAppDirectory + params.append('isAppDirectory', String(true)); // will force server to do more thorough checking + params.append('errorMessage', error.toString()); + Object.keys(frame).forEach(key => { + params.append(key, (frame[key as keyof typeof frame] ?? '').toString()); + }); + + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), 3000); + const res = await fetch( + `${ + // eslint-disable-next-line no-restricted-globals + typeof window === 'undefined' ? 'http://localhost:3000' : '' // TODO: handle the case where users define a different port + }/__nextjs_original-stack-frame?${params.toString()}`, + { + signal: controller.signal, + }, + ).finally(() => { + clearTimeout(timer); + }); + + if (!res.ok || res.status === 204) { + return null; + } + + const body: OriginalStackFrameResponse = await res.json(); + + return { + originalCodeFrame: body.originalCodeFrame, + originalStackFrame: body.originalStackFrame, + }; + } catch (e) { + return null; + } +} + +function parseOriginalCodeFrame(codeFrame: string): { + contextLine: string | undefined; + preContextLines: string[]; + postContextLines: string[]; +} { + const preProcessedLines = codeFrame + // Remove ASCII control characters that are used for syntax highlighting + .replace( + // eslint-disable-next-line no-control-regex + /[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g, // https://stackoverflow.com/a/29497680 + '', + ) + .split('\n') + // Remove line that is supposed to indicate where the error happened + .filter(line => !line.match(/^\s*\|/)) + // Find the error line + .map(line => ({ + line, + isErrorLine: !!line.match(/^>/), + })) + // Remove the leading part that is just for prettier output + .map(lineObj => ({ + ...lineObj, + line: lineObj.line.replace(/^.*\|/, ''), + })); + + const preContextLines = []; + let contextLine: string | undefined = undefined; + const postContextLines = []; + + let reachedContextLine = false; + + for (const preProcessedLine of preProcessedLines) { + if (preProcessedLine.isErrorLine) { + contextLine = preProcessedLine.line; + reachedContextLine = true; + } else if (reachedContextLine) { + postContextLines.push(preProcessedLine.line); + } else { + preContextLines.push(preProcessedLine.line); + } + } + + return { + contextLine, + preContextLines, + postContextLines, + }; +} + +/** + * TODO + */ +export async function devErrorSymbolicationEventProcessor(event: Event, hint: EventHint): Promise { + let stackTraceParser: { parse: typeof parse }; + try { + // @ts-ignore it's there () + // eslint-disable-next-line import/no-unresolved + const stackTraceParserPromise = import('next/dist/compiled/stacktrace-parser'); + stackTraceParser = await stackTraceParserPromise; + } catch (e) { + return event; + } + + if (hint.originalException && hint.originalException instanceof Error && hint.originalException.stack) { + const frames = stackTraceParser.parse(hint.originalException.stack); + + const resolvedFrames = await Promise.all( + frames.map(async frame => await resolveStackFrame(frame, hint.originalException as Error)), + ); + + if (event.exception?.values?.[0].stacktrace?.frames) { + event.exception.values[0].stacktrace.frames = event.exception.values[0].stacktrace.frames.map( + (frame, i, frames) => { + const resolvedFrame = resolvedFrames[frames.length - 1 - i]; + if (!resolvedFrame || !resolvedFrame.originalStackFrame || !resolvedFrame.originalCodeFrame) { + return { + ...frame, + platform: frame.abs_path?.startsWith('node:internal') ? 'nodejs' : undefined, // simple hack that will prevent a source mapping error from showing up + in_app: false, + }; + } + + const { contextLine, preContextLines, postContextLines } = parseOriginalCodeFrame( + resolvedFrame.originalCodeFrame, + ); + + return { + ...frame, + pre_context: preContextLines, + context_line: contextLine, + post_context: postContextLines, + function: resolvedFrame.originalStackFrame.methodName, + filename: resolvedFrame.originalStackFrame.file || undefined, + lineno: resolvedFrame.originalStackFrame.lineNumber || undefined, + colno: resolvedFrame.originalStackFrame.column || undefined, + }; + }, + ); + } + } + + return event; +} diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 8c16195aa570..b38ee49946f2 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -8,6 +8,7 @@ import { escapeStringForRegex, logger } from '@sentry/utils'; import * as domainModule from 'domain'; import * as path from 'path'; +import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor'; import { getVercelEnv } from '../common/getVercelEnv'; import { buildMetadata } from '../common/metadata'; import type { IntegrationWithExclusionOption } from '../common/userIntegrations'; @@ -112,6 +113,10 @@ export function init(options: NodeOptions): void { } scope.addEventProcessor(filterTransactions); + + if (process.env.NODE_ENV === 'development') { + scope.addEventProcessor(devErrorSymbolicationEventProcessor); + } }); if (activeDomain) { diff --git a/yarn.lock b/yarn.lock index 34c5254dbcb6..a64134ad26bd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -24103,7 +24103,7 @@ stack-utils@^2.0.3: dependencies: escape-string-regexp "^2.0.0" -stacktrace-parser@0.1.10: +stacktrace-parser@0.1.10, stacktrace-parser@^0.1.10: version "0.1.10" resolved "https://registry.yarnpkg.com/stacktrace-parser/-/stacktrace-parser-0.1.10.tgz#29fb0cae4e0d0b85155879402857a1639eb6051a" integrity sha512-KJP1OCML99+8fhOHxwwzyWrlUuVX5GQ0ZpJTd1DFXhdkrvg1szxfHhawXUZ3g9TkXORQd4/WG68jMlQZ2p8wlg== From 48e1a3b835a8e22d9f13112f161b04c56f293b55 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 28 Feb 2023 08:51:39 +0000 Subject: [PATCH 02/16] Add comment --- .../nextjs/src/common/devErrorSymbolicationEventProcessor.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/nextjs/src/common/devErrorSymbolicationEventProcessor.ts b/packages/nextjs/src/common/devErrorSymbolicationEventProcessor.ts index 317ef9404b46..5e018a77b278 100644 --- a/packages/nextjs/src/common/devErrorSymbolicationEventProcessor.ts +++ b/packages/nextjs/src/common/devErrorSymbolicationEventProcessor.ts @@ -105,7 +105,8 @@ function parseOriginalCodeFrame(codeFrame: string): { } /** - * TODO + * Event processor that will symbolicate errors by using the webpack/nextjs dev server that is used to show stack traces + * in the dev overlay. */ export async function devErrorSymbolicationEventProcessor(event: Event, hint: EventHint): Promise { let stackTraceParser: { parse: typeof parse }; From 617e34246becc16b05873bf059da9e9fbc75bd17 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 28 Feb 2023 08:56:07 +0000 Subject: [PATCH 03/16] try catch --- .../devErrorSymbolicationEventProcessor.ts | 74 +++++++++---------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/packages/nextjs/src/common/devErrorSymbolicationEventProcessor.ts b/packages/nextjs/src/common/devErrorSymbolicationEventProcessor.ts index 5e018a77b278..9d6a37fee7a2 100644 --- a/packages/nextjs/src/common/devErrorSymbolicationEventProcessor.ts +++ b/packages/nextjs/src/common/devErrorSymbolicationEventProcessor.ts @@ -109,52 +109,52 @@ function parseOriginalCodeFrame(codeFrame: string): { * in the dev overlay. */ export async function devErrorSymbolicationEventProcessor(event: Event, hint: EventHint): Promise { - let stackTraceParser: { parse: typeof parse }; + // Do to changes across Next.js versions, there are a million things that can go wrong here so we just try-catch the + // entire event processor.Symbolicated stack traces are just a nice to have. try { - // @ts-ignore it's there () + // @ts-ignore it's there // eslint-disable-next-line import/no-unresolved - const stackTraceParserPromise = import('next/dist/compiled/stacktrace-parser'); - stackTraceParser = await stackTraceParserPromise; - } catch (e) { - return event; - } + const stackTraceParser: { parse: typeof parse } = import('next/dist/compiled/stacktrace-parser'); + + if (hint.originalException && hint.originalException instanceof Error && hint.originalException.stack) { + const frames = stackTraceParser.parse(hint.originalException.stack); - if (hint.originalException && hint.originalException instanceof Error && hint.originalException.stack) { - const frames = stackTraceParser.parse(hint.originalException.stack); + const resolvedFrames = await Promise.all( + frames.map(async frame => await resolveStackFrame(frame, hint.originalException as Error)), + ); - const resolvedFrames = await Promise.all( - frames.map(async frame => await resolveStackFrame(frame, hint.originalException as Error)), - ); + if (event.exception?.values?.[0].stacktrace?.frames) { + event.exception.values[0].stacktrace.frames = event.exception.values[0].stacktrace.frames.map( + (frame, i, frames) => { + const resolvedFrame = resolvedFrames[frames.length - 1 - i]; + if (!resolvedFrame || !resolvedFrame.originalStackFrame || !resolvedFrame.originalCodeFrame) { + return { + ...frame, + platform: frame.abs_path?.startsWith('node:internal') ? 'nodejs' : undefined, // simple hack that will prevent a source mapping error from showing up + in_app: false, + }; + } + + const { contextLine, preContextLines, postContextLines } = parseOriginalCodeFrame( + resolvedFrame.originalCodeFrame, + ); - if (event.exception?.values?.[0].stacktrace?.frames) { - event.exception.values[0].stacktrace.frames = event.exception.values[0].stacktrace.frames.map( - (frame, i, frames) => { - const resolvedFrame = resolvedFrames[frames.length - 1 - i]; - if (!resolvedFrame || !resolvedFrame.originalStackFrame || !resolvedFrame.originalCodeFrame) { return { ...frame, - platform: frame.abs_path?.startsWith('node:internal') ? 'nodejs' : undefined, // simple hack that will prevent a source mapping error from showing up - in_app: false, + pre_context: preContextLines, + context_line: contextLine, + post_context: postContextLines, + function: resolvedFrame.originalStackFrame.methodName, + filename: resolvedFrame.originalStackFrame.file || undefined, + lineno: resolvedFrame.originalStackFrame.lineNumber || undefined, + colno: resolvedFrame.originalStackFrame.column || undefined, }; - } - - const { contextLine, preContextLines, postContextLines } = parseOriginalCodeFrame( - resolvedFrame.originalCodeFrame, - ); - - return { - ...frame, - pre_context: preContextLines, - context_line: contextLine, - post_context: postContextLines, - function: resolvedFrame.originalStackFrame.methodName, - filename: resolvedFrame.originalStackFrame.file || undefined, - lineno: resolvedFrame.originalStackFrame.lineNumber || undefined, - colno: resolvedFrame.originalStackFrame.column || undefined, - }; - }, - ); + }, + ); + } } + } catch (e) { + return event; } return event; From a9f118c5819012006bd60fad9139a0542ddcda07 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 28 Feb 2023 09:08:23 +0000 Subject: [PATCH 04/16] e2e --- .../nextjs-13-app-dir/package.json | 3 ++- .../nextjs-13-app-dir/playwright.config.ts | 8 +++++++- .../nextjs-13-app-dir/test-recipe.json | 15 +++++++++++++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/packages/e2e-tests/test-applications/nextjs-13-app-dir/package.json b/packages/e2e-tests/test-applications/nextjs-13-app-dir/package.json index 2bb563d85e25..8ed25dbf0b8c 100644 --- a/packages/e2e-tests/test-applications/nextjs-13-app-dir/package.json +++ b/packages/e2e-tests/test-applications/nextjs-13-app-dir/package.json @@ -7,7 +7,8 @@ "build": "next build", "start": "next start", "lint": "next lint", - "test": "playwright test" + "test:prod": "TEST_ENV=production playwright test", + "test:dev": "TEST_ENV=development playwright test" }, "dependencies": { "@next/font": "13.0.7", diff --git a/packages/e2e-tests/test-applications/nextjs-13-app-dir/playwright.config.ts b/packages/e2e-tests/test-applications/nextjs-13-app-dir/playwright.config.ts index 55e54aefaefa..20003101d2fe 100644 --- a/packages/e2e-tests/test-applications/nextjs-13-app-dir/playwright.config.ts +++ b/packages/e2e-tests/test-applications/nextjs-13-app-dir/playwright.config.ts @@ -1,6 +1,12 @@ import type { PlaywrightTestConfig } from '@playwright/test'; import { devices } from '@playwright/test'; +const testEnv = process.env.TEST_ENV; + +if (!testEnv) { + throw new Error('No test env defined'); +} + /** * See https://playwright.dev/docs/test-configuration. */ @@ -49,7 +55,7 @@ const config: PlaywrightTestConfig = { /* Run your local dev server before starting the tests */ webServer: [ { - command: 'yarn start', + command: testEnv === 'development' ? 'yarn dev' : 'yarn start', port: 3000, }, { diff --git a/packages/e2e-tests/test-applications/nextjs-13-app-dir/test-recipe.json b/packages/e2e-tests/test-applications/nextjs-13-app-dir/test-recipe.json index bfdef9508d7b..e5d433707e05 100644 --- a/packages/e2e-tests/test-applications/nextjs-13-app-dir/test-recipe.json +++ b/packages/e2e-tests/test-applications/nextjs-13-app-dir/test-recipe.json @@ -4,8 +4,19 @@ "buildCommand": "yarn install --pure-lockfile && npx playwright install && yarn build", "tests": [ { - "testName": "Playwright tests", - "testCommand": "yarn test" + "testName": "Production Build", + "testCommand": "yarn test:prod" + }, + { + "testName": "Development Build", + "testCommand": "yarn test:dev" + } + ], + "canaryVersions": [ + { + "dependencyOverrides": { + "next": "latest" + } } ] } From c7dfdb9c095cf0fb3d85cc9a3881b9a342c7caea Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 28 Feb 2023 09:30:57 +0000 Subject: [PATCH 05/16] bust cache From d965cfb01e981bd1e24b188de3a743efd67f822e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 28 Feb 2023 09:50:44 +0000 Subject: [PATCH 06/16] test --- .../app/client-component/page.tsx | 16 +++++++++++++ .../tests/devErrorSymbolification.test.ts | 24 +++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 packages/e2e-tests/test-applications/nextjs-13-app-dir/app/client-component/page.tsx create mode 100644 packages/e2e-tests/test-applications/nextjs-13-app-dir/tests/devErrorSymbolification.test.ts diff --git a/packages/e2e-tests/test-applications/nextjs-13-app-dir/app/client-component/page.tsx b/packages/e2e-tests/test-applications/nextjs-13-app-dir/app/client-component/page.tsx new file mode 100644 index 000000000000..c4f65bdf00d3 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-13-app-dir/app/client-component/page.tsx @@ -0,0 +1,16 @@ +export default async function Page() { + return ( +
+

Press to throw:

+ +
+ ); +} + +export const BUTTON_CLICK_ERROR_MESSAGE = 'button-click-error'; diff --git a/packages/e2e-tests/test-applications/nextjs-13-app-dir/tests/devErrorSymbolification.test.ts b/packages/e2e-tests/test-applications/nextjs-13-app-dir/tests/devErrorSymbolification.test.ts new file mode 100644 index 000000000000..e064681dadbc --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-13-app-dir/tests/devErrorSymbolification.test.ts @@ -0,0 +1,24 @@ +import { test } from '@playwright/test'; +import { waitForError } from '../../../test-utils/event-proxy-server'; +import { BUTTON_CLICK_ERROR_MESSAGE } from '../app/client-component/page'; + +test.describe('dev mode error symbolification', () => { + if (process.env.TEST_ENV !== 'development') { + test.skip('should be skipped for non-dev mode', () => {}); + return; + } + + test('should have symbaolicated dev errors', async ({ page }) => { + await page.goto('/client-component'); + + const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === BUTTON_CLICK_ERROR_MESSAGE; + }); + + const exceptionButton = page.locator('id=exception-button'); + await exceptionButton.click(); + + const errorEvent = await errorEventPromise; + console.log(JSON.stringify(errorEvent)); + }); +}); From 37dfc6dfa50c1397802ba582f295a82f6d77b055 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 28 Feb 2023 10:43:11 +0000 Subject: [PATCH 07/16] f --- .../nextjs-13-app-dir/app/client-component/page.tsx | 4 +--- .../nextjs-13-app-dir/tests/devErrorSymbolification.test.ts | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/e2e-tests/test-applications/nextjs-13-app-dir/app/client-component/page.tsx b/packages/e2e-tests/test-applications/nextjs-13-app-dir/app/client-component/page.tsx index c4f65bdf00d3..24527f0d892a 100644 --- a/packages/e2e-tests/test-applications/nextjs-13-app-dir/app/client-component/page.tsx +++ b/packages/e2e-tests/test-applications/nextjs-13-app-dir/app/client-component/page.tsx @@ -4,7 +4,7 @@ export default async function Page() {

Press to throw: