diff --git a/packages/sveltekit/src/client/load.ts b/packages/sveltekit/src/client/load.ts index e7e9a6673999..a154816b3dba 100644 --- a/packages/sveltekit/src/client/load.ts +++ b/packages/sveltekit/src/client/load.ts @@ -3,11 +3,18 @@ import { captureException } from '@sentry/svelte'; import { addExceptionMechanism, objectify } from '@sentry/utils'; import type { LoadEvent } from '@sveltejs/kit'; +import { isRedirect } from '../common/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. const objectifiedErr = objectify(e); + // We don't want to capture thrown `Redirect`s as these are not errors but expected behaviour + if (isRedirect(objectifiedErr)) { + return objectifiedErr; + } + captureException(objectifiedErr, scope => { scope.addEventProcessor(event => { addExceptionMechanism(event, { diff --git a/packages/sveltekit/src/common/utils.ts b/packages/sveltekit/src/common/utils.ts new file mode 100644 index 000000000000..86035117b6f6 --- /dev/null +++ b/packages/sveltekit/src/common/utils.ts @@ -0,0 +1,16 @@ +import type { Redirect } from '@sveltejs/kit'; + +/** + * 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 + * @param error the potential redirect error + */ +export function isRedirect(error: unknown): error is Redirect { + if (error == null || typeof error !== 'object') { + return false; + } + const hasValidLocation = 'location' in error && typeof error.location === 'string'; + const hasValidStatus = + 'status' in error && typeof error.status === 'number' && error.status >= 300 && error.status <= 308; + return hasValidLocation && hasValidStatus; +} diff --git a/packages/sveltekit/src/server/load.ts b/packages/sveltekit/src/server/load.ts index a050a5e904ba..7355d2a21fc4 100644 --- a/packages/sveltekit/src/server/load.ts +++ b/packages/sveltekit/src/server/load.ts @@ -5,6 +5,7 @@ import type { TransactionContext } from '@sentry/types'; import { addExceptionMechanism, objectify } from '@sentry/utils'; import type { HttpError, LoadEvent, ServerLoadEvent } from '@sveltejs/kit'; +import { isRedirect } from '../common/utils'; import { getTracePropagationData } from './utils'; function isHttpError(err: unknown): err is HttpError { @@ -19,7 +20,12 @@ function sendErrorToSentry(e: unknown): unknown { // 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. - if (isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400) { + // 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; } diff --git a/packages/sveltekit/test/client/load.test.ts b/packages/sveltekit/test/client/load.test.ts index f4d18c9f9909..ed5193e81e47 100644 --- a/packages/sveltekit/test/client/load.test.ts +++ b/packages/sveltekit/test/client/load.test.ts @@ -1,5 +1,6 @@ import { addTracingExtensions, Scope } from '@sentry/svelte'; import type { Load } from '@sveltejs/kit'; +import { redirect } from '@sveltejs/kit'; import { vi } from 'vitest'; import { wrapLoadWithSentry } from '../../src/client/load'; @@ -99,6 +100,18 @@ describe('wrapLoadWithSentry', () => { expect(mockCaptureException).toHaveBeenCalledTimes(1); }); + it("doesn't call captureException for thrown `Redirect`s", async () => { + async function load(_: Parameters[0]): Promise> { + throw redirect(300, 'other/route'); + } + + const wrappedLoad = wrapLoadWithSentry(load); + const res = wrappedLoad(MOCK_LOAD_ARGS); + await expect(res).rejects.toThrow(); + + expect(mockCaptureException).not.toHaveBeenCalled(); + }); + it('calls trace function', async () => { async function load({ params }: Parameters[0]): Promise> { return { diff --git a/packages/sveltekit/test/common/utils.test.ts b/packages/sveltekit/test/common/utils.test.ts new file mode 100644 index 000000000000..51af645cc961 --- /dev/null +++ b/packages/sveltekit/test/common/utils.test.ts @@ -0,0 +1,25 @@ +import { isRedirect } from '../../src/common/utils'; + +describe('isRedirect', () => { + it.each([ + { location: '/users/id', status: 300 }, + { location: '/users/id', status: 304 }, + { location: '/users/id', status: 308 }, + { location: '', status: 308 }, + ])('returns `true` for valid Redirect objects', redirectObject => { + expect(isRedirect(redirectObject)).toBe(true); + }); + + it.each([ + 300, + 'redirect', + { location: { route: { id: 'users/id' } }, status: 300 }, + { status: 308 }, + { location: '/users/id' }, + { location: '/users/id', status: 201 }, + { location: '/users/id', status: 400 }, + { location: '/users/id', status: 500 }, + ])('returns `false` for invalid Redirect objects', redirectObject => { + expect(isRedirect(redirectObject)).toBe(false); + }); +}); diff --git a/packages/sveltekit/test/server/load.test.ts b/packages/sveltekit/test/server/load.test.ts index 2671b0d5d217..16f6ef95a21c 100644 --- a/packages/sveltekit/test/server/load.test.ts +++ b/packages/sveltekit/test/server/load.test.ts @@ -1,7 +1,7 @@ import { addTracingExtensions } from '@sentry/core'; import { Scope } from '@sentry/node'; import type { Load, ServerLoad } from '@sveltejs/kit'; -import { error } from '@sveltejs/kit'; +import { error, redirect } from '@sveltejs/kit'; import { vi } from 'vitest'; import { wrapLoadWithSentry, wrapServerLoadWithSentry } from '../../src/server/load'; @@ -166,6 +166,18 @@ describe.each([ }); }); + it("doesn't call captureException for thrown `Redirect`s", async () => { + async function load(_: Parameters[0]): Promise> { + throw redirect(300, 'other/route'); + } + + const wrappedLoad = wrapLoadWithSentry(load); + const res = wrappedLoad(MOCK_LOAD_ARGS); + await expect(res).rejects.toThrow(); + + expect(mockCaptureException).not.toHaveBeenCalled(); + }); + it('adds an exception mechanism', async () => { const addEventProcessorSpy = vi.spyOn(mockScope, 'addEventProcessor').mockImplementationOnce(callback => { void callback({}, { event_id: 'fake-event-id' });