fix(v9/sveltekit): Align error status filtering and mechanism in handleErrorWithSentry

packages/sveltekit/src/client/handleError.ts

@@ -28,20 +28,35 @@ type SafeHandleServerErrorInput = Omit<HandleClientErrorInput, 'status' | 'messa
  *
  * @param handleError The original SvelteKit error handler.
  */
-export function handleErrorWithSentry(handleError: HandleClientError = defaultErrorHandler): HandleClientError {
-  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,
-        },
-      });
+export function handleErrorWithSentry(handleError?: HandleClientError): HandleClientError {
+  const errorHandler = handleError ?? defaultErrorHandler;
+
+  return (input: HandleClientErrorInput): ReturnType<HandleClientError> => {
+    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;
+}

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

@@ -27,18 +27,18 @@ type SafeHandleServerErrorInput = Omit<HandleServerErrorInput, 'status' | 'messa
  *
  * @param handleError The original SvelteKit error handler.
  */
-export function handleErrorWithSentry(handleError: HandleServerError = defaultErrorHandler): HandleServerError {
-  return async (input: SafeHandleServerErrorInput): Promise<void | App.Error> => {
-    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<void | App.Error> => {
+    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;
   }
 

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!');

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);
     });