Skip to content

Commit

Permalink
fix(sveltekit): Avoid capturing 404 errors on client side (#9902)
Browse files Browse the repository at this point in the history
Looks like 404 errors weren't passed to the client side `handleError`
hook in Kit 1.x but in 2.x they're now passed into the hook. This means,
we need to filter them out.
  • Loading branch information
Lms24 committed Dec 19, 2023
1 parent cef3621 commit cf773fc
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"@sveltejs/adapter-auto": "^3.0.0",
"@sveltejs/adapter-node": "^2.0.0",
"@sveltejs/kit": "^2.0.0",
"@sveltejs/vite-plugin-svelte": "^3.0.0",
"svelte": "^4.2.8",
"svelte-check": "^3.6.0",
"ts-node": "10.9.1",
Expand Down
30 changes: 22 additions & 8 deletions packages/sveltekit/src/client/handleError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,37 @@ function defaultErrorHandler({ error }: Parameters<HandleClientError>[0]): Retur
});
}

// TODO: add backwards-compatible type for kit 1.x (soon)
type HandleClientErrorInput = Parameters<HandleClientError>[0];

/**
* Backwards-compatible HandleServerError Input type for SvelteKit 1.x and 2.x
* `message` and `status` were added in 2.x.
* For backwards-compatibility, we make them optional
*
* @see https://kit.svelte.dev/docs/migrating-to-sveltekit-2#improved-error-handling
*/
type SafeHandleServerErrorInput = Omit<HandleClientErrorInput, 'status' | 'message'> &
Partial<Pick<HandleClientErrorInput, 'status' | 'message'>>;

/**
* Wrapper for the SvelteKit error handler that sends the error to Sentry.
*
* @param handleError The original SvelteKit error handler.
*/
export function handleErrorWithSentry(handleError: HandleClientError = defaultErrorHandler): HandleClientError {
return (input: HandleClientErrorInput): ReturnType<HandleClientError> => {
captureException(input.error, {
mechanism: {
type: 'sveltekit',
handled: false,
},
});
return (input: SafeHandleServerErrorInput): ReturnType<HandleClientError> => {
// SvelteKit 2.0 offers a reliable way to check for a 404 error:
if (input.status !== 404) {
captureException(input.error, {
mechanism: {
type: 'sveltekit',
handled: false,
},
});
}

// We're extra cautious with SafeHandleServerErrorInput - this type is not compatible with HandleServerErrorInput
// @ts-expect-error - we're still passing the same object, just with a different (backwards-compatible) type
return handleError(input);
};
}
17 changes: 17 additions & 0 deletions packages/sveltekit/test/client/handleError.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe('handleError', () => {
it('invokes the default handler if no handleError func is provided', async () => {
const wrappedHandleError = handleErrorWithSentry();
const mockError = new Error('test');
// @ts-expect-error - purposefully omitting status and message to cover SvelteKit 1.x compatibility
const returnVal = await wrappedHandleError({ error: mockError, event: navigationEvent });

expect(returnVal).not.toBeDefined();
Expand All @@ -50,6 +51,7 @@ describe('handleError', () => {
it('invokes the user-provided error handler', async () => {
const wrappedHandleError = handleErrorWithSentry(handleError);
const mockError = new Error('test');
// @ts-expect-error - purposefully omitting status and message to cover SvelteKit 1.x compatibility
const returnVal = (await wrappedHandleError({ error: mockError, event: navigationEvent })) as any;

expect(returnVal.message).toEqual('Whoops!');
Expand All @@ -59,4 +61,19 @@ describe('handleError', () => {
expect(consoleErrorSpy).toHaveBeenCalledTimes(0);
});
});

it("doesn't capture 404 errors", async () => {
const wrappedHandleError = handleErrorWithSentry(handleError);
const returnVal = (await wrappedHandleError({
error: new Error('404 Not Found'),
event: navigationEvent,
status: 404,
message: 'Not Found',
})) as any;

expect(returnVal.message).toEqual('Whoops!');
expect(mockCaptureException).not.toHaveBeenCalled();
// Check that the default handler wasn't invoked
expect(consoleErrorSpy).toHaveBeenCalledTimes(0);
});
});

0 comments on commit cf773fc

Please sign in to comment.