avoid double request isolation, add tests

Author: Lms24

packages/sveltekit/src/server-common/handle.ts

@@ -73,6 +73,22 @@ export function addSentryCodeToPage(options: { injectFetchProxyScript: boolean }
   };
 }
 
+/**
+ * We only need to inject the fetch proxy script for SvelteKit versions < 2.16.0.
+ * Exported only for testing.
+ */
+export function isFetchProxyRequired(version: string): boolean {
+  try {
+    const [major, minor] = version.trim().replace(/-.*/, '').split('.').map(Number);
+    if (major != null && minor != null && (major > 2 || (major === 2 && minor >= 16))) {
+      return false;
+    }
+  } catch {
+    // ignore
+  }
+  return true;
+}
+
 async function instrumentHandle(
   { event, resolve }: Parameters<Handle>[0],
   options: SentryHandleOptions,
@@ -134,22 +150,6 @@ async function instrumentHandle(
   }
 }
 
-/**
- * We only need to inject the fetch proxy script for SvelteKit versions < 2.16.0.
- * Exported only for testing.
- */
-export function isFetchProxyRequired(version: string): boolean {
-  try {
-    const [major, minor] = version.trim().replace(/-.*/, '').split('.').map(Number);
-    if (major != null && minor != null && (major > 2 || (major === 2 && minor >= 16))) {
-      return false;
-    }
-  } catch {
-    // ignore
-  }
-  return true;
-}
-
 /**
  * A SvelteKit handle function that wraps the request for Sentry error and
  * performance monitoring.
@@ -166,10 +166,10 @@ export function isFetchProxyRequired(version: string): boolean {
  * ```
  */
 export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle {
+  const { handleUnknownRoutes, ...rest } = handlerOptions ?? {};
   const options = {
-    handleUnknownRoutes: false,
-    injectFetchProxyScript: true,
-    ...handlerOptions,
+    handleUnknownRoutes: handleUnknownRoutes ?? false,
+    ...rest,
   };
 
   const sentryRequestHandler: Handle = input => {
@@ -185,7 +185,11 @@ export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle {
     // we create a new execution context.
     const isSubRequest = typeof input.event.isSubRequest === 'boolean' ? input.event.isSubRequest : !!getActiveSpan();
 
-    if (isSubRequest) {
+    // Escape hatch to suppress request isolation and trace continuation (see initCloudflareSentryHandle)
+    const skipIsolation =
+      '_sentrySkipRequestIsolation' in input.event.locals && input.event.locals._sentrySkipRequestIsolation;
+
+    if (isSubRequest || skipIsolation) {
       return instrumentHandle(input, options);
     }
 

packages/sveltekit/src/server/handle.ts

@@ -149,7 +149,10 @@ export function isFetchProxyRequired(version: string): boolean {
   return true;
 }
 
-/** Documented in `worker/handle.ts` */
+/**
+ * actual implementation in ../worker/handle.ts
+ * @return no-op handler when initCLoudflareSentryHandle is called via node/server entry point
+ */
 export function initCloudflareSentryHandle(_options: unknown): Handle {
   return ({ event, resolve }) => resolve(event);
 }

packages/sveltekit/src/worker/handle.ts

@@ -3,12 +3,13 @@ import { getDefaultIntegrations as getDefaultCloudflareIntegrations } from '@sen
 import type { Handle } from '@sveltejs/kit';
 
 import { rewriteFramesIntegration } from '../server-common/rewriteFramesIntegration';
+import { addNonEnumerableProperty } from '@sentry/core';
 
 /** Initializes Sentry SvelteKit Cloudflare SDK
  *  This should be before the sentryHandle() call.
  *
- *  In Node.js, this is a stub that does nothing.
- * */
+ *  In the Node export, this is a stub that does nothing.
+ */
 export function initCloudflareSentryHandle(options: CloudflareOptions): Handle {
   const opts: CloudflareOptions = {
     defaultIntegrations: [...getDefaultCloudflareIntegrations(options), rewriteFramesIntegration()],
@@ -17,17 +18,24 @@ export function initCloudflareSentryHandle(options: CloudflareOptions): Handle {
 
   const handleInitSentry: Handle = ({ event, resolve }) => {
     // if event.platform exists (should be there in a cloudflare worker), then do the cloudflare sentry init
-    return event.platform
-      ? wrapRequestHandler(
-          {
-            options: opts,
-            request: event.request,
-            // @ts-expect-error This will exist in Cloudflare
-            context: event.platform.context,
-          },
-          () => resolve(event),
-        )
-      : resolve(event);
+    if (event.platform) {
+      // This is an optional local that the `sentryHandle` handler checks for to avoid double isolation
+      // In Cloudflare the `wrapRequestHandler` function already takes care of
+      // - request isolation
+      // - trace continuation
+      // -setting the request onto the scope
+      addNonEnumerableProperty(event.locals, '_sentrySkipRequestIsolation', true);
+      return wrapRequestHandler(
+        {
+          options: opts,
+          request: event.request,
+          // @ts-expect-error This will exist in Cloudflare
+          context: event.platform.context,
+        },
+        () => resolve(event),
+      );
+    }
+    return resolve(event);
   };
 
   return handleInitSentry;

packages/sveltekit/test/server/handle.test.ts

@@ -99,6 +99,7 @@ beforeEach(() => {
   client.init();
 
   mockCaptureException.mockClear();
+  vi.clearAllMocks();
 });
 
 describe('sentryHandle', () => {
@@ -367,6 +368,23 @@ describe('sentryHandle', () => {
 
       expect(_span!).toBeDefined();
     });
+
+    it("doesn't create an isolation scope when the `_sentrySkipRequestIsolation` local is set", async () => {
+      const withIsolationScopeSpy = vi.spyOn(SentryCore, 'withIsolationScope');
+      const continueTraceSpy = vi.spyOn(SentryCore, 'continueTrace');
+
+      try {
+        await sentryHandle({ handleUnknownRoutes: true })({
+          event: { ...mockEvent({ route: undefined }), locals: { _sentrySkipRequestIsolation: true } },
+          resolve: resolve(type, isError),
+        });
+      } catch {
+        //
+      }
+
+      expect(withIsolationScopeSpy).not.toHaveBeenCalled();
+      expect(continueTraceSpy).not.toHaveBeenCalled();
+    });
   });
 });