From 25af09f7da606ec1c2d1af06e06f82d8f8f3be03 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 25 Jul 2025 13:28:22 +0200 Subject: [PATCH 1/2] fix(sveltekit): Align error status filtering and mechanism in `handleErrorWithSentry` (#17157) Our `handleErrorWithSentry` logic was diverging a bit from the rest of the SvelteKit SDK and more general SDK patterns. Specifically, which errors would be ignored and when we set an error mechanism to `handled: true|false`. This PR now aligns two behaviours: - Ignore all 4xx errors (previously, we only ignored 404 errors) - Set `handled: true` if a custom error handler was defined by users and `handled: false` otherwise. --- packages/sveltekit/src/client/handleError.ts | 41 +++++++++++++------ .../src/server-common/handleError.ts | 22 +++++----- .../sveltekit/test/client/handleError.test.ts | 15 ++++--- .../test/server-common/handleError.test.ts | 33 ++++++++------- 4 files changed, 66 insertions(+), 45 deletions(-) diff --git a/packages/sveltekit/src/client/handleError.ts b/packages/sveltekit/src/client/handleError.ts index 447acce45e60..14a0a5b4b9cd 100644 --- a/packages/sveltekit/src/client/handleError.ts +++ b/packages/sveltekit/src/client/handleError.ts @@ -28,20 +28,35 @@ type SafeHandleServerErrorInput = Omit => { - // 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, - }, - }); +export function handleErrorWithSentry(handleError?: HandleClientError): HandleClientError { + const errorHandler = handleError ?? defaultErrorHandler; + + return (input: HandleClientErrorInput): ReturnType => { + if (is4xxError(input)) { + return errorHandler(input); } - // 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); + captureException(input.error, { + mechanism: { + type: 'sveltekit', + handled: !!handleError, + }, + }); + + return errorHandler(input); }; } + +// 4xx are expected errors and thus we don't want to capture them +function is4xxError(input: SafeHandleServerErrorInput): boolean { + const { status } = input; + + // Pre-SvelteKit 2.x, the status is not available, + // so we don't know if this is a 4xx error + if (!status) { + return false; + } + + // SvelteKit 2.0 offers a reliable way to check for a Not Found error: + return status >= 400 && status < 500; +} diff --git a/packages/sveltekit/src/server-common/handleError.ts b/packages/sveltekit/src/server-common/handleError.ts index 1586a9bc201e..046e4201c3cb 100644 --- a/packages/sveltekit/src/server-common/handleError.ts +++ b/packages/sveltekit/src/server-common/handleError.ts @@ -27,18 +27,18 @@ type SafeHandleServerErrorInput = Omit => { - if (isNotFoundError(input)) { - // 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); +export function handleErrorWithSentry(handleError?: HandleServerError): HandleServerError { + const errorHandler = handleError ?? defaultErrorHandler; + + return async (input: HandleServerErrorInput): Promise => { + if (is4xxError(input)) { + return errorHandler(input); } captureException(input.error, { mechanism: { type: 'sveltekit', - handled: false, + handled: !!handleError, }, }); @@ -60,20 +60,18 @@ export function handleErrorWithSentry(handleError: HandleServerError = defaultEr await flushIfServerless(); } - // 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); + return errorHandler(input); }; } /** * When a page request fails because the page is not found, SvelteKit throws a "Not found" error. */ -function isNotFoundError(input: SafeHandleServerErrorInput): boolean { +function is4xxError(input: SafeHandleServerErrorInput): boolean { const { error, event, status } = input; // SvelteKit 2.0 offers a reliable way to check for a Not Found error: - if (status === 404) { + if (!!status && status >= 400 && status < 500) { return true; } diff --git a/packages/sveltekit/test/client/handleError.test.ts b/packages/sveltekit/test/client/handleError.test.ts index 520f98fec56d..810acd865aa2 100644 --- a/packages/sveltekit/test/client/handleError.test.ts +++ b/packages/sveltekit/test/client/handleError.test.ts @@ -27,7 +27,7 @@ const captureExceptionEventHint = { const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(_ => {}); -describe('handleError', () => { +describe('handleError (client)', () => { beforeEach(() => { mockCaptureException.mockClear(); consoleErrorSpy.mockClear(); @@ -55,19 +55,22 @@ describe('handleError', () => { expect(returnVal.message).toEqual('Whoops!'); expect(mockCaptureException).toHaveBeenCalledTimes(1); - expect(mockCaptureException).toHaveBeenCalledWith(mockError, captureExceptionEventHint); + expect(mockCaptureException).toHaveBeenCalledWith(mockError, { + mechanism: { handled: true, type: 'sveltekit' }, + }); + // Check that the default handler wasn't invoked expect(consoleErrorSpy).toHaveBeenCalledTimes(0); }); }); - it("doesn't capture 404 errors", async () => { + it.each([400, 401, 402, 403, 404, 429, 499])("doesn't capture %s errors", async statusCode => { const wrappedHandleError = handleErrorWithSentry(handleError); const returnVal = (await wrappedHandleError({ - error: new Error('404 Not Found'), + error: new Error(`Error with status ${statusCode}`), event: navigationEvent, - status: 404, - message: 'Not Found', + status: statusCode, + message: `Error with status ${statusCode}`, })) as any; expect(returnVal.message).toEqual('Whoops!'); diff --git a/packages/sveltekit/test/server-common/handleError.test.ts b/packages/sveltekit/test/server-common/handleError.test.ts index 287e8f2c88e8..6b4b3af992d1 100644 --- a/packages/sveltekit/test/server-common/handleError.test.ts +++ b/packages/sveltekit/test/server-common/handleError.test.ts @@ -19,7 +19,7 @@ const requestEvent = {} as RequestEvent; const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(_ => {}); -describe('handleError', () => { +describe('handleError (server)', () => { beforeEach(() => { mockCaptureException.mockClear(); consoleErrorSpy.mockClear(); @@ -42,20 +42,23 @@ describe('handleError', () => { expect(consoleErrorSpy).toHaveBeenCalledTimes(1); }); - it('doesn\'t capture "Not found" errors for incorrect navigations [Kit 2.x]', async () => { - const wrappedHandleError = handleErrorWithSentry(); + it.each([400, 401, 402, 403, 404, 429, 499])( + "doesn't capture %s errors for incorrect navigations [Kit 2.x]", + async statusCode => { + const wrappedHandleError = handleErrorWithSentry(); - const returnVal = await wrappedHandleError({ - error: new Error('404 /asdf/123'), - event: requestEvent, - status: 404, - message: 'Not Found', - }); + const returnVal = await wrappedHandleError({ + error: new Error(`Error with status ${statusCode}`), + event: requestEvent, + status: statusCode, + message: `Error with status ${statusCode}`, + }); - expect(returnVal).not.toBeDefined(); - expect(mockCaptureException).toHaveBeenCalledTimes(0); - expect(consoleErrorSpy).toHaveBeenCalledTimes(1); - }); + expect(returnVal).not.toBeDefined(); + expect(mockCaptureException).toHaveBeenCalledTimes(0); + expect(consoleErrorSpy).toHaveBeenCalledTimes(1); + }, + ); describe('calls captureException', () => { it('invokes the default handler if no handleError func is provided', async () => { @@ -87,7 +90,9 @@ describe('handleError', () => { expect(returnVal.message).toEqual('Whoops!'); expect(mockCaptureException).toHaveBeenCalledTimes(1); - expect(mockCaptureException).toHaveBeenCalledWith(mockError, captureExceptionEventHint); + expect(mockCaptureException).toHaveBeenCalledWith(mockError, { + mechanism: { handled: true, type: 'sveltekit' }, + }); // Check that the default handler wasn't invoked expect(consoleErrorSpy).toHaveBeenCalledTimes(0); }); From c05b2ce934c2ce45989a1d9af6a91aa42cb8b9dc Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 25 Jul 2025 13:50:21 +0200 Subject: [PATCH 2/2] start ci?