Skip to content

Commit cbd6721

Browse files
Lms24mydea
andcommitted
fix(sveltekit): Avoid capturing Http 4xx errors on the client (#10571)
Add the Http 400 avoidance logic from our server-side `load` function instrumentation to the client-side wrapper. Didn't know that these errors were a thing on the client side but now that we know, we definitely don't want to capture them. Co-authored-by: Francesco Novy <[email protected]>
1 parent 150b257 commit cbd6721

File tree

2 files changed

+30
-2
lines changed

2 files changed

+30
-2
lines changed

packages/sveltekit/src/client/load.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { addNonEnumerableProperty, objectify } from '@sentry/utils';
44
import type { LoadEvent } from '@sveltejs/kit';
55

66
import type { SentryWrappedFlag } from '../common/utils';
7-
import { isRedirect } from '../common/utils';
7+
import { isHttpError, isRedirect } from '../common/utils';
88

99
type PatchedLoadEvent = LoadEvent & Partial<SentryWrappedFlag>;
1010

@@ -14,7 +14,11 @@ function sendErrorToSentry(e: unknown): unknown {
1414
const objectifiedErr = objectify(e);
1515

1616
// We don't want to capture thrown `Redirect`s as these are not errors but expected behaviour
17-
if (isRedirect(objectifiedErr)) {
17+
// Neither 4xx errors, given that they are not valuable.
18+
if (
19+
isRedirect(objectifiedErr) ||
20+
(isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400)
21+
) {
1822
return objectifiedErr;
1923
}
2024

packages/sveltekit/test/client/load.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,30 @@ describe('wrapLoadWithSentry', () => {
6969
expect(mockCaptureException).not.toHaveBeenCalled();
7070
});
7171

72+
it.each([400, 404, 499])("doesn't call captureException for thrown `HttpError`s with status %s", async status => {
73+
async function load(_: Parameters<Load>[0]): Promise<ReturnType<Load>> {
74+
throw { status, body: 'error' };
75+
}
76+
77+
const wrappedLoad = wrapLoadWithSentry(load);
78+
const res = wrappedLoad(MOCK_LOAD_ARGS);
79+
await expect(res).rejects.toThrow();
80+
81+
expect(mockCaptureException).not.toHaveBeenCalled();
82+
});
83+
84+
it.each([500, 501, 599])('calls captureException for thrown `HttpError`s with status %s', async status => {
85+
async function load(_: Parameters<Load>[0]): Promise<ReturnType<Load>> {
86+
throw { status, body: 'error' };
87+
}
88+
89+
const wrappedLoad = wrapLoadWithSentry(load);
90+
const res = wrappedLoad(MOCK_LOAD_ARGS);
91+
await expect(res).rejects.toThrow();
92+
93+
expect(mockCaptureException).toHaveBeenCalledTimes(1);
94+
});
95+
7296
describe('calls trace function', async () => {
7397
it('creates a load span', async () => {
7498
async function load({ params }: Parameters<Load>[0]): Promise<ReturnType<Load>> {

0 commit comments

Comments
 (0)