From 262ff629e70fc7edc164649854bb731b1739832e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 29 Mar 2023 15:51:51 +0200 Subject: [PATCH] ref(sveltekit): Split up universal and server page load wrappers --- packages/sveltekit/src/client/load.ts | 29 ++++++++--- packages/sveltekit/src/index.types.ts | 22 +++++++- packages/sveltekit/src/server/index.ts | 2 +- packages/sveltekit/src/server/load.ts | 48 ++++++++++------- packages/sveltekit/src/server/utils.ts | 19 +++++++ packages/sveltekit/test/server/utils.test.ts | 55 ++++++++++++++++++++ 6 files changed, 148 insertions(+), 27 deletions(-) create mode 100644 packages/sveltekit/src/server/utils.ts create mode 100644 packages/sveltekit/test/server/utils.test.ts diff --git a/packages/sveltekit/src/client/load.ts b/packages/sveltekit/src/client/load.ts index 43696ebc54d0..1941b5a5d050 100644 --- a/packages/sveltekit/src/client/load.ts +++ b/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,15 +27,32 @@ function sendErrorToSentry(e: unknown): unknown { } /** - * Wrap load function with Sentry - * TODO: usage + * Wrap a universal load function (e.g. +page.js or +layout.js) with Sentry functionality * - * @param origLoad SvelteKit user defined load function + * Usage: + * + * ```js + * // +page.js + * + * import { wrapLoadWithSentry } + * + * export const load = wrapLoadWithSentry((event) => { + * // your load code + * }); + * ``` + * + * @param origLoad SvelteKit user defined universal `load` function */ -export function wrapLoadWithSentry(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 any>(origLoad: T): T { return new Proxy(origLoad, { apply: (wrappingTarget, thisArg, args: Parameters) => { - const [event] = args; + // 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( diff --git a/packages/sveltekit/src/index.types.ts b/packages/sveltekit/src/index.types.ts index 34ec54ddd5d9..c29315bc0181 100644 --- a/packages/sveltekit/src/index.types.ts +++ b/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 } 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; -export declare function wrapLoadWithSentry(origLoad: T): T; +/** + * 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 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; diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index 9109f29499d4..1307a22e2846 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -2,5 +2,5 @@ export * from '@sentry/node'; export { init } from './sdk'; export { handleErrorWithSentry } from './handleError'; -export { wrapLoadWithSentry } from './load'; +export { wrapLoadWithSentry, wrapServerLoadWithSentry } from './load'; export { sentryHandle } from './handle'; diff --git a/packages/sveltekit/src/server/load.ts b/packages/sveltekit/src/server/load.ts index b0727810dea6..a050a5e904ba 100644 --- a/packages/sveltekit/src/server/load.ts +++ b/packages/sveltekit/src/server/load.ts @@ -3,7 +3,7 @@ import { trace } from '@sentry/core'; import { captureException } from '@sentry/node'; import type { TransactionContext } from '@sentry/types'; import { addExceptionMechanism, objectify } from '@sentry/utils'; -import type { HttpError, Load, ServerLoad } from '@sveltejs/kit'; +import type { HttpError, LoadEvent, ServerLoadEvent } from '@sveltejs/kit'; import { getTracePropagationData } from './utils'; @@ -42,19 +42,18 @@ function sendErrorToSentry(e: unknown): unknown { } /** - * Wrap a universal load function (e.g. +page.js or +layout.js) with Sentry functionality - * - * Usage: - * ```js - * import { } - * ``` - * - * @param origLoad SvelteKit user defined load function + * @inheritdoc */ -export function wrapLoadWithSentry(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 any>(origLoad: T): T { return new Proxy(origLoad, { apply: (wrappingTarget, thisArg, args: Parameters) => { - const [event] = args; + // 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 traceLoadContext: TransactionContext = { @@ -73,20 +72,33 @@ export function wrapLoadWithSentry(origLoad: T): T { /** * Wrap a server-only load function (e.g. +page.server.js or +layout.server.js) with Sentry functionality - * TODO: usage + * + * Usage: + * + * ```js + * // +page.serverjs + * + * import { wrapServerLoadWithSentry } + * + * export const load = wrapServerLoadWithSentry((event) => { + * // your load code + * }); + * ``` * * @param origServerLoad SvelteKit user defined server-only load function */ -export function wrapServerLoadWithSentry(origServerLoad: T): T { +// 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 any>(origServerLoad: T): T { return new Proxy(origServerLoad, { apply: (wrappingTarget, thisArg, args: Parameters) => { - const [event] = args; + // 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; - // Usually, the `handleWithSentry` hook handler should already create a transaction and store - // traceparent and DSC on that transaction before the server-only load function is called. - // However, since we have access to `event.request` we can still pass it to `trace` - // in case our handler isn't called or for some reason the handle hook is bypassed. const { dynamicSamplingContext, traceparentData } = getTracePropagationData(event); const traceLoadContext: TransactionContext = { diff --git a/packages/sveltekit/src/server/utils.ts b/packages/sveltekit/src/server/utils.ts new file mode 100644 index 000000000000..67c3bfe9e050 --- /dev/null +++ b/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; +} { + 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 }; +} diff --git a/packages/sveltekit/test/server/utils.test.ts b/packages/sveltekit/test/server/utils.test.ts new file mode 100644 index 000000000000..8e5c064c338c --- /dev/null +++ b/packages/sveltekit/test/server/utils.test.ts @@ -0,0 +1,55 @@ +import { getTracePropagationData } from '../../src/server/utils'; + +const MOCK_REQUEST_EVENT: any = { + request: { + headers: { + get: (key: string) => { + if (key === 'sentry-trace') { + return '1234567890abcdef1234567890abcdef-1234567890abcdef-1'; + } + + if (key === 'baggage') { + return ( + 'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,' + + 'sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' + + 'sentry-trace_id=1234567890abcdef1234567890abcdef,sentry-sample_rate=1' + ); + } + + return null; + }, + }, + }, +}; + +describe('getTracePropagationData', () => { + it('returns traceParentData and DSC if both are available', () => { + const event: any = MOCK_REQUEST_EVENT; + + const { traceparentData, dynamicSamplingContext } = getTracePropagationData(event); + + expect(traceparentData).toEqual({ + parentSampled: true, + parentSpanId: '1234567890abcdef', + traceId: '1234567890abcdef1234567890abcdef', + }); + + expect(dynamicSamplingContext).toEqual({ + environment: 'production', + public_key: 'dogsarebadatkeepingsecrets', + release: '1.0.0', + sample_rate: '1', + trace_id: '1234567890abcdef1234567890abcdef', + transaction: 'dogpark', + user_segment: 'segmentA', + }); + }); + + it('returns undefined if the necessary header is not avaolable', () => { + const event: any = { request: { headers: { get: () => undefined } } }; + const { traceparentData, dynamicSamplingContext } = getTracePropagationData(event); + + expect(traceparentData).toBeUndefined(); + expect(dynamicSamplingContext).toBeUndefined(); + }); +});