From b979a5845601115531ba49e578a60c592c2b9ccb Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 8 Feb 2024 13:17:31 +0100 Subject: [PATCH 1/3] fix(sveltekit): Avoid capturing Http 400 errors on the client --- packages/sveltekit/src/client/load.ts | 7 ++++-- packages/sveltekit/test/client/load.test.ts | 24 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/packages/sveltekit/src/client/load.ts b/packages/sveltekit/src/client/load.ts index 14528959d34e..9ae329ee0a6b 100644 --- a/packages/sveltekit/src/client/load.ts +++ b/packages/sveltekit/src/client/load.ts @@ -4,7 +4,7 @@ import { addNonEnumerableProperty, objectify } from '@sentry/utils'; import type { LoadEvent } from '@sveltejs/kit'; import type { SentryWrappedFlag } from '../common/utils'; -import { isRedirect } from '../common/utils'; +import { isHttpError, isRedirect } from '../common/utils'; type PatchedLoadEvent = LoadEvent & Partial; @@ -14,7 +14,10 @@ function sendErrorToSentry(e: unknown): unknown { const objectifiedErr = objectify(e); // We don't want to capture thrown `Redirect`s as these are not errors but expected behaviour - if (isRedirect(objectifiedErr)) { + 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 e839b5a9cba5..ca7c1d625224 100644 --- a/packages/sveltekit/test/client/load.test.ts +++ b/packages/sveltekit/test/client/load.test.ts @@ -69,6 +69,30 @@ describe('wrapLoadWithSentry', () => { expect(mockCaptureException).not.toHaveBeenCalled(); }); + it.each([400, 404, 499])("doesn't call captureException for thrown `HttpError`s with status %s", async status => { + async function load(_: Parameters[0]): Promise> { + throw { status, body: 'error' }; + } + + const wrappedLoad = wrapLoadWithSentry(load); + const res = wrappedLoad(MOCK_LOAD_ARGS); + await expect(res).rejects.toThrow(); + + expect(mockCaptureException).not.toHaveBeenCalled(); + }); + + it.each([500, 501, 599])('calls captureException for thrown `HttpError`s with status %s', async status => { + async function load(_: Parameters[0]): Promise> { + throw { status, body: 'error' }; + } + + const wrappedLoad = wrapLoadWithSentry(load); + const res = wrappedLoad(MOCK_LOAD_ARGS); + await expect(res).rejects.toThrow(); + + expect(mockCaptureException).toHaveBeenCalledTimes(1); + }); + describe('calls trace function', async () => { it('creates a load span', async () => { async function load({ params }: Parameters[0]): Promise> { From caf5da1b7473c8458a0e200de9e96abf8b6081c1 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 8 Feb 2024 13:21:36 +0100 Subject: [PATCH 2/3] cleanup --- packages/sveltekit/src/client/load.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/sveltekit/src/client/load.ts b/packages/sveltekit/src/client/load.ts index 9ae329ee0a6b..72d39474eaa7 100644 --- a/packages/sveltekit/src/client/load.ts +++ b/packages/sveltekit/src/client/load.ts @@ -14,6 +14,7 @@ function sendErrorToSentry(e: unknown): unknown { const objectifiedErr = objectify(e); // We don't want to capture thrown `Redirect`s as these are not errors but expected behaviour + // Neither 400 errors, given that they are not valuable. if ( isRedirect(objectifiedErr) || (isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400) From 6e0c249ca1c560174dda019229a6f85ea333d29f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 8 Feb 2024 14:51:41 +0100 Subject: [PATCH 3/3] Update packages/sveltekit/src/client/load.ts Co-authored-by: Francesco Novy --- packages/sveltekit/src/client/load.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sveltekit/src/client/load.ts b/packages/sveltekit/src/client/load.ts index 72d39474eaa7..3e454bdb364a 100644 --- a/packages/sveltekit/src/client/load.ts +++ b/packages/sveltekit/src/client/load.ts @@ -14,7 +14,7 @@ function sendErrorToSentry(e: unknown): unknown { const objectifiedErr = objectify(e); // We don't want to capture thrown `Redirect`s as these are not errors but expected behaviour - // Neither 400 errors, given that they are not valuable. + // Neither 4xx errors, given that they are not valuable. if ( isRedirect(objectifiedErr) || (isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400)