ref(nextjs): Fix tests (#14058)

Adds finishing touches to the target branch, unskipping tests ensuring
we are not breaking anything.

dev-packages/e2e-tests/test-applications/nextjs-13/instrumentation.ts

@@ -12,7 +12,6 @@ export function register() {
         // We are doing a lot of events at once in this test app
         bufferSize: 1000,
       },
-      debug: false,
     });
   }
 }

dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/excluded-api-endpoints.test.ts

@@ -1,9 +1,7 @@
 import { expect, test } from '@playwright/test';
 import { waitForTransaction } from '@sentry-internal/test-utils';
 
-// TODO(lforst): This cannot make it into production - Make sure to fix this test
-// The problem is that if we are not applying build time instrumentation Next.js will still emit spans (which is fine, but we need to find a different way of testing that build time instrumentation is successfully disabled - maybe with an attribute or something if build-time instrumentation is applied)
-test.skip('should not automatically create transactions for routes that were excluded from auto wrapping (string)', async ({
+test('should not apply build-time instrumentation for routes that were excluded from auto wrapping (string)', async ({
   request,
 }) => {
   const transactionPromise = waitForTransaction('nextjs-13', async transactionEvent => {
@@ -15,19 +13,13 @@ test.skip('should not automatically create transactions for routes that were exc
 
   expect(await (await request.get(`/api/endpoint-excluded-with-string`)).text()).toBe('{"success":true}');
 
-  let transactionPromiseReceived = false;
-  transactionPromise.then(() => {
-    transactionPromiseReceived = true;
-  });
-
-  await new Promise(resolve => setTimeout(resolve, 5_000));
+  const transaction = await transactionPromise;
 
-  expect(transactionPromiseReceived).toBe(false);
+  expect(transaction.contexts?.trace?.data?.['sentry.origin']).toBeDefined();
+  expect(transaction.contexts?.trace?.data?.['sentry.origin']).not.toBe('auto.http.nextjs'); // This is the origin set by the build time instrumentation
 });
 
-// TODO(lforst): This cannot make it into production - Make sure to fix this test
-// The problem is that if we are not applying build time instrumentation Next.js will still emit spans (which is fine, but we need to find a different way of testing that build time instrumentation is successfully disabled - maybe with an attribute or something if build-time instrumentation is applied)
-test.skip('should not automatically create transactions for routes that were excluded from auto wrapping (regex)', async ({
+test('should not apply build-time instrumentation for routes that were excluded from auto wrapping (regex)', async ({
   request,
 }) => {
   const transactionPromise = waitForTransaction('nextjs-13', async transactionEvent => {
@@ -39,12 +31,8 @@ test.skip('should not automatically create transactions for routes that were exc
 
   expect(await (await request.get(`/api/endpoint-excluded-with-regex`)).text()).toBe('{"success":true}');
 
-  let transactionPromiseReceived = false;
-  transactionPromise.then(() => {
-    transactionPromiseReceived = true;
-  });
-
-  await new Promise(resolve => setTimeout(resolve, 5_000));
+  const transaction = await transactionPromise;
 
-  expect(transactionPromiseReceived).toBe(false);
+  expect(transaction.contexts?.trace?.data?.['sentry.origin']).toBeDefined();
+  expect(transaction.contexts?.trace?.data?.['sentry.origin']).not.toBe('auto.http.nextjs'); // This is the origin set by the build time instrumentation
 });

dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/getServerSideProps.test.ts

@@ -60,11 +60,11 @@ test('Should report an error event for errors thrown in getServerSideProps', asy
         data: {
           'http.response.status_code': 500,
           'sentry.op': 'http.server',
-          'sentry.origin': expect.stringMatching(/^(auto|auto\.http\.otel\.http)$/),
+          'sentry.origin': 'auto',
           'sentry.source': 'route',
         },
         op: 'http.server',
-        origin: expect.stringMatching(/^(auto|auto\.http\.otel\.http)$/),
+        origin: 'auto',
         span_id: expect.any(String),
         status: 'internal_error',
         trace_id: expect.any(String),
@@ -81,8 +81,7 @@ test('Should report an error event for errors thrown in getServerSideProps', asy
     start_timestamp: expect.any(Number),
     timestamp: expect.any(Number),
     transaction: 'GET /[param]/error-getServerSideProps',
-    // TODO: This test fails depending on the next version (next 13: 'custom', next >14: 'route')
-    // transaction_info: { source: 'custom' },
+    transaction_info: { source: 'route' },
     type: 'transaction',
   });
 });
@@ -148,11 +147,11 @@ test('Should report an error event for errors thrown in getServerSideProps in pa
         data: {
           'http.response.status_code': 500,
           'sentry.op': 'http.server',
-          'sentry.origin': expect.stringMatching(/^auto(\.http\.otel\.http)?$/),
+          'sentry.origin': 'auto',
           'sentry.source': 'route',
         },
         op: 'http.server',
-        origin: expect.stringMatching(/^auto(\.http\.otel\.http)?$/),
+        origin: 'auto',
         span_id: expect.any(String),
         status: 'internal_error',
         trace_id: expect.any(String),
@@ -169,8 +168,7 @@ test('Should report an error event for errors thrown in getServerSideProps in pa
     start_timestamp: expect.any(Number),
     timestamp: expect.any(Number),
     transaction: 'GET /[param]/customPageExtension',
-    // TODO: This test fails depending on the next version (next 13: 'custom', next >14: 'route')
-    // transaction_info: { source: 'custom' },
+    transaction_info: { source: 'route' },
     type: 'transaction',
   });
 });

dev-packages/e2e-tests/test-applications/nextjs-app-dir/next-env.d.ts

@@ -3,4 +3,4 @@
 /// <reference types="next/navigation-types/compat/navigation" />
 
 // NOTE: This file should not be edited
-// see https://nextjs.org/docs/app/building-your-application/configuring/typescript for more information.
+// see https://nextjs.org/docs/basic-features/typescript for more information.

dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge-route.test.ts

@@ -25,11 +25,17 @@ test('Should create a transaction for edge routes', async ({ request }) => {
 
 test('Faulty edge routes', async ({ request }) => {
   const edgerouteTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => {
-    return transactionEvent?.transaction === 'GET /api/error-edge-endpoint';
+    return (
+      transactionEvent?.transaction === 'GET /api/error-edge-endpoint' &&
+      transactionEvent.contexts?.runtime?.name === 'vercel-edge'
+    );
   });
 
   const errorEventPromise = waitForError('nextjs-app-dir', errorEvent => {
-    return errorEvent?.exception?.values?.[0]?.value === 'Edge Route Error';
+    return (
+      errorEvent?.exception?.values?.[0]?.value === 'Edge Route Error' &&
+      errorEvent.contexts?.runtime?.name === 'vercel-edge'
+    );
   });
 
   request.get('/api/error-edge-endpoint').catch(() => {
@@ -44,7 +50,6 @@ test('Faulty edge routes', async ({ request }) => {
   test.step('should create transactions with the right fields', () => {
     expect(edgerouteTransaction.contexts?.trace?.status).toBe('unknown_error');
     expect(edgerouteTransaction.contexts?.trace?.op).toBe('http.server');
-    expect(edgerouteTransaction.contexts?.runtime?.name).toBe('vercel-edge');
   });
 
   test.step('should have scope isolation', () => {

dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge.test.ts

@@ -1,6 +1,8 @@
 import { expect, test } from '@playwright/test';
 import { waitForError, waitForTransaction } from '@sentry-internal/test-utils';
 
+const packageJson = require('../package.json');
+
 test('Should record exceptions for faulty edge server components', async ({ page }) => {
   const errorEventPromise = waitForError('nextjs-app-dir', errorEvent => {
     return errorEvent?.exception?.values?.[0]?.value === 'Edge Server Component Error';
@@ -19,8 +21,10 @@ test('Should record exceptions for faulty edge server components', async ({ page
   expect(errorEvent.transaction).toBe(`Page Server Component (/edge-server-components/error)`);
 });
 
-// TODO(lforst): Fix this test
-test.skip('Should record transaction for edge server components', async ({ page }) => {
+test('Should record transaction for edge server components', async ({ page }) => {
+  const nextjsVersion = packageJson.dependencies.next;
+  const nextjsMajor = Number(nextjsVersion.split('.')[0]);
+
   const serverComponentTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => {
     return (
       transactionEvent?.transaction === 'GET /edge-server-components' &&
@@ -33,9 +37,14 @@ test.skip('Should record transaction for edge server components', async ({ page
   const serverComponentTransaction = await serverComponentTransactionPromise;
 
   expect(serverComponentTransaction).toBeDefined();
-  expect(serverComponentTransaction.request?.headers).toBeDefined();
+  expect(serverComponentTransaction.contexts?.trace?.op).toBe('http.server');
 
-  // Assert that isolation scope works properly
-  expect(serverComponentTransaction.tags?.['my-isolated-tag']).toBe(true);
-  expect(serverComponentTransaction.tags?.['my-global-scope-isolated-tag']).not.toBeDefined();
+  // For some reason headers aren't picked up on Next.js 13 - also causing scope isolation to be broken
+  if (nextjsMajor >= 14) {
+    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();
+  }
 });

dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts

@@ -63,11 +63,13 @@ test('Should record exceptions and transactions for faulty route handlers', asyn
   expect(routehandlerError.transaction).toBe('PUT /route-handlers/[param]/error');
 });
 
-// TODO(lforst): This cannot make it into production - Make sure to fix this test
-test.describe.skip('Edge runtime', () => {
+test.describe('Edge runtime', () => {
   test('should create a transaction for route handlers', async ({ request }) => {
     const routehandlerTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => {
-      return transactionEvent?.transaction === 'PATCH /route-handlers/[param]/edge';
+      return (
+        transactionEvent?.transaction === 'PATCH /route-handlers/[param]/edge' &&
+        transactionEvent.contexts?.runtime?.name === 'vercel-edge'
+      );
     });
 
     const response = await request.patch('/route-handlers/bar/edge');
@@ -81,11 +83,17 @@ test.describe.skip('Edge runtime', () => {
 
   test('should record exceptions and transactions for faulty route handlers', async ({ request }) => {
     const errorEventPromise = waitForError('nextjs-app-dir', errorEvent => {
-      return errorEvent?.exception?.values?.[0]?.value === 'route-handler-edge-error';
+      return (
+        errorEvent?.exception?.values?.[0]?.value === 'route-handler-edge-error' &&
+        errorEvent.contexts?.runtime?.name === 'vercel-edge'
+      );
     });
 
     const routehandlerTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => {
-      return transactionEvent?.transaction === 'DELETE /route-handlers/[param]/edge';
+      return (
+        transactionEvent?.transaction === 'DELETE /route-handlers/[param]/edge' &&
+        transactionEvent.contexts?.runtime?.name === 'vercel-edge'
+      );
     });
 
     await request.delete('/route-handlers/baz/edge').catch(() => {
@@ -101,12 +109,10 @@ test.describe.skip('Edge runtime', () => {
     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?.status).toBe('unknown_error');
     expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server');
-    expect(routehandlerTransaction.contexts?.runtime?.name).toBe('vercel-edge');
 
     expect(routehandlerError.exception?.values?.[0].value).toBe('route-handler-edge-error');
-    expect(routehandlerError.contexts?.runtime?.name).toBe('vercel-edge');
 
     expect(routehandlerError.transaction).toBe('DELETE /route-handlers/[param]/edge');
   });

dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts

@@ -16,7 +16,7 @@ test('Sends a transaction for a request to app router', async ({ page }) => {
   expect(transactionEvent.contexts?.trace).toEqual({
     data: expect.objectContaining({
       'sentry.op': 'http.server',
-      'sentry.origin': expect.stringMatching(/^(auto|auto\.http\.otel\.http)$/),
+      'sentry.origin': 'auto',
       'sentry.sample_rate': 1,
       'sentry.source': 'route',
       'http.method': 'GET',
@@ -27,7 +27,7 @@ test('Sends a transaction for a request to app router', async ({ page }) => {
       'otel.kind': 'SERVER',
     }),
     op: 'http.server',
-    origin: expect.stringMatching(/^(auto|auto\.http\.otel\.http)$/),
+    origin: 'auto',
     span_id: expect.any(String),
     status: 'ok',
     trace_id: expect.any(String),

packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts

@@ -1,11 +1,14 @@
 import {
+  SEMANTIC_ATTRIBUTE_SENTRY_OP,
+  SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
   Scope,
   captureException,
   getActiveSpan,
   getCapturedScopesOnSpan,
   getRootSpan,
   handleCallbackErrors,
   setCapturedScopesOnSpan,
+  setHttpStatus,
   withIsolationScope,
   withScope,
 } from '@sentry/core';
@@ -14,7 +17,7 @@ import type { RouteHandlerContext } from './types';
 
 import { propagationContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils';
 
-import { isRedirectNavigationError } from './nextNavigationErrorUtils';
+import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils';
 import { commonObjectToIsolationScope, commonObjectToPropagationContext } from './utils/tracingUtils';
 
 /**
@@ -49,22 +52,36 @@ export function wrapRouteHandlerWithSentry<F extends (...args: any[]) => any>(
       const propagationContext = commonObjectToPropagationContext(headers, incomingPropagationContext);
 
       const activeSpan = getActiveSpan();
-      if (activeSpan) {
-        const rootSpan = getRootSpan(activeSpan);
+      const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined;
+      if (rootSpan) {
         const { scope } = getCapturedScopesOnSpan(rootSpan);
         setCapturedScopesOnSpan(rootSpan, scope ?? new Scope(), isolationScope);
+
+        if (process.env.NEXT_RUNTIME === 'edge') {
+          rootSpan.updateName(`${method} ${parameterizedRoute}`);
+          rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
+          rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'http.server');
+        }
       }
 
       return withIsolationScope(isolationScope, () => {
-        return withScope(scope => {
+        return withScope(async scope => {
           scope.setTransactionName(`${method} ${parameterizedRoute}`);
           scope.setPropagationContext(propagationContext);
-          return handleCallbackErrors(
+
+          const response: Response = await handleCallbackErrors(
             () => originalFunction.apply(thisArg, args),
             error => {
               // Next.js throws errors when calling `redirect()`. We don't wanna report these.
               if (isRedirectNavigationError(error)) {
                 // Don't do anything
+              } else if (isNotFoundNavigationError(error)) {
+                if (activeSpan) {
+                  setHttpStatus(activeSpan, 404);
+                }
+                if (rootSpan) {
+                  setHttpStatus(rootSpan, 404);
+                }
               } else {
                 captureException(error, {
                   mechanism: {
@@ -74,6 +91,21 @@ export function wrapRouteHandlerWithSentry<F extends (...args: any[]) => any>(
               }
             },
           );
+
+          try {
+            if (response.status) {
+              if (activeSpan) {
+                setHttpStatus(activeSpan, response.status);
+              }
+              if (rootSpan) {
+                setHttpStatus(rootSpan, response.status);
+              }
+            }
+          } catch {
+            // best effort - response may be undefined?
+          }
+
+          return response;
         });
       });
     },