ref(sveltekit): Split up universal and server load wrappers (#7652)

 Split up the previously generic `handleLoadWithSentry` wrapper function into two functions on the server side:

* `handleLoadWithSentry` to wrap universal load on server and client
* `handleServerLoadWithSentry` to wrap server load on the server (duh)

It further adjusts types on the client as well as on the server side to return a generic. The very liberal typing of the functions' generic is on purpose because any type that's more narrowed down (e.g. `T extends Load`) would create a type error with the users' actual load types. These types (e.g. `PageData` or `ServerLayoutData`) are generated at build time per route/page. Hence, we cannot access them.

Author: Lms24

packages/sveltekit/src/client/load.ts

@@ -1,7 +1,7 @@
 import { trace } from '@sentry/core';
 import { captureException } from '@sentry/svelte';
 import { addExceptionMechanism, objectify } from '@sentry/utils';
-import type { Load } from '@sveltejs/kit';
+import type { LoadEvent } from '@sveltejs/kit';
 
 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
@@ -27,14 +27,18 @@ function sendErrorToSentry(e: unknown): unknown {
 }
 
 /**
- * Wrap load function with Sentry
- *
- * @param origLoad SvelteKit user defined load function
+ * @inheritdoc
  */
-export function wrapLoadWithSentry(origLoad: Load): Load {
+// The liberal generic typing of `T` is necessary because we cannot let T extend `Load`.
+// This function needs to tell TS that it returns exactly the type that it was called with
+// because SvelteKit generates the narrowed down `PageLoad` or `LayoutLoad` types
+// at build time for every route.
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+export function wrapLoadWithSentry<T extends (...args: any) => any>(origLoad: T): T {
   return new Proxy(origLoad, {
-    apply: (wrappingTarget, thisArg, args: Parameters<Load>) => {
-      const [event] = args;
+    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 routeId = event.route.id;
       return trace(

packages/sveltekit/src/index.types.ts

@@ -9,7 +9,7 @@ export * from './server';
 
 import type { Integration, Options, StackParser } from '@sentry/types';
 // eslint-disable-next-line import/no-unresolved
-import type { HandleClientError, HandleServerError, Load, ServerLoad } from '@sveltejs/kit';
+import type { HandleClientError, HandleServerError } from '@sveltejs/kit';
 
 import type * as clientSdk from './client';
 import type * as serverSdk from './server';
@@ -21,7 +21,25 @@ export declare function handleErrorWithSentry<T extends HandleClientError | Hand
   handleError?: T,
 ): ReturnType<T>;
 
-export declare function wrapLoadWithSentry<S extends ServerLoad | Load>(origLoad: S): S;
+/**
+ * Wrap a universal load function (e.g. +page.js or +layout.js) with Sentry functionality
+ *
+ * Usage:
+ *
+ * ```js
+ * // +page.js
+ *
+ * import { wrapLoadWithSentry }
+ *
+ * export const load = wrapLoadWithSentry((event) => {
+ *   // your load code
+ * });
+ * ```
+ *
+ * @param origLoad SvelteKit user defined universal `load` function
+ */
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+export declare function wrapLoadWithSentry<T extends (...args: any) => any>(origLoad: T): T;
 
 // We export a merged Integrations object so that users can (at least typing-wise) use all integrations everywhere.
 export declare const Integrations: typeof clientSdk.Integrations & typeof serverSdk.Integrations;

packages/sveltekit/src/server/handle.ts

@@ -2,16 +2,12 @@
 import type { Span } from '@sentry/core';
 import { getActiveTransaction, getCurrentHub, trace } from '@sentry/core';
 import { captureException } from '@sentry/node';
-import {
-  addExceptionMechanism,
-  baggageHeaderToDynamicSamplingContext,
-  dynamicSamplingContextToSentryBaggageHeader,
-  extractTraceparentData,
-  objectify,
-} from '@sentry/utils';
+import { addExceptionMechanism, dynamicSamplingContextToSentryBaggageHeader, objectify } from '@sentry/utils';
 import type { Handle, ResolveOptions } from '@sveltejs/kit';
 import * as domain from 'domain';
 
+import { getTracePropagationData } from './utils';
+
 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.
@@ -77,10 +73,7 @@ export const sentryHandle: Handle = input => {
 };
 
 function instrumentHandle({ event, resolve }: Parameters<Handle>[0]): ReturnType<Handle> {
-  const sentryTraceHeader = event.request.headers.get('sentry-trace');
-  const baggageHeader = event.request.headers.get('baggage');
-  const traceparentData = sentryTraceHeader ? extractTraceparentData(sentryTraceHeader) : undefined;
-  const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggageHeader);
+  const { traceparentData, dynamicSamplingContext } = getTracePropagationData(event);
 
   return trace(
     {

packages/sveltekit/src/server/index.ts

@@ -52,5 +52,5 @@ export * from '@sentry/node';
 // SvelteKit SDK exports:
 export { init } from './sdk';
 export { handleErrorWithSentry } from './handleError';
-export { wrapLoadWithSentry } from './load';
+export { wrapLoadWithSentry, wrapServerLoadWithSentry } from './load';
 export { sentryHandle } from './handle';

packages/sveltekit/src/server/load.ts

@@ -1,14 +1,11 @@
 /* eslint-disable @sentry-internal/sdk/no-optional-chaining */
 import { trace } from '@sentry/core';
 import { captureException } from '@sentry/node';
-import type { DynamicSamplingContext, TraceparentData, TransactionContext } from '@sentry/types';
-import {
-  addExceptionMechanism,
-  baggageHeaderToDynamicSamplingContext,
-  extractTraceparentData,
-  objectify,
-} from '@sentry/utils';
-import type { HttpError, Load, LoadEvent, ServerLoad, ServerLoadEvent } from '@sveltejs/kit';
+import type { TransactionContext } from '@sentry/types';
+import { addExceptionMechanism, objectify } from '@sentry/utils';
+import type { HttpError, LoadEvent, ServerLoadEvent } from '@sveltejs/kit';
+
+import { getTracePropagationData } from './utils';
 
 function isHttpError(err: unknown): err is HttpError {
   return typeof err === 'object' && err !== null && 'status' in err && 'body' in err;
@@ -45,58 +42,77 @@ function sendErrorToSentry(e: unknown): unknown {
 }
 
 /**
- * Wrap load function with Sentry
- *
- * @param origLoad SvelteKit user defined load function
+ * @inheritdoc
  */
-export function wrapLoadWithSentry<T extends ServerLoad | Load>(origLoad: T): T {
+// The liberal generic typing of `T` is necessary because we cannot let T extend `Load`.
+// This function needs to tell TS that it returns exactly the type that it was called with
+// because SvelteKit generates the narrowed down `PageLoad` or `LayoutLoad` types
+// at build time for every route.
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+export function wrapLoadWithSentry<T extends (...args: any) => any>(origLoad: T): T {
   return new Proxy(origLoad, {
-    apply: (wrappingTarget, thisArg, args: Parameters<ServerLoad | Load>) => {
-      const [event] = args;
+    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 routeId = event.route && event.route.id;
 
-      const { traceparentData, dynamicSamplingContext } = getTracePropagationData(event);
-
       const traceLoadContext: TransactionContext = {
-        op: `function.sveltekit${isServerOnlyLoad(event) ? '.server' : ''}.load`,
+        op: 'function.sveltekit.load',
         name: routeId ? routeId : event.url.pathname,
         status: 'ok',
         metadata: {
           source: routeId ? 'route' : 'url',
-          dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
         },
-        ...traceparentData,
       };
 
       return trace(traceLoadContext, () => wrappingTarget.apply(thisArg, args), sendErrorToSentry);
     },
   });
 }
 
-function getTracePropagationData(event: ServerLoadEvent | LoadEvent): {
-  traceparentData?: TraceparentData;
-  dynamicSamplingContext?: Partial<DynamicSamplingContext>;
-} {
-  if (!isServerOnlyLoad(event)) {
-    return {};
-  }
-
-  const sentryTraceHeader = event.request.headers.get('sentry-trace');
-  const baggageHeader = event.request.headers.get('baggage');
-  const traceparentData = sentryTraceHeader ? extractTraceparentData(sentryTraceHeader) : undefined;
-  const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggageHeader);
-
-  return { traceparentData, dynamicSamplingContext };
-}
-
 /**
- * Our server-side wrapLoadWithSentry can be used to wrap two different kinds of `load` functions:
- *  - load functions from `+(page|layout).ts`: These can be called both on client and on server
- *  - load functions from `+(page|layout).server.ts`: These are only called on the server
+ * Wrap a server-only load function (e.g. +page.server.js or +layout.server.js) with Sentry functionality
+ *
+ * Usage:
+ *
+ * ```js
+ * // +page.serverjs
  *
- * In both cases, load events look differently. We can distinguish them by checking if the
- * event has a `request` field (which only the server-exclusive load event has).
+ * import { wrapServerLoadWithSentry }
+ *
+ * export const load = wrapServerLoadWithSentry((event) => {
+ *   // your load code
+ * });
+ * ```
+ *
+ * @param origServerLoad SvelteKit user defined server-only load function
  */
-function isServerOnlyLoad(event: ServerLoadEvent | LoadEvent): event is ServerLoadEvent {
-  return 'request' in event;
+// The liberal generic typing of `T` is necessary because we cannot let T extend `ServerLoad`.
+// This function needs to tell TS that it returns exactly the type that it was called with
+// because SvelteKit generates the narrowed down `PageServerLoad` or `LayoutServerLoad` types
+// at build time for every route.
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+export function wrapServerLoadWithSentry<T extends (...args: any) => any>(origServerLoad: T): T {
+  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;
+      const routeId = event.route && event.route.id;
+
+      const { dynamicSamplingContext, traceparentData } = getTracePropagationData(event);
+
+      const traceLoadContext: TransactionContext = {
+        op: 'function.sveltekit.server.load',
+        name: routeId ? routeId : event.url.pathname,
+        status: 'ok',
+        metadata: {
+          source: routeId ? 'route' : 'url',
+          dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
+        },
+        ...traceparentData,
+      };
+
+      return trace(traceLoadContext, () => wrappingTarget.apply(thisArg, args), sendErrorToSentry);
+    },
+  });
 }

packages/sveltekit/src/server/utils.ts

@@ -0,0 +1,19 @@
+import type { DynamicSamplingContext, TraceparentData } from '@sentry/types';
+import { baggageHeaderToDynamicSamplingContext, extractTraceparentData } from '@sentry/utils';
+import type { RequestEvent } from '@sveltejs/kit';
+
+/**
+ * Takes a request event and extracts traceparent and DSC data
+ * from the `sentry-trace` and `baggage` DSC headers.
+ */
+export function getTracePropagationData(event: RequestEvent): {
+  traceparentData?: TraceparentData;
+  dynamicSamplingContext?: Partial<DynamicSamplingContext>;
+} {
+  const sentryTraceHeader = event.request.headers.get('sentry-trace');
+  const baggageHeader = event.request.headers.get('baggage');
+  const traceparentData = sentryTraceHeader ? extractTraceparentData(sentryTraceHeader) : undefined;
+  const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggageHeader);
+
+  return { traceparentData, dynamicSamplingContext };
+}