fix(sveltekit): Avoid capturing redirects and 4xx Http errors in request Handler

Author: Lms24

packages/sveltekit/src/common/utils.ts

@@ -1,4 +1,4 @@
-import type { Redirect } from '@sveltejs/kit';
+import type { HttpError, Redirect } from '@sveltejs/kit';
 
 export type SentryWrappedFlag = {
   /**
@@ -22,3 +22,10 @@ export function isRedirect(error: unknown): error is Redirect {
     'status' in error && typeof error.status === 'number' && error.status >= 300 && error.status <= 308;
   return hasValidLocation && hasValidStatus;
 }
+
+/**
+ * Determines if a thrown "error" is a HttpError
+ */
+export function isHttpError(err: unknown): err is HttpError {
+  return typeof err === 'object' && err !== null && 'status' in err && 'body' in err;
+}

packages/sveltekit/src/server/handle.ts

@@ -5,13 +5,22 @@ import { captureException } from '@sentry/node';
 import { addExceptionMechanism, dynamicSamplingContextToSentryBaggageHeader, objectify } from '@sentry/utils';
 import type { Handle, ResolveOptions } from '@sveltejs/kit';
 
+import { isHttpError, isRedirect } from '../common/utils';
 import { getTracePropagationData } from './utils';
 
 function sendErrorToSentry(e: unknown): unknown {
   // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can
   // store a seen flag on it.
   const objectifiedErr = objectify(e);
 
+  // similarly to the `load` function, we don't want to capture 4xx errors or redirects
+  if (
+    isRedirect(objectifiedErr) ||
+    (isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400)
+  ) {
+    return objectifiedErr;
+  }
+
   captureException(objectifiedErr, scope => {
     scope.addEventProcessor(event => {
       addExceptionMechanism(event, {

packages/sveltekit/src/server/load.ts

@@ -3,19 +3,15 @@ import { trace } from '@sentry/core';
 import { captureException } from '@sentry/node';
 import type { TransactionContext } from '@sentry/types';
 import { addExceptionMechanism, addNonEnumerableProperty, objectify } from '@sentry/utils';
-import type { HttpError, LoadEvent, ServerLoadEvent } from '@sveltejs/kit';
+import type { LoadEvent, ServerLoadEvent } from '@sveltejs/kit';
 
 import type { SentryWrappedFlag } from '../common/utils';
-import { isRedirect } from '../common/utils';
+import { isHttpError, isRedirect } from '../common/utils';
 import { getTracePropagationData } from './utils';
 
 type PatchedLoadEvent = LoadEvent & SentryWrappedFlag;
 type PatchedServerLoadEvent = ServerLoadEvent & SentryWrappedFlag;
 
-function isHttpError(err: unknown): err is HttpError {
-  return typeof err === 'object' && err !== null && 'status' in err && 'body' in err;
-}
-
 function sendErrorToSentry(e: unknown): unknown {
   // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can
   // store a seen flag on it.

packages/sveltekit/test/common/utils.test.ts

@@ -1,4 +1,6 @@
-import { isRedirect } from '../../src/common/utils';
+import { redirect } from '@sveltejs/kit';
+
+import { isHttpError, isRedirect } from '../../src/common/utils';
 
 describe('isRedirect', () => {
   it.each([
@@ -23,3 +25,19 @@ describe('isRedirect', () => {
     expect(isRedirect(redirectObject)).toBe(false);
   });
 });
+
+describe('isHttpError', () => {
+  it.each([
+    { status: 404, body: 'Not found' },
+    { status: 500, body: 'Internal server error' },
+  ])('returns `true` for valid HttpError objects (%s)', httpErrorObject => {
+    expect(isHttpError(httpErrorObject)).toBe(true);
+  });
+
+  it.each([new Error(), redirect(301, '/users/id'), 'string error', { status: 404 }, { body: 'Not found' }])(
+    'returns `false` for other thrown objects (%s)',
+    httpErrorObject => {
+      expect(isHttpError(httpErrorObject)).toBe(false);
+    },
+  );
+});

packages/sveltekit/test/server/handle.test.ts

@@ -2,6 +2,7 @@ import { addTracingExtensions, Hub, makeMain, Scope } from '@sentry/core';
 import { NodeClient } from '@sentry/node';
 import type { Transaction } from '@sentry/types';
 import type { Handle } from '@sveltejs/kit';
+import { redirect } from '@sveltejs/kit';
 import { vi } from 'vitest';
 
 import { sentryHandle, transformPageChunk } from '../../src/server/handle';
@@ -69,7 +70,21 @@ const enum Type {
   Async = 'async',
 }
 
-function resolve(type: Type, isError: boolean): Parameters<Handle>[0]['resolve'] {
+function resolve(
+  type: Type,
+  isError: boolean,
+  throwSpecialError?: 'redirect' | 'httpError4xx' | 'httpError5xx',
+): Parameters<Handle>[0]['resolve'] {
+  if (throwSpecialError === 'redirect') {
+    throw redirect(302, '/redirect');
+  }
+  if (throwSpecialError === 'httpError4xx') {
+    throw { status: 404, body: 'Not found' };
+  }
+  if (throwSpecialError === 'httpError5xx') {
+    throw { status: 500, body: 'Internal error' };
+  }
+
   if (type === Type.Sync) {
     return (..._args: unknown[]) => {
       if (isError) {
@@ -292,6 +307,22 @@ describe('handleSentry', () => {
       }
     });
 
+    it("doesn't send redirects in a request handler to Sentry", async () => {
+      try {
+        await sentryHandle()({ event: mockEvent(), resolve: resolve(type, false, 'redirect') });
+      } catch (e) {
+        expect(mockCaptureException).toBeCalledTimes(0);
+      }
+    });
+
+    it("doesn't send Http 4xx errors in a request handler to Sentry", async () => {
+      try {
+        await sentryHandle()({ event: mockEvent(), resolve: resolve(type, false, 'httpError4xx') });
+      } catch (e) {
+        expect(mockCaptureException).toBeCalledTimes(0);
+      }
+    });
+
     it('calls `transformPageChunk`', async () => {
       const mockResolve = vi.fn().mockImplementation(resolve(type, isError));
       const event = mockEvent();