test(e2e): Assert correct isolation scopes in Next.js (#11480)

Luca Forstner · web-flow · commit da758e31c990 · 2024-04-09T09:13:28.000+02:00
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
@@ -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
@@ -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
@@ -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
@@ -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 <h1>Hello world!</h1>;
 }
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
@@ -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<Response> {
+  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
@@ -1,3 +1,9 @@
+import { getDefaultIsolationScope } from '@sentry/core';
+import * as Sentry from '@sentry/nextjs';
+
 export async function PUT(): Promise<Response> {
+  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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 <div>Hello world!</div>;
 }
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,105 @@
+import { expect, test } from '@playwright/test';
+import { waitForError, 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');
+});
+
+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();
+});
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
diff --git a/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts b/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts
diff --git a/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+import { getDefaultIsolationScope } from '@sentry/core';`
	`2`	`+import * as Sentry from '@sentry/nextjs';`
`1`	`3`	`import { NextResponse } from 'next/server';`
`2`	`4`
`3`	`5`	`export const runtime = 'edge';`
`@@ -7,5 +9,8 @@ export async function PATCH() {`
`7`	`9`	`}`
`8`	`10`
`9`	`11`	`export async function DELETE(): Promise<Response> {`
	`12`	`+ Sentry.setTag('my-isolated-tag', true);`
	`13`	`+ 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`
	`14`	`+`
`10`	`15`	`throw new Error('route-handler-edge-error');`
`11`	`16`	`}`