Skip to content

Commit f1128bd

Browse files
authored
fix(sveltekit): Termporarily disable serverside load tracing (#7587)
We discovered that our serverside `handleLoadWithSentry` wrapper caused two distinct transactions being created on page loads - one for the page load request and one for the `load` function (if one exists for that page). This shouldn't happen. We need to investigate how to fix this. As a short-term solution, we disable tracing on the serverside load wrapper to avoid quota usage increase for our users.
1 parent b0be4dd commit f1128bd

File tree

3 files changed

+21
-32
lines changed

3 files changed

+21
-32
lines changed

packages/sveltekit/src/index.types.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export * from './server';
99

1010
import type { Integration, Options, StackParser } from '@sentry/types';
1111
// eslint-disable-next-line import/no-unresolved
12-
import type { HandleClientError, HandleServerError, ServerLoad } from '@sveltejs/kit';
12+
import type { HandleClientError, HandleServerError, Load, ServerLoad } from '@sveltejs/kit';
1313

1414
import type * as clientSdk from './client';
1515
import type * as serverSdk from './server';
@@ -21,7 +21,7 @@ export declare function handleErrorWithSentry<T extends HandleClientError | Hand
2121
handleError?: T,
2222
): ReturnType<T>;
2323

24-
export declare function wrapLoadWithSentry<S extends ServerLoad>(origLoad: S): S;
24+
export declare function wrapLoadWithSentry<S extends ServerLoad | Load>(origLoad: S): S;
2525

2626
// We export a merged Integrations object so that users can (at least typing-wise) use all integrations everywhere.
2727
export declare const Integrations: typeof clientSdk.Integrations & typeof serverSdk.Integrations;

packages/sveltekit/src/server/load.ts

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,7 @@
11
/* eslint-disable @sentry-internal/sdk/no-optional-chaining */
2-
import { trace } from '@sentry/core';
32
import { captureException } from '@sentry/node';
4-
import {
5-
addExceptionMechanism,
6-
baggageHeaderToDynamicSamplingContext,
7-
extractTraceparentData,
8-
objectify,
9-
} from '@sentry/utils';
10-
import type { HttpError, ServerLoad } from '@sveltejs/kit';
3+
import { addExceptionMechanism, isThenable, objectify } from '@sentry/utils';
4+
import type { HttpError, Load, ServerLoad } from '@sveltejs/kit';
115
import * as domain from 'domain';
126

137
function isHttpError(err: unknown): err is HttpError {
@@ -49,32 +43,26 @@ function sendErrorToSentry(e: unknown): unknown {
4943
*
5044
* @param origLoad SvelteKit user defined load function
5145
*/
52-
export function wrapLoadWithSentry(origLoad: ServerLoad): ServerLoad {
46+
export function wrapLoadWithSentry<T extends ServerLoad | Load>(origLoad: T): T {
5347
return new Proxy(origLoad, {
5448
apply: (wrappingTarget, thisArg, args: Parameters<ServerLoad>) => {
5549
return domain.create().bind(() => {
56-
const [event] = args;
50+
let maybePromiseResult: ReturnType<T>;
5751

58-
const sentryTraceHeader = event.request.headers.get('sentry-trace');
59-
const baggageHeader = event.request.headers.get('baggage');
60-
const traceparentData = sentryTraceHeader ? extractTraceparentData(sentryTraceHeader) : undefined;
61-
const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggageHeader);
52+
try {
53+
maybePromiseResult = wrappingTarget.apply(thisArg, args);
54+
} catch (e) {
55+
sendErrorToSentry(e);
56+
throw e;
57+
}
6258

63-
const routeId = event.route.id;
64-
return trace(
65-
{
66-
op: 'function.sveltekit.load',
67-
name: routeId ? routeId : event.url.pathname,
68-
status: 'ok',
69-
...traceparentData,
70-
metadata: {
71-
source: routeId ? 'route' : 'url',
72-
dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
73-
},
74-
},
75-
() => wrappingTarget.apply(thisArg, args),
76-
sendErrorToSentry,
77-
);
59+
if (isThenable(maybePromiseResult)) {
60+
Promise.resolve(maybePromiseResult).then(null, e => {
61+
sendErrorToSentry(e);
62+
});
63+
}
64+
65+
return maybePromiseResult;
7866
})();
7967
},
8068
});

packages/sveltekit/test/server/load.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ describe('wrapLoadWithSentry', () => {
101101
expect(mockCaptureException).toHaveBeenCalledTimes(1);
102102
});
103103

104-
it('calls trace function', async () => {
104+
// TODO: enable this once we figured out how tracing the load function doesn't result in creating a new transaction
105+
it.skip('calls trace function', async () => {
105106
async function load({ params }: Parameters<ServerLoad>[0]): Promise<ReturnType<ServerLoad>> {
106107
return {
107108
post: params.id,

0 commit comments

Comments
 (0)