From cf773fccb1735ff2a3152aafb5e14d75bbd7be38 Mon Sep 17 00:00:00 2001
From: Lukas Stracke <lukas.stracke@sentry.io>
Date: Tue, 19 Dec 2023 14:16:57 +0100
Subject: [PATCH] fix(sveltekit): Avoid capturing 404 errors on client side
 (#9902)

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.
---
 .../sveltekit-2/package.json                  |  1 +
 packages/sveltekit/src/client/handleError.ts  | 30 ++++++++++++++-----
 .../sveltekit/test/client/handleError.test.ts | 17 +++++++++++
 3 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/packages/e2e-tests/test-applications/sveltekit-2/package.json b/packages/e2e-tests/test-applications/sveltekit-2/package.json
index e10818e519e6..b55d9ff74df6 100644
--- a/packages/e2e-tests/test-applications/sveltekit-2/package.json
+++ b/packages/e2e-tests/test-applications/sveltekit-2/package.json
@@ -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",
diff --git a/packages/sveltekit/src/client/handleError.ts b/packages/sveltekit/src/client/handleError.ts
index 34c235ee43df..799e6e36db72 100644
--- a/packages/sveltekit/src/client/handleError.ts
+++ b/packages/sveltekit/src/client/handleError.ts
@@ -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);
   };
 }
diff --git a/packages/sveltekit/test/client/handleError.test.ts b/packages/sveltekit/test/client/handleError.test.ts
index 0262f0b1b1cc..b4eba9ff56e9 100644
--- a/packages/sveltekit/test/client/handleError.test.ts
+++ b/packages/sveltekit/test/client/handleError.test.ts
@@ -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();
@@ -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!');
@@ -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);
+  });
 });