From 07cd2bc0c9fc1b7a642842ad8514547d76e16708 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 8 Apr 2024 13:53:10 +0000 Subject: [PATCH 1/6] Move servercomponent tests --- .../tests/server-components.test.ts | 83 +++++++++++++++++++ .../nextjs-app-dir/tests/transactions.test.ts | 69 --------------- 2 files changed, 83 insertions(+), 69 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts new file mode 100644 index 000000000000..0b844a0402b1 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts @@ -0,0 +1,83 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/event-proxy-server'; +import axios, { AxiosError } from 'axios'; + +const authToken = process.env.E2E_TEST_AUTH_TOKEN; +const sentryTestOrgSlug = process.env.E2E_TEST_SENTRY_ORG_SLUG; +const sentryTestProject = process.env.E2E_TEST_SENTRY_TEST_PROJECT; +const EVENT_POLLING_TIMEOUT = 90_000; + +test('Sends a transaction for a server component', async ({ page }) => { + // TODO: Fix that this is flakey on dev server - might be an SDK bug + test.skip(process.env.TEST_ENV === 'production', 'Flakey on dev-server'); + + const serverComponentTransactionPromise = waitForTransaction('nextjs-13-app-dir', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'function.nextjs' && + transactionEvent?.transaction === 'Page Server Component (/server-component/parameter/[...parameters])' + ); + }); + + await page.goto('/server-component/parameter/1337/42'); + + const transactionEvent = await serverComponentTransactionPromise; + const transactionEventId = transactionEvent.event_id; + + expect(transactionEvent.request?.headers).toBeDefined(); + + 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; + } else { + return e.response.status; + } + } else { + throw e; + } + } + }, + { + timeout: EVENT_POLLING_TIMEOUT, + }, + ) + .toBe(200); +}); + +test('Should not set an error status on a server component transaction when it redirects', async ({ page }) => { + // TODO: Fix that this is flakey on dev server - might be an SDK bug + test.skip(process.env.TEST_ENV === 'production', 'Flakey on dev-server'); + + const serverComponentTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return transactionEvent?.transaction === 'Page Server Component (/server-component/redirect)'; + }); + + await page.goto('/server-component/redirect'); + + expect((await serverComponentTransactionPromise).contexts?.trace?.status).not.toBe('internal_error'); +}); + +test('Should set a "not_found" status on a server component transaction when notFound() is called', async ({ + page, +}) => { + // TODO: Fix that this is flakey on dev server - might be an SDK bug + test.skip(process.env.TEST_ENV === 'production', 'Flakey on dev-server'); + + const serverComponentTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return transactionEvent?.transaction === 'Page Server Component (/server-component/not-found)'; + }); + + await page.goto('/server-component/not-found'); + + expect((await serverComponentTransactionPromise).contexts?.trace?.status).toBe('not_found'); +}); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts index 57ddb57f75cf..5f7d4dc8496d 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts @@ -48,75 +48,6 @@ test('Sends a pageload transaction', async ({ page }) => { .toBe(200); }); -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 (/server-component/parameter/[...parameters])' - ); - }); - - await page.goto('/server-component/parameter/1337/42'); - - const transactionEvent = await serverComponentTransactionPromise; - const transactionEventId = transactionEvent.event_id; - - expect(transactionEvent.request?.headers).toBeDefined(); - - 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; - } else { - return e.response.status; - } - } else { - throw e; - } - } - }, - { - timeout: EVENT_POLLING_TIMEOUT, - }, - ) - .toBe(200); - }); - - test('Should not set an error status on a server component transaction when it redirects', async ({ page }) => { - const serverComponentTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { - return transactionEvent?.transaction === 'Page Server Component (/server-component/redirect)'; - }); - - await page.goto('/server-component/redirect'); - - expect((await serverComponentTransactionPromise).contexts?.trace?.status).not.toBe('internal_error'); - }); - - test('Should set a "not_found" status on a server component transaction when notFound() is called', async ({ - page, - }) => { - const serverComponentTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { - return transactionEvent?.transaction === 'Page Server Component (/server-component/not-found)'; - }); - - await page.goto('/server-component/not-found'); - - expect((await serverComponentTransactionPromise).contexts?.trace?.status).toBe('not_found'); - }); -} - test('Should send a transaction for instrumented server actions', async ({ page }) => { const nextjsVersion = packageJson.dependencies.next; const nextjsMajor = Number(nextjsVersion.split('.')[0]); From 69e3d2f6c2483c80d53098cec746bc14350f5f44 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 8 Apr 2024 14:54:35 +0000 Subject: [PATCH 2/6] Add tests --- .../app/generation-functions/page.tsx | 6 +++++ .../tests/generation-functions.test.ts | 10 ++++++-- .../app/edge-server-components/error/page.tsx | 5 ++++ .../app/edge-server-components/page.tsx | 6 +++++ .../app/route-handlers/[param]/edge/route.ts | 5 ++++ .../app/route-handlers/[param]/error/route.ts | 6 +++++ .../app/server-component/faulty/page.tsx | 15 ++++++++++++ .../nextjs-app-dir/middleware.ts | 5 ++++ .../nextjs-app-dir/pages/api/edge-endpoint.ts | 6 +++++ .../pages/api/error-edge-endpoint.ts | 5 ++++ .../pages/pages-router/ssr-error-fc.tsx | 6 +++++ .../nextjs-app-dir/tests/edge-route.test.ts | 10 +++++++- .../nextjs-app-dir/tests/edge.test.ts | 12 +++++++++- .../nextjs-app-dir/tests/middleware.test.ts | 10 +++++++- .../tests/pages-ssr-errors.test.ts | 14 ++++++++--- .../tests/route-handlers.test.ts | 12 ++++++++++ .../tests/server-components.test.ts | 24 ++++++++++++++++++- 17 files changed, 148 insertions(+), 9 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/server-component/faulty/page.tsx diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/app/generation-functions/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-14/app/generation-functions/page.tsx index 5ae73102057d..92bee1dbac4b 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-14/app/generation-functions/page.tsx +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/app/generation-functions/page.tsx @@ -1,3 +1,6 @@ +import { getDefaultIsolationScope } from '@sentry/core'; +import * as Sentry from '@sentry/nextjs'; + export const dynamic = 'force-dynamic'; export default function Page() { @@ -9,6 +12,9 @@ export async function generateMetadata({ }: { searchParams: { [key: string]: string | string[] | undefined }; }) { + Sentry.setTag('my-isolated-tag', true); + Sentry.setTag('my-global-scope-isolated-tag', getDefaultIsolationScope().getScopeData().tags['my-isolated-tag']); // We set this tag to be able to assert that the previously set tag has not leaked into the global isolation scope + if (searchParams['shouldThrowInGenerateMetadata']) { throw new Error('generateMetadata Error'); } diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts index 52c28e1d974a..da08ccb481bf 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts @@ -37,8 +37,14 @@ test('Should send a transaction and an error event for a faulty generateMetadata await page.goto(`/generation-functions?metadataTitle=${testTitle}&shouldThrowInGenerateMetadata=1`); - expect(await transactionPromise).toBeDefined(); - expect(await errorEventPromise).toBeDefined(); + const errorEvent = await errorEventPromise; + const transactionEvent = await transactionPromise; + + // Assert that isolation scope works properly + expect(errorEvent.tags?.['my-isolated-tag']).toBe(true); + expect(errorEvent.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); + expect(transactionEvent.tags?.['my-isolated-tag']).toBe(true); + expect(transactionEvent.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); }); test('Should send a transaction event for a generateViewport() function invokation', async ({ page }) => { diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-server-components/error/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-server-components/error/page.tsx index 35d15616fd1c..1a86e2ac59cf 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-server-components/error/page.tsx +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-server-components/error/page.tsx @@ -1,7 +1,12 @@ +import { getDefaultIsolationScope } from '@sentry/core'; +import * as Sentry from '@sentry/nextjs'; + export const dynamic = 'force-dynamic'; export const runtime = 'edge'; export default async function Page() { + Sentry.setTag('my-isolated-tag', true); + Sentry.setTag('my-global-scope-isolated-tag', getDefaultIsolationScope().getScopeData().tags['my-isolated-tag']); // We set this tag to be able to assert that the previously set tag has not leaked into the global isolation scope throw new Error('Edge Server Component Error'); } diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-server-components/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-server-components/page.tsx index c7a6a8887e90..9d6ec241fca6 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-server-components/page.tsx +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/edge-server-components/page.tsx @@ -1,7 +1,13 @@ +import { getDefaultIsolationScope } from '@sentry/core'; +import * as Sentry from '@sentry/nextjs'; + export const dynamic = 'force-dynamic'; export const runtime = 'edge'; export default async function Page() { + Sentry.setTag('my-isolated-tag', true); + Sentry.setTag('my-global-scope-isolated-tag', getDefaultIsolationScope().getScopeData().tags['my-isolated-tag']); // We set this tag to be able to assert that the previously set tag has not leaked into the global isolation scope + return

Hello world!

; } diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/edge/route.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/edge/route.ts index a43862231568..8879a85c488a 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/edge/route.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/edge/route.ts @@ -1,3 +1,5 @@ +import { getDefaultIsolationScope } from '@sentry/core'; +import * as Sentry from '@sentry/nextjs'; import { NextResponse } from 'next/server'; export const runtime = 'edge'; @@ -7,5 +9,8 @@ export async function PATCH() { } export async function DELETE(): Promise { + Sentry.setTag('my-isolated-tag', true); + Sentry.setTag('my-global-scope-isolated-tag', getDefaultIsolationScope().getScopeData().tags['my-isolated-tag']); // We set this tag to be able to assert that the previously set tag has not leaked into the global isolation scope + throw new Error('route-handler-edge-error'); } diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/error/route.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/error/route.ts index e2de561c4783..e873849d22df 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/error/route.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/error/route.ts @@ -1,3 +1,9 @@ +import { getDefaultIsolationScope } from '@sentry/core'; +import * as Sentry from '@sentry/nextjs'; + export async function PUT(): Promise { + Sentry.setTag('my-isolated-tag', true); + Sentry.setTag('my-global-scope-isolated-tag', getDefaultIsolationScope().getScopeData().tags['my-isolated-tag']); // We set this tag to be able to assert that the previously set tag has not leaked into the global isolation scope + throw new Error('route-handler-error'); } diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/server-component/faulty/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/server-component/faulty/page.tsx new file mode 100644 index 000000000000..f31b3f1899da --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/server-component/faulty/page.tsx @@ -0,0 +1,15 @@ +import { getDefaultIsolationScope } from '@sentry/core'; +import * as Sentry from '@sentry/nextjs'; + +export const dynamic = 'force-dynamic'; + +export default async function FaultyServerComponent() { + Sentry.setTag('my-isolated-tag', true); + Sentry.setTag('my-global-scope-isolated-tag', getDefaultIsolationScope().getScopeData().tags['my-isolated-tag']); // We set this tag to be able to assert that the previously set tag has not leaked into the global isolation scope + + if (Math.random() + 1 > 0) { + throw new Error('I am a faulty server component'); + } + + return null; +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/middleware.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/middleware.ts index a491ccde0a91..6096fcfb1493 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/middleware.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/middleware.ts @@ -1,7 +1,12 @@ +import { getDefaultIsolationScope } from '@sentry/core'; +import * as Sentry from '@sentry/nextjs'; import { NextResponse } from 'next/server'; import type { NextRequest } from 'next/server'; export async function middleware(request: NextRequest) { + Sentry.setTag('my-isolated-tag', true); + Sentry.setTag('my-global-scope-isolated-tag', getDefaultIsolationScope().getScopeData().tags['my-isolated-tag']); // We set this tag to be able to assert that the previously set tag has not leaked into the global isolation scope + if (request.headers.has('x-should-throw')) { throw new Error('Middleware Error'); } diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/edge-endpoint.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/edge-endpoint.ts index b2b2dfdf4fc3..6236aa63d936 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/edge-endpoint.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/edge-endpoint.ts @@ -1,8 +1,14 @@ +import { getDefaultIsolationScope } from '@sentry/core'; +import * as Sentry from '@sentry/nextjs'; + export const config = { runtime: 'edge', }; export default async function handler() { + Sentry.setTag('my-isolated-tag', true); + Sentry.setTag('my-global-scope-isolated-tag', getDefaultIsolationScope().getScopeData().tags['my-isolated-tag']); // We set this tag to be able to assert that the previously set tag has not leaked into the global isolation scope + return new Response( JSON.stringify({ name: 'Jim Halpert', diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/error-edge-endpoint.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/error-edge-endpoint.ts index 043112494c23..ed1a0acdf412 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/error-edge-endpoint.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/error-edge-endpoint.ts @@ -1,5 +1,10 @@ +import { getDefaultIsolationScope } from '@sentry/core'; +import * as Sentry from '@sentry/nextjs'; + export const config = { runtime: 'edge' }; export default () => { + Sentry.setTag('my-isolated-tag', true); + Sentry.setTag('my-global-scope-isolated-tag', getDefaultIsolationScope().getScopeData().tags['my-isolated-tag']); // We set this tag to be able to assert that the previously set tag has not leaked into the global isolation scope throw new Error('Edge Route Error'); }; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/pages/pages-router/ssr-error-fc.tsx b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/pages/pages-router/ssr-error-fc.tsx index 6342caec47ca..552aeae3b331 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/pages/pages-router/ssr-error-fc.tsx +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/pages/pages-router/ssr-error-fc.tsx @@ -1,4 +1,10 @@ +import { getDefaultIsolationScope } from '@sentry/core'; +import * as Sentry from '@sentry/nextjs'; + export default function Page() { + Sentry.setTag('my-isolated-tag', true); + Sentry.setTag('my-global-scope-isolated-tag', getDefaultIsolationScope().getScopeData().tags['my-isolated-tag']); // We set this tag to be able to assert that the previously set tag has not leaked into the global isolation scope + throw new Error('Pages SSR Error FC'); return
Hello world!
; } diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge-route.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge-route.test.ts index cbe9dcafae71..33bf951337a8 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge-route.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge-route.test.ts @@ -40,6 +40,10 @@ test('Should create a transaction with error status for faulty edge routes', asy expect(edgerouteTransaction.contexts?.trace?.status).toBe('internal_error'); expect(edgerouteTransaction.contexts?.trace?.op).toBe('http.server'); expect(edgerouteTransaction.contexts?.runtime?.name).toBe('vercel-edge'); + + // Assert that isolation scope works properly + expect(edgerouteTransaction.tags?.['my-isolated-tag']).toBe(true); + expect(edgerouteTransaction.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); }); test('Should record exceptions for faulty edge routes', async ({ request }) => { @@ -51,5 +55,9 @@ test('Should record exceptions for faulty edge routes', async ({ request }) => { // Noop }); - expect(await errorEventPromise).toBeDefined(); + const errorEvent = await errorEventPromise; + + // Assert that isolation scope works properly + expect(errorEvent.tags?.['my-isolated-tag']).toBe(true); + expect(errorEvent.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); }); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge.test.ts index 4e69abbdd3e2..f5f3e70c9770 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge.test.ts @@ -8,7 +8,13 @@ test('Should record exceptions for faulty edge server components', async ({ page await page.goto('/edge-server-components/error'); - expect(await errorEventPromise).toBeDefined(); + const errorEvent = await errorEventPromise; + + expect(errorEvent).toBeDefined(); + + // Assert that isolation scope works properly + expect(errorEvent.tags?.['my-isolated-tag']).toBe(true); + expect(errorEvent.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); }); test('Should record transaction for edge server components', async ({ page }) => { @@ -22,4 +28,8 @@ test('Should record transaction for edge server components', async ({ page }) => expect(serverComponentTransaction).toBeDefined(); expect(serverComponentTransaction.request?.headers).toBeDefined(); + + // Assert that isolation scope works properly + expect(serverComponentTransaction.tags?.['my-isolated-tag']).toBe(true); + expect(serverComponentTransaction.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); }); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts index 240e04ebe37f..79b07bd37a15 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts @@ -14,6 +14,10 @@ test('Should create a transaction for middleware', async ({ request }) => { expect(middlewareTransaction.contexts?.trace?.status).toBe('ok'); expect(middlewareTransaction.contexts?.trace?.op).toBe('middleware.nextjs'); expect(middlewareTransaction.contexts?.runtime?.name).toBe('vercel-edge'); + + // Assert that isolation scope works properly + expect(middlewareTransaction.tags?.['my-isolated-tag']).toBe(true); + expect(middlewareTransaction.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); }); test('Should create a transaction with error status for faulty middleware', async ({ request }) => { @@ -43,7 +47,11 @@ test('Records exceptions happening in middleware', async ({ request }) => { // Noop }); - expect(await errorEventPromise).toBeDefined(); + const errorEvent = await errorEventPromise; + + // Assert that isolation scope works properly + expect(errorEvent.tags?.['my-isolated-tag']).toBe(true); + expect(errorEvent.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); }); test('Should trace outgoing fetch requests inside middleware and create breadcrumbs for it', async ({ request }) => { diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/pages-ssr-errors.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/pages-ssr-errors.test.ts index 73f8bd5e31b9..3e6396c4a618 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/pages-ssr-errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/pages-ssr-errors.test.ts @@ -24,7 +24,7 @@ test('Will capture error for SSR rendering error with a connected trace (Functio return errorEvent?.exception?.values?.[0]?.value === 'Pages SSR Error FC'; }); - const serverComponentTransaction = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + const ssrTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { return ( transactionEvent?.transaction === '/pages-router/ssr-error-fc' && (await errorEventPromise).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id @@ -33,6 +33,14 @@ test('Will capture error for SSR rendering error with a connected trace (Functio await page.goto('/pages-router/ssr-error-fc'); - expect(await errorEventPromise).toBeDefined(); - expect(await serverComponentTransaction).toBeDefined(); + const errorEvent = await errorEventPromise; + const ssrTransaction = await ssrTransactionPromise; + + // Assert that isolation scope works properly + expect(errorEvent.tags?.['my-isolated-tag']).toBe(true); + expect(errorEvent.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); + + // TODO(lforst): Reuse SSR request span isolation scope to fix the following two assertions + // expect(ssrTransaction.tags?.['my-isolated-tag']).toBe(true); + // expect(ssrTransaction.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); }); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts index 70f9bb32d3bc..84969d7c84ca 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts @@ -48,6 +48,12 @@ test('Should record exceptions and transactions for faulty route handlers', asyn const routehandlerTransaction = await routehandlerTransactionPromise; const routehandlerError = await errorEventPromise; + // Assert that isolation scope works properly + expect(routehandlerTransaction.tags?.['my-isolated-tag']).toBe(true); + expect(routehandlerTransaction.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); + expect(routehandlerError.tags?.['my-isolated-tag']).toBe(true); + expect(routehandlerError.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); + expect(routehandlerTransaction.contexts?.trace?.status).toBe('unknown_error'); expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server'); @@ -87,6 +93,12 @@ test.describe('Edge runtime', () => { const routehandlerTransaction = await routehandlerTransactionPromise; const routehandlerError = await errorEventPromise; + // Assert that isolation scope works properly + expect(routehandlerTransaction.tags?.['my-isolated-tag']).toBe(true); + expect(routehandlerTransaction.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); + expect(routehandlerError.tags?.['my-isolated-tag']).toBe(true); + expect(routehandlerError.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); + expect(routehandlerTransaction.contexts?.trace?.status).toBe('internal_error'); expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server'); expect(routehandlerTransaction.contexts?.runtime?.name).toBe('vercel-edge'); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts index 0b844a0402b1..00aeae924fcc 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts @@ -1,5 +1,5 @@ import { expect, test } from '@playwright/test'; -import { waitForTransaction } from '@sentry-internal/event-proxy-server'; +import { waitForError, waitForTransaction } from '@sentry-internal/event-proxy-server'; import axios, { AxiosError } from 'axios'; const authToken = process.env.E2E_TEST_AUTH_TOKEN; @@ -81,3 +81,25 @@ test('Should set a "not_found" status on a server component transaction when not expect((await serverComponentTransactionPromise).contexts?.trace?.status).toBe('not_found'); }); + +test('Should capture an error and transaction with correct status for a faulty server component', async ({ page }) => { + const transactionEventPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return transactionEvent?.transaction === 'Page Server Component (/server-component/faulty)'; + }); + + const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'I am a faulty server component'; + }); + + await page.goto('/server-component/faulty'); + + const transactionEvent = await transactionEventPromise; + const errorEvent = await errorEventPromise; + + expect(transactionEvent.contexts?.trace?.status).toBe('internal_error'); + + expect(errorEvent.tags?.['my-isolated-tag']).toBe(true); + expect(errorEvent.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); + expect(transactionEvent.tags?.['my-isolated-tag']).toBe(true); + expect(transactionEvent.tags?.['my-global-scope-isolated-tag']).not.toBeDefined(); +}); From a48cf9bde3912ae4efc1294b456bdc61549e54c3 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 8 Apr 2024 14:55:38 +0000 Subject: [PATCH 3/6] Fix edge api handlers --- .../src/edge/wrapApiHandlerWithSentry.ts | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts index f66a03ee9586..1fd8dc103e19 100644 --- a/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts @@ -1,6 +1,7 @@ -import { getActiveSpan } from '@sentry/core'; +import { getActiveSpan } from '@sentry/nextjs'; import { withEdgeWrapping } from '../common/utils/edgeWrapperUtils'; +import { withIsolationScopeOrReuseFromRootSpan } from '../common/utils/withIsolationScopeOrReuseFromRootSpan'; import type { EdgeRouteHandler } from './types'; /** @@ -16,16 +17,18 @@ export function wrapApiHandlerWithSentry( const activeSpan = getActiveSpan(); - const wrappedHandler = withEdgeWrapping(wrappingTarget, { - spanDescription: - activeSpan || !(req instanceof Request) - ? `handler (${parameterizedRoute})` - : `${req.method} ${parameterizedRoute}`, - spanOp: activeSpan ? 'function' : 'http.server', - mechanismFunctionName: 'wrapApiHandlerWithSentry', - }); + return withIsolationScopeOrReuseFromRootSpan(() => { + const wrappedHandler = withEdgeWrapping(wrappingTarget, { + spanDescription: + activeSpan || !(req instanceof Request) + ? `handler (${parameterizedRoute})` + : `${req.method} ${parameterizedRoute}`, + spanOp: activeSpan ? 'function' : 'http.server', + mechanismFunctionName: 'wrapApiHandlerWithSentry', + }); - return wrappedHandler.apply(thisArg, args); + return wrappedHandler.apply(thisArg, args); + }); }, }); } From 2f326f27a6e4b78f41ba8e4592497a14ee4aaeab Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 8 Apr 2024 15:02:27 +0000 Subject: [PATCH 4/6] whoops --- packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts index 1fd8dc103e19..c87c526e92cb 100644 --- a/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts @@ -1,4 +1,4 @@ -import { getActiveSpan } from '@sentry/nextjs'; +import { getActiveSpan } from '@sentry/core'; import { withEdgeWrapping } from '../common/utils/edgeWrapperUtils'; import { withIsolationScopeOrReuseFromRootSpan } from '../common/utils/withIsolationScopeOrReuseFromRootSpan'; From 33c06ca94f6aabc23f3df09925565f520099a481 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 8 Apr 2024 15:02:43 +0000 Subject: [PATCH 5/6] double whoops --- packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts index c87c526e92cb..717090905f6a 100644 --- a/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts @@ -1,4 +1,4 @@ -import { getActiveSpan } from '@sentry/core'; +import { getActiveSpan } from '@sentry/vercel-edge'; import { withEdgeWrapping } from '../common/utils/edgeWrapperUtils'; import { withIsolationScopeOrReuseFromRootSpan } from '../common/utils/withIsolationScopeOrReuseFromRootSpan'; From cd0f41bddd7048313af522ae5df193003552a003 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 8 Apr 2024 15:15:10 +0000 Subject: [PATCH 6/6] Fix middleware --- .../nextjs/src/common/wrapMiddlewareWithSentry.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts b/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts index 66cbbb046300..25b0e2b6d5d4 100644 --- a/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts +++ b/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts @@ -1,5 +1,6 @@ import type { EdgeRouteHandler } from '../edge/types'; import { withEdgeWrapping } from './utils/edgeWrapperUtils'; +import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; /** * Wraps Next.js middleware with Sentry error and performance instrumentation. @@ -12,11 +13,13 @@ export function wrapMiddlewareWithSentry( ): (...params: Parameters) => Promise> { return new Proxy(middleware, { apply: (wrappingTarget, thisArg, args: Parameters) => { - return withEdgeWrapping(wrappingTarget, { - spanDescription: 'middleware', - spanOp: 'middleware.nextjs', - mechanismFunctionName: 'withSentryMiddleware', - }).apply(thisArg, args); + return withIsolationScopeOrReuseFromRootSpan(() => { + return withEdgeWrapping(wrappingTarget, { + spanDescription: 'middleware', + spanOp: 'middleware.nextjs', + mechanismFunctionName: 'withSentryMiddleware', + }).apply(thisArg, args); + }); }, }); }