review comments

Author: s1gr1d

packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts

@@ -3,10 +3,10 @@ import {
   addTracingExtensions,
   captureException,
   getActiveSpan,
-  getIsolationScope,
   getRootSpan,
   handleCallbackErrors,
   setHttpStatus,
+  withIsolationScope,
 } from '@sentry/core';
 import { winterCGHeadersToDict } from '@sentry/utils';
 import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils';
@@ -29,50 +29,52 @@ export function wrapRouteHandlerWithSentry<F extends (...args: any[]) => any>(
 
   return new Proxy(routeHandler, {
     apply: async (originalFunction, thisArg, args) => {
-      getIsolationScope().setSDKProcessingMetadata({
-        request: {
-          headers: headers ? winterCGHeadersToDict(headers) : undefined,
-        },
-      });
-
-      try {
-        const activeSpan = getActiveSpan();
-        const rootSpan = activeSpan && getRootSpan(activeSpan);
-
-        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) && rootSpan) {
-              rootSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' });
-            } else {
-              captureException(error, {
-                mechanism: {
-                  handled: false,
-                },
-              });
-            }
+      return withIsolationScope(async isolationScope => {
+        isolationScope.setSDKProcessingMetadata({
+          request: {
+            headers: headers ? winterCGHeadersToDict(headers) : undefined,
           },
-        );
+        });
 
         try {
-          if (rootSpan && response.status) {
-            setHttpStatus(rootSpan, response.status);
+          const activeSpan = getActiveSpan();
+          const rootSpan = activeSpan && getRootSpan(activeSpan);
+
+          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) && rootSpan) {
+                rootSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' });
+              } else {
+                captureException(error, {
+                  mechanism: {
+                    handled: false,
+                  },
+                });
+              }
+            },
+          );
+
+          try {
+            if (rootSpan && response.status) {
+              setHttpStatus(rootSpan, response.status);
+            }
+          } catch {
+            // best effort - response may be undefined?
           }
-        } catch {
-          // best effort - response may be undefined?
-        }
 
-        return response;
-      } finally {
-        if (!platformSupportsStreaming() || process.env.NEXT_RUNTIME === 'edge') {
-          // 1. Edge transport requires manual flushing
-          // 2. Lambdas require manual flushing to prevent execution freeze before the event is sent
-          await flushQueue();
+          return response;
+        } finally {
+          if (!platformSupportsStreaming() || process.env.NEXT_RUNTIME === 'edge') {
+            // 1. Edge transport requires manual flushing
+            // 2. Lambdas require manual flushing to prevent execution freeze before the event is sent
+            await flushQueue();
+          }
         }
-      }
+      });
     },
   });
 }

packages/nextjs/src/server/index.ts

@@ -75,8 +75,9 @@ export function init(options: NodeOptions): void {
     ...getDefaultIntegrations(options).filter(
       integration =>
         integration.name !== 'OnUncaughtException' &&
-        // Next.js comes with its own Node-Fetch and Http instrumentation, so we shouldn't add ours on-top
+        // Next.js comes with its own Node-Fetch instrumentation, so we shouldn't add ours on-top
         integration.name !== 'NodeFetch' &&
+        // Next.js comes with its own Http instrumentation for OTel which lead to double spans for route handler requests
         integration.name !== 'Http',
     ),
     onUncaughtExceptionIntegration(),