Skip to content

Commit

Permalink
feat(sveltekit): Add wrapServerRouteWithSentry wrapper (#13247)
Browse files Browse the repository at this point in the history
Add a wrapper for SvelteKit server routes.
The reason is that some errors (e.g. sveltekit `error()` calls) are not
caught within server (API) routes, as reported in #13224 because in
contrast to `load` function we don't directly try/catch the function
invokation.

For now, users will have to add this wrapper manually. At a later time
we can think about auto instrumentation, similarly to `load` functions
but for now this will remain manual.
  • Loading branch information
Lms24 authored Aug 8, 2024
1 parent 21830b1 commit a67a69e
Show file tree
Hide file tree
Showing 11 changed files with 307 additions and 79 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,7 @@
<li>
<a href="components">Component Tracking</a>
</li>
<li>
<a href="/wrap-server-route">server routes</a>
</li>
</ul>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script lang="ts">
export let data;
</script>

<p>
Message from API: {data.myMessage}
</p>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const load = async ({ fetch }) => {
const res = await fetch('/wrap-server-route/api');
const myMessage = await res.json();
return {
myMessage: myMessage.myMessage,
};
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { wrapServerRouteWithSentry } from '@sentry/sveltekit';
import { error } from '@sveltejs/kit';

export const GET = wrapServerRouteWithSentry(async () => {
error(500, 'error() error');
});
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,37 @@ test.describe('server-side errors', () => {

expect(errorEvent.transaction).toEqual('GET /server-route-error');
});

test('captures error() thrown in server route with `wrapServerRouteWithSentry`', async ({ page }) => {
const errorEventPromise = waitForError('sveltekit-2', errorEvent => {
return errorEvent?.exception?.values?.[0]?.value === "'HttpError' captured as exception with keys: body, status";
});

await page.goto('/wrap-server-route');

expect(await errorEventPromise).toMatchObject({
exception: {
values: [
{
value: "'HttpError' captured as exception with keys: body, status",
mechanism: {
handled: false,
data: {
function: 'serverRoute',
},
},
stacktrace: { frames: expect.any(Array) },
},
],
},
extra: {
__serialized__: {
body: {
message: 'error() error',
},
status: 500,
},
},
});
});
});
40 changes: 4 additions & 36 deletions packages/sveltekit/src/server/handle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,15 @@ import {
withIsolationScope,
} from '@sentry/core';
import { startSpan } from '@sentry/core';
import { captureException, continueTrace } from '@sentry/node';
import { continueTrace } from '@sentry/node';
import type { Span } from '@sentry/types';
import {
dynamicSamplingContextToSentryBaggageHeader,
logger,
objectify,
winterCGRequestToRequestData,
} from '@sentry/utils';
import { dynamicSamplingContextToSentryBaggageHeader, logger, winterCGRequestToRequestData } from '@sentry/utils';
import type { Handle, ResolveOptions } from '@sveltejs/kit';

import { getDynamicSamplingContextFromSpan } from '@sentry/opentelemetry';

import { DEBUG_BUILD } from '../common/debug-build';
import { isHttpError, isRedirect } from '../common/utils';
import { flushIfServerless, getTracePropagationData } from './utils';
import { flushIfServerless, getTracePropagationData, sendErrorToSentry } from './utils';

export type SentryHandleOptions = {
/**
Expand Down Expand Up @@ -62,32 +56,6 @@ export type SentryHandleOptions = {
fetchProxyScriptNonce?: string;
};

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.
const objectifiedErr = objectify(e);

// similarly to the `load` function, we don't want to capture 4xx errors or redirects
if (
isRedirect(objectifiedErr) ||
(isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400)
) {
return objectifiedErr;
}

captureException(objectifiedErr, {
mechanism: {
type: 'sveltekit',
handled: false,
data: {
function: 'handle',
},
},
});

return objectifiedErr;
}

/**
* Exported only for testing
*/
Expand Down Expand Up @@ -225,7 +193,7 @@ async function instrumentHandle(
);
return resolveResult;
} catch (e: unknown) {
sendErrorToSentry(e);
sendErrorToSentry(e, 'handle');
throw e;
} finally {
await flushIfServerless();
Expand Down
1 change: 1 addition & 0 deletions packages/sveltekit/src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ export { init } from './sdk';
export { handleErrorWithSentry } from './handleError';
export { wrapLoadWithSentry, wrapServerLoadWithSentry } from './load';
export { sentryHandle } from './handle';
export { wrapServerRouteWithSentry } from './serverRoute';

/**
* Tracks the Svelte component's initialization and mounting operation as well as
Expand Down
46 changes: 5 additions & 41 deletions packages/sveltekit/src/server/load.ts
Original file line number Diff line number Diff line change
@@ -1,49 +1,13 @@
import {
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
captureException,
startSpan,
} from '@sentry/node';
import { addNonEnumerableProperty, objectify } from '@sentry/utils';
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, startSpan } from '@sentry/node';
import { addNonEnumerableProperty } from '@sentry/utils';
import type { LoadEvent, ServerLoadEvent } from '@sveltejs/kit';

import type { SentryWrappedFlag } from '../common/utils';
import { isHttpError, isRedirect } from '../common/utils';
import { flushIfServerless } from './utils';
import { flushIfServerless, sendErrorToSentry } from './utils';

type PatchedLoadEvent = LoadEvent & SentryWrappedFlag;
type PatchedServerLoadEvent = ServerLoadEvent & 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.
const objectifiedErr = objectify(e);

// The error() helper is commonly used to throw errors in load functions: https://kit.svelte.dev/docs/modules#sveltejs-kit-error
// If we detect a thrown error that is an instance of HttpError, we don't want to capture 4xx errors as they
// could be noisy.
// Also the `redirect(...)` helper is used to redirect users from one page to another. We don't want to capture thrown
// `Redirect`s as they're not errors but expected behaviour
if (
isRedirect(objectifiedErr) ||
(isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400)
) {
return objectifiedErr;
}

captureException(objectifiedErr, {
mechanism: {
type: 'sveltekit',
handled: false,
data: {
function: 'load',
},
},
});

return objectifiedErr;
}

/**
* @inheritdoc
*/
Expand Down Expand Up @@ -81,7 +45,7 @@ export function wrapLoadWithSentry<T extends (...args: any) => any>(origLoad: T)
() => wrappingTarget.apply(thisArg, args),
);
} catch (e) {
sendErrorToSentry(e);
sendErrorToSentry(e, 'load');
throw e;
} finally {
await flushIfServerless();
Expand Down Expand Up @@ -146,7 +110,7 @@ export function wrapServerLoadWithSentry<T extends (...args: any) => any>(origSe
() => wrappingTarget.apply(thisArg, args),
);
} catch (e: unknown) {
sendErrorToSentry(e);
sendErrorToSentry(e, 'load');
throw e;
} finally {
await flushIfServerless();
Expand Down
67 changes: 67 additions & 0 deletions packages/sveltekit/src/server/serverRoute.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, startSpan } from '@sentry/node';
import { addNonEnumerableProperty } from '@sentry/utils';
import type { RequestEvent } from '@sveltejs/kit';
import { flushIfServerless, sendErrorToSentry } from './utils';

type PatchedServerRouteEvent = RequestEvent & { __sentry_wrapped__?: boolean };

/**
* Wraps a server route handler for API or server routes registered in `+server.(js|js)` files.
*
* This function will automatically capture any errors that occur during the execution of the route handler
* and it will start a span for the duration of your route handler.
*
* @example
* ```js
* import { wrapServerRouteWithSentry } from '@sentry/sveltekit';
*
* const get = async event => {
* return new Response(JSON.stringify({ message: 'hello world' }));
* }
*
* export const GET = wrapServerRouteWithSentry(get);
* ```
*
* @param originalRouteHandler your server route handler
* @param httpMethod the HTTP method of your route handler
*
* @returns a wrapped version of your server route handler
*/
export function wrapServerRouteWithSentry(
originalRouteHandler: (request: RequestEvent) => Promise<Response>,
): (requestEvent: RequestEvent) => Promise<Response> {
return new Proxy(originalRouteHandler, {
apply: async (wrappingTarget, thisArg, args) => {
const event = args[0] as PatchedServerRouteEvent;

if (event.__sentry_wrapped__) {
return wrappingTarget.apply(thisArg, args);
}

const routeId = event.route && event.route.id;
const httpMethod = event.request.method;

addNonEnumerableProperty(event as unknown as Record<string, unknown>, '__sentry_wrapped__', true);

try {
return await startSpan(
{
name: `${httpMethod} ${routeId || 'Server Route'}`,
op: `function.sveltekit.server.${httpMethod.toLowerCase()}`,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.sveltekit',
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
},
onlyIfParent: true,
},
() => wrappingTarget.apply(thisArg, args),
);
} catch (e) {
sendErrorToSentry(e, 'serverRoute');
throw e;
} finally {
await flushIfServerless();
}
},
});
}
43 changes: 41 additions & 2 deletions packages/sveltekit/src/server/utils.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { flush } from '@sentry/node';
import { logger } from '@sentry/utils';
import { captureException, flush } from '@sentry/node';
import { logger, objectify } from '@sentry/utils';
import type { RequestEvent } from '@sveltejs/kit';

import { DEBUG_BUILD } from '../common/debug-build';
import { isHttpError, isRedirect } from '../common/utils';

/**
* Takes a request event and extracts traceparent and DSC data
Expand Down Expand Up @@ -31,3 +32,41 @@ export async function flushIfServerless(): Promise<void> {
}
}
}

/**
* Extracts a server-side sveltekit error, filters a couple of known errors we don't want to capture
* and captures the error via `captureException`.
*
* @param e error
*
* @returns an objectified version of @param e
*/
export function sendErrorToSentry(e: unknown, handlerFn: 'handle' | 'load' | 'serverRoute'): object {
// 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.
const objectifiedErr = objectify(e);

// The error() helper is commonly used to throw errors in load functions: https://kit.svelte.dev/docs/modules#sveltejs-kit-error
// If we detect a thrown error that is an instance of HttpError, we don't want to capture 4xx errors as they
// could be noisy.
// Also the `redirect(...)` helper is used to redirect users from one page to another. We don't want to capture thrown
// `Redirect`s as they're not errors but expected behaviour
if (
isRedirect(objectifiedErr) ||
(isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400)
) {
return objectifiedErr;
}

captureException(objectifiedErr, {
mechanism: {
type: 'sveltekit',
handled: false,
data: {
function: handlerFn,
},
},
});

return objectifiedErr;
}
Loading

0 comments on commit a67a69e

Please sign in to comment.