Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(nextjs): Automatically resolve source of errors in dev mode #7294

Merged
merged 18 commits into from
Mar 1, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -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). */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
'use client';

export default function Page() {
return (
<div>
<p>Press to throw:</p>
<button
id="exception-button"
onClick={() => {
throw new Error('client-component-button-click-error');
}}
>
throw
</button>
</div>
);
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
*/
Expand All @@ -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). */
Expand All @@ -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,
},
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { startEventProxyServer, waitForTransaction } from '../../test-utils/event-proxy-server';
import { startEventProxyServer } from '../../test-utils/event-proxy-server';

startEventProxyServer({
port: 27496,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
]
}
Original file line number Diff line number Diff line change
@@ -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'],
}),
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -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). */
Expand Down
3 changes: 2 additions & 1 deletion packages/nextjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 3 additions & 1 deletion packages/nextjs/rollup.npm.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
5 changes: 5 additions & 0 deletions packages/nextjs/src/client/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}
});
}

Expand Down