Skip to content

Commit

Permalink
fix(sveltekit): Avoid capturing Http 4xx errors on the client (#10571)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
Lms24 and mydea committed Feb 12, 2024
1 parent 150b257 commit cbd6721
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 2 deletions.
8 changes: 6 additions & 2 deletions packages/sveltekit/src/client/load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<SentryWrappedFlag>;

Expand All @@ -14,7 +14,11 @@ 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)) {
// Neither 4xx errors, given that they are not valuable.
if (
isRedirect(objectifiedErr) ||
(isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400)
) {
return objectifiedErr;
}

Expand Down
24 changes: 24 additions & 0 deletions packages/sveltekit/test/client/load.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Load>[0]): Promise<ReturnType<Load>> {
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<Load>[0]): Promise<ReturnType<Load>> {
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<Load>[0]): Promise<ReturnType<Load>> {
Expand Down

0 comments on commit cbd6721

Please sign in to comment.