diff --git a/packages/e2e-tests/test-applications/create-next-app/playwright.config.ts b/packages/e2e-tests/test-applications/create-next-app/playwright.config.ts index dabfa9c23619..289976304812 100644 --- a/packages/e2e-tests/test-applications/create-next-app/playwright.config.ts +++ b/packages/e2e-tests/test-applications/create-next-app/playwright.config.ts @@ -24,7 +24,7 @@ const config: PlaywrightTestConfig = { /* Opt out of parallel tests on CI. */ workers: 1, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ - reporter: 'dot', + reporter: 'list', /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */ 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..b1d7f708cfae --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-13-app-dir/app/client-component/page.tsx @@ -0,0 +1,17 @@ +'use client'; + +export default function Page() { + return ( +
+

Press to throw:

+ +
+ ); +} diff --git a/packages/e2e-tests/test-applications/nextjs-13-app-dir/app/head.tsx b/packages/e2e-tests/test-applications/nextjs-13-app-dir/app/head.tsx deleted file mode 100644 index f11b259697ff..000000000000 --- a/packages/e2e-tests/test-applications/nextjs-13-app-dir/app/head.tsx +++ /dev/null @@ -1,10 +0,0 @@ -export default function Head() { - return ( - <> - Create Next App - - - - - ); -} 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..4e7a3cbd804d 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. */ @@ -13,18 +19,18 @@ const config: PlaywrightTestConfig = { * Maximum time expect() should wait for the condition to be met. * For example in `await expect(locator).toHaveText();` */ - timeout: 5000, + timeout: 10000, }, /* Run tests in files in parallel */ fullyParallel: true, /* Fail the build on CI if you accidentally left test.only in the source code. */ forbidOnly: !!process.env.CI, - /* Retry on CI only */ - retries: 0, + /* `next dev` is incredibly buggy with the app dir */ + retries: testEnv === 'development' ? 3 : 0, /* Opt out of parallel tests on CI. */ workers: 1, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ - reporter: 'dot', + reporter: 'list', /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */ @@ -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/start-event-proxy.ts b/packages/e2e-tests/test-applications/nextjs-13-app-dir/start-event-proxy.ts index 9c959e1e180e..c4b99606e9bb 100644 --- a/packages/e2e-tests/test-applications/nextjs-13-app-dir/start-event-proxy.ts +++ b/packages/e2e-tests/test-applications/nextjs-13-app-dir/start-event-proxy.ts @@ -1,4 +1,4 @@ -import { startEventProxyServer, waitForTransaction } from '../../test-utils/event-proxy-server'; +import { startEventProxyServer } from '../../test-utils/event-proxy-server'; startEventProxyServer({ port: 27496, 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..d1fc9eafa240 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": "Prod Mode", + "testCommand": "yarn test:prod" + }, + { + "testName": "Dev Mode", + "testCommand": "yarn test:dev" + } + ], + "canaryVersions": [ + { + "dependencyOverrides": { + "next": "latest" + } } ] } 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..25e2b9d19067 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-13-app-dir/tests/devErrorSymbolification.test.ts @@ -0,0 +1,36 @@ +import { test, expect } from '@playwright/test'; +import { waitForError } from '../../../test-utils/event-proxy-server'; + +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 symbolicated dev errors', async ({ page }) => { + await page.goto('/client-component'); + + const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'client-component-button-click-error'; + }); + + await page.locator('id=exception-button').click(); + + const errorEvent = await errorEventPromise; + const errorEventFrames = errorEvent.exception?.values?.[0]?.stacktrace?.frames; + + expect(errorEventFrames?.[errorEventFrames?.length - 1]).toEqual( + expect.objectContaining({ + filename: 'app/client-component/page.tsx', + abs_path: 'webpack-internal:///(app-client)/./app/client-component/page.tsx', + function: 'onClick', + in_app: true, + lineno: 10, + colno: 16, + pre_context: [' id="exception-button"', ' onClick={() => {'], + context_line: " throw new Error('client-component-button-click-error');", + post_context: [' }}', ' >', ' throw'], + }), + ); + }); +}); diff --git a/packages/e2e-tests/test-applications/nextjs-13-app-dir/tests/transactions.test.ts b/packages/e2e-tests/test-applications/nextjs-13-app-dir/tests/transactions.test.ts index c52ab35475bd..2b166954074c 100644 --- a/packages/e2e-tests/test-applications/nextjs-13-app-dir/tests/transactions.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-13-app-dir/tests/transactions.test.ts @@ -46,44 +46,47 @@ test('Sends a pageload transaction', async ({ page }) => { .toBe(200); }); -test('Sends a transaction for a server component', async ({ page }) => { - const serverComponentTransactionPromise = waitForTransaction('nextjs-13-app-dir', transactionEvent => { - return ( - transactionEvent?.contexts?.trace?.op === 'function.nextjs' && - transactionEvent?.transaction === 'Page Server Component (/user/[id])' - ); - }); +if (process.env.TEST_ENV === 'production') { + // TODO: Fix that this is flakey on dev server - might be an SDK bug + test('Sends a transaction for a server component', async ({ page }) => { + const serverComponentTransactionPromise = waitForTransaction('nextjs-13-app-dir', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'function.nextjs' && + transactionEvent?.transaction === 'Page Server Component (/user/[id])' + ); + }); - await page.goto('/user/4'); + await page.goto('/user/4'); - const transactionEvent = await serverComponentTransactionPromise; - const transactionEventId = transactionEvent.event_id; + const transactionEvent = await serverComponentTransactionPromise; + const transactionEventId = transactionEvent.event_id; - await expect - .poll( - async () => { - try { - const response = await axios.get( - `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${transactionEventId}/`, - { headers: { Authorization: `Bearer ${authToken}` } }, - ); + await expect + .poll( + async () => { + try { + const response = await axios.get( + `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${transactionEventId}/`, + { headers: { Authorization: `Bearer ${authToken}` } }, + ); - return response.status; - } catch (e) { - if (e instanceof AxiosError && e.response) { - if (e.response.status !== 404) { - throw e; + return response.status; + } catch (e) { + if (e instanceof AxiosError && e.response) { + if (e.response.status !== 404) { + throw e; + } else { + return e.response.status; + } } else { - return e.response.status; + throw e; } - } else { - throw e; } - } - }, - { - timeout: EVENT_POLLING_TIMEOUT, - }, - ) - .toBe(200); -}); + }, + { + timeout: EVENT_POLLING_TIMEOUT, + }, + ) + .toBe(200); + }); +} diff --git a/packages/e2e-tests/test-applications/standard-frontend-react/playwright.config.ts b/packages/e2e-tests/test-applications/standard-frontend-react/playwright.config.ts index 7990109edc1e..9249280b4c34 100644 --- a/packages/e2e-tests/test-applications/standard-frontend-react/playwright.config.ts +++ b/packages/e2e-tests/test-applications/standard-frontend-react/playwright.config.ts @@ -24,7 +24,7 @@ const config: PlaywrightTestConfig = { /* Opt out of parallel tests on CI. */ workers: 1, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ - reporter: 'dot', + reporter: 'list', /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */ diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 411f95635160..74b8066ddf84 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -28,7 +28,8 @@ "@sentry/webpack-plugin": "1.20.0", "chalk": "3.0.0", "rollup": "2.78.0", - "tslib": "^1.9.3" + "tslib": "^1.9.3", + "stacktrace-parser": "^0.1.10" }, "devDependencies": { "@types/webpack": "^4.41.31", diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index 5b23aa2fa0a4..19e349f70f8f 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', '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..2315f246cc91 --- /dev/null +++ b/packages/nextjs/src/common/devErrorSymbolicationEventProcessor.ts @@ -0,0 +1,157 @@ +import type { Event, EventHint } from '@sentry/types'; +import type { StackFrame } from 'stacktrace-parser'; +import * as stackTraceParser 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, + }; +} + +/** + * 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 { + // Due 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 { + 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, + }; + }, + ); + } + } + } catch (e) { + return event; + } + + 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 33ae0ff022dc..aa778db88c85 100644 --- a/yarn.lock +++ b/yarn.lock @@ -24198,7 +24198,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==