fix(sveltekit): Avoid double-wrapping load functions (#8094)

Applying the `wrap(server)?LoadWithSentry` wrappers multiple times shouldn't lead to double wrapping but instead we should detect if we already wrapped a load function and no-op in this case. This patch adds a flag to the respective load events to detect double wrapping.

Author: Lms24

packages/sveltekit/src/client/load.ts

@@ -6,6 +6,7 @@ import { captureException } from '@sentry/svelte';
 import type { ClientOptions, SanitizedRequestData } from '@sentry/types';
 import {
   addExceptionMechanism,
+  addNonEnumerableProperty,
   getSanitizedUrlString,
   objectify,
   parseFetchArgs,
@@ -14,8 +15,11 @@ import {
 } from '@sentry/utils';
 import type { LoadEvent } from '@sveltejs/kit';
 
+import type { SentryWrappedFlag } from '../common/utils';
 import { isRedirect } from '../common/utils';
 
+type PatchedLoadEvent = LoadEvent & Partial<SentryWrappedFlag>;
+
 function sendErrorToSentry(e: unknown): unknown {
   // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can
   // store a seen flag on it.
@@ -66,13 +70,20 @@ export function wrapLoadWithSentry<T extends (...args: any) => any>(origLoad: T)
   return new Proxy(origLoad, {
     apply: (wrappingTarget, thisArg, args: Parameters<T>) => {
       // Type casting here because `T` cannot extend `Load` (see comment above function signature)
-      const event = args[0] as LoadEvent;
+      const event = args[0] as PatchedLoadEvent;
 
-      const patchedEvent = {
+      // Check if already wrapped
+      if (event.__sentry_wrapped__) {
+        return wrappingTarget.apply(thisArg, args);
+      }
+
+      const patchedEvent: PatchedLoadEvent = {
         ...event,
         fetch: instrumentSvelteKitFetch(event.fetch),
       };
 
+      addNonEnumerableProperty(patchedEvent as unknown as Record<string, unknown>, '__sentry_wrapped__', true);
+
       const routeId = event.route.id;
       return trace(
         {

packages/sveltekit/src/common/utils.ts

@@ -1,5 +1,13 @@
 import type { Redirect } from '@sveltejs/kit';
 
+export type SentryWrappedFlag = {
+  /**
+   * If this flag is set, we know that the load event was already wrapped once
+   * and we shouldn't wrap it again.
+   */
+  __sentry_wrapped__?: true;
+};
+
 /**
  * Determines if a thrown "error" is a Redirect object which SvelteKit users can throw to redirect to another route
  * see: https://kit.svelte.dev/docs/modules#sveltejs-kit-redirect

packages/sveltekit/src/server/load.ts

@@ -2,12 +2,16 @@
 import { trace } from '@sentry/core';
 import { captureException } from '@sentry/node';
 import type { TransactionContext } from '@sentry/types';
-import { addExceptionMechanism, objectify } from '@sentry/utils';
+import { addExceptionMechanism, addNonEnumerableProperty, objectify } from '@sentry/utils';
 import type { HttpError, LoadEvent, ServerLoadEvent } from '@sveltejs/kit';
 
+import type { SentryWrappedFlag } from '../common/utils';
 import { isRedirect } from '../common/utils';
 import { getTracePropagationData } from './utils';
 
+type PatchedLoadEvent = LoadEvent & SentryWrappedFlag;
+type PatchedServerLoadEvent = ServerLoadEvent & SentryWrappedFlag;
+
 function isHttpError(err: unknown): err is HttpError {
   return typeof err === 'object' && err !== null && 'status' in err && 'body' in err;
 }
@@ -59,7 +63,15 @@ export function wrapLoadWithSentry<T extends (...args: any) => any>(origLoad: T)
   return new Proxy(origLoad, {
     apply: (wrappingTarget, thisArg, args: Parameters<T>) => {
       // Type casting here because `T` cannot extend `Load` (see comment above function signature)
-      const event = args[0] as LoadEvent;
+      // Also, this event possibly already has a sentry wrapped flag attached
+      const event = args[0] as PatchedLoadEvent;
+
+      if (event.__sentry_wrapped__) {
+        return wrappingTarget.apply(thisArg, args);
+      }
+
+      addNonEnumerableProperty(event as unknown as Record<string, unknown>, '__sentry_wrapped__', true);
+
       const routeId = event.route && event.route.id;
 
       const traceLoadContext: TransactionContext = {
@@ -102,7 +114,15 @@ export function wrapServerLoadWithSentry<T extends (...args: any) => any>(origSe
   return new Proxy(origServerLoad, {
     apply: (wrappingTarget, thisArg, args: Parameters<T>) => {
       // Type casting here because `T` cannot extend `ServerLoad` (see comment above function signature)
-      const event = args[0] as ServerLoadEvent;
+      // Also, this event possibly already has a sentry wrapped flag attached
+      const event = args[0] as PatchedServerLoadEvent;
+
+      if (event.__sentry_wrapped__) {
+        return wrappingTarget.apply(thisArg, args);
+      }
+
+      addNonEnumerableProperty(event as unknown as Record<string, unknown>, '__sentry_wrapped__', true);
+
       const routeId = event.route && event.route.id;
 
       const { dynamicSamplingContext, traceparentData } = getTracePropagationData(event);

packages/sveltekit/test/client/load.test.ts

@@ -450,4 +450,17 @@ describe('wrapLoadWithSentry', () => {
       { handled: false, type: 'sveltekit', data: { function: 'load' } },
     );
   });
+
+  it("doesn't wrap load more than once if the wrapper was applied multiple times", async () => {
+    async function load({ params }: Parameters<Load>[0]): Promise<ReturnType<Load>> {
+      return {
+        post: params.id,
+      };
+    }
+
+    const wrappedLoad = wrapLoadWithSentry(wrapLoadWithSentry(load));
+    await wrappedLoad(MOCK_LOAD_ARGS);
+
+    expect(mockTrace).toHaveBeenCalledTimes(1);
+  });
 });