Skip to content

Commit 38771bc

Browse files
authored
fix(v9/sveltekit): Align error status filtering and mechanism in handleErrorWithSentry (#17174)
backport of #17157
1 parent 6863635 commit 38771bc

File tree

4 files changed

+66
-45
lines changed

4 files changed

+66
-45
lines changed

packages/sveltekit/src/client/handleError.ts

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,35 @@ type SafeHandleServerErrorInput = Omit<HandleClientErrorInput, 'status' | 'messa
2828
*
2929
* @param handleError The original SvelteKit error handler.
3030
*/
31-
export function handleErrorWithSentry(handleError: HandleClientError = defaultErrorHandler): HandleClientError {
32-
return (input: SafeHandleServerErrorInput): ReturnType<HandleClientError> => {
33-
// SvelteKit 2.0 offers a reliable way to check for a 404 error:
34-
if (input.status !== 404) {
35-
captureException(input.error, {
36-
mechanism: {
37-
type: 'sveltekit',
38-
handled: false,
39-
},
40-
});
31+
export function handleErrorWithSentry(handleError?: HandleClientError): HandleClientError {
32+
const errorHandler = handleError ?? defaultErrorHandler;
33+
34+
return (input: HandleClientErrorInput): ReturnType<HandleClientError> => {
35+
if (is4xxError(input)) {
36+
return errorHandler(input);
4137
}
4238

43-
// We're extra cautious with SafeHandleServerErrorInput - this type is not compatible with HandleServerErrorInput
44-
// @ts-expect-error - we're still passing the same object, just with a different (backwards-compatible) type
45-
return handleError(input);
39+
captureException(input.error, {
40+
mechanism: {
41+
type: 'sveltekit',
42+
handled: !!handleError,
43+
},
44+
});
45+
46+
return errorHandler(input);
4647
};
4748
}
49+
50+
// 4xx are expected errors and thus we don't want to capture them
51+
function is4xxError(input: SafeHandleServerErrorInput): boolean {
52+
const { status } = input;
53+
54+
// Pre-SvelteKit 2.x, the status is not available,
55+
// so we don't know if this is a 4xx error
56+
if (!status) {
57+
return false;
58+
}
59+
60+
// SvelteKit 2.0 offers a reliable way to check for a Not Found error:
61+
return status >= 400 && status < 500;
62+
}

packages/sveltekit/src/server-common/handleError.ts

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,18 @@ type SafeHandleServerErrorInput = Omit<HandleServerErrorInput, 'status' | 'messa
2727
*
2828
* @param handleError The original SvelteKit error handler.
2929
*/
30-
export function handleErrorWithSentry(handleError: HandleServerError = defaultErrorHandler): HandleServerError {
31-
return async (input: SafeHandleServerErrorInput): Promise<void | App.Error> => {
32-
if (isNotFoundError(input)) {
33-
// We're extra cautious with SafeHandleServerErrorInput - this type is not compatible with HandleServerErrorInput
34-
// @ts-expect-error - we're still passing the same object, just with a different (backwards-compatible) type
35-
return handleError(input);
30+
export function handleErrorWithSentry(handleError?: HandleServerError): HandleServerError {
31+
const errorHandler = handleError ?? defaultErrorHandler;
32+
33+
return async (input: HandleServerErrorInput): Promise<void | App.Error> => {
34+
if (is4xxError(input)) {
35+
return errorHandler(input);
3636
}
3737

3838
captureException(input.error, {
3939
mechanism: {
4040
type: 'sveltekit',
41-
handled: false,
41+
handled: !!handleError,
4242
},
4343
});
4444

@@ -60,20 +60,18 @@ export function handleErrorWithSentry(handleError: HandleServerError = defaultEr
6060
await flushIfServerless();
6161
}
6262

63-
// We're extra cautious with SafeHandleServerErrorInput - this type is not compatible with HandleServerErrorInput
64-
// @ts-expect-error - we're still passing the same object, just with a different (backwards-compatible) type
65-
return handleError(input);
63+
return errorHandler(input);
6664
};
6765
}
6866

6967
/**
7068
* When a page request fails because the page is not found, SvelteKit throws a "Not found" error.
7169
*/
72-
function isNotFoundError(input: SafeHandleServerErrorInput): boolean {
70+
function is4xxError(input: SafeHandleServerErrorInput): boolean {
7371
const { error, event, status } = input;
7472

7573
// SvelteKit 2.0 offers a reliable way to check for a Not Found error:
76-
if (status === 404) {
74+
if (!!status && status >= 400 && status < 500) {
7775
return true;
7876
}
7977

packages/sveltekit/test/client/handleError.test.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const captureExceptionEventHint = {
2727

2828
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(_ => {});
2929

30-
describe('handleError', () => {
30+
describe('handleError (client)', () => {
3131
beforeEach(() => {
3232
mockCaptureException.mockClear();
3333
consoleErrorSpy.mockClear();
@@ -55,19 +55,22 @@ describe('handleError', () => {
5555

5656
expect(returnVal.message).toEqual('Whoops!');
5757
expect(mockCaptureException).toHaveBeenCalledTimes(1);
58-
expect(mockCaptureException).toHaveBeenCalledWith(mockError, captureExceptionEventHint);
58+
expect(mockCaptureException).toHaveBeenCalledWith(mockError, {
59+
mechanism: { handled: true, type: 'sveltekit' },
60+
});
61+
5962
// Check that the default handler wasn't invoked
6063
expect(consoleErrorSpy).toHaveBeenCalledTimes(0);
6164
});
6265
});
6366

64-
it("doesn't capture 404 errors", async () => {
67+
it.each([400, 401, 402, 403, 404, 429, 499])("doesn't capture %s errors", async statusCode => {
6568
const wrappedHandleError = handleErrorWithSentry(handleError);
6669
const returnVal = (await wrappedHandleError({
67-
error: new Error('404 Not Found'),
70+
error: new Error(`Error with status ${statusCode}`),
6871
event: navigationEvent,
69-
status: 404,
70-
message: 'Not Found',
72+
status: statusCode,
73+
message: `Error with status ${statusCode}`,
7174
})) as any;
7275

7376
expect(returnVal.message).toEqual('Whoops!');

packages/sveltekit/test/server-common/handleError.test.ts

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ const requestEvent = {} as RequestEvent;
1919

2020
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(_ => {});
2121

22-
describe('handleError', () => {
22+
describe('handleError (server)', () => {
2323
beforeEach(() => {
2424
mockCaptureException.mockClear();
2525
consoleErrorSpy.mockClear();
@@ -42,20 +42,23 @@ describe('handleError', () => {
4242
expect(consoleErrorSpy).toHaveBeenCalledTimes(1);
4343
});
4444

45-
it('doesn\'t capture "Not found" errors for incorrect navigations [Kit 2.x]', async () => {
46-
const wrappedHandleError = handleErrorWithSentry();
45+
it.each([400, 401, 402, 403, 404, 429, 499])(
46+
"doesn't capture %s errors for incorrect navigations [Kit 2.x]",
47+
async statusCode => {
48+
const wrappedHandleError = handleErrorWithSentry();
4749

48-
const returnVal = await wrappedHandleError({
49-
error: new Error('404 /asdf/123'),
50-
event: requestEvent,
51-
status: 404,
52-
message: 'Not Found',
53-
});
50+
const returnVal = await wrappedHandleError({
51+
error: new Error(`Error with status ${statusCode}`),
52+
event: requestEvent,
53+
status: statusCode,
54+
message: `Error with status ${statusCode}`,
55+
});
5456

55-
expect(returnVal).not.toBeDefined();
56-
expect(mockCaptureException).toHaveBeenCalledTimes(0);
57-
expect(consoleErrorSpy).toHaveBeenCalledTimes(1);
58-
});
57+
expect(returnVal).not.toBeDefined();
58+
expect(mockCaptureException).toHaveBeenCalledTimes(0);
59+
expect(consoleErrorSpy).toHaveBeenCalledTimes(1);
60+
},
61+
);
5962

6063
describe('calls captureException', () => {
6164
it('invokes the default handler if no handleError func is provided', async () => {
@@ -87,7 +90,9 @@ describe('handleError', () => {
8790

8891
expect(returnVal.message).toEqual('Whoops!');
8992
expect(mockCaptureException).toHaveBeenCalledTimes(1);
90-
expect(mockCaptureException).toHaveBeenCalledWith(mockError, captureExceptionEventHint);
93+
expect(mockCaptureException).toHaveBeenCalledWith(mockError, {
94+
mechanism: { handled: true, type: 'sveltekit' },
95+
});
9196
// Check that the default handler wasn't invoked
9297
expect(consoleErrorSpy).toHaveBeenCalledTimes(0);
9398
});

0 commit comments

Comments
 (0)