add case for load

Author: AbhiPrasad

packages/sveltekit/.eslintrc.js

@@ -3,5 +3,14 @@ module.exports = {
     browser: true,
     node: true,
   },
+  overrides: [
+    {
+      files: ['*.ts'],
+      rules: {
+        // Turning this off because it's not working with @sveltejs/kit
+        'import/no-unresolved': 'off',
+      },
+    },
+  ],
   extends: ['../../.eslintrc.js'],
 };

packages/sveltekit/src/server/load.ts

@@ -1,7 +1,10 @@
 import { captureException } from '@sentry/node';
 import { addExceptionMechanism, objectify } from '@sentry/utils';
-// eslint-disable-next-line import/no-unresolved
-import type { ServerLoad } from '@sveltejs/kit';
+import type { HttpError, ServerLoad } from '@sveltejs/kit';
+
+function isHttpError(err: unknown): err is HttpError {
+  return typeof err === 'object' && err !== null && 'status' in err && 'body' in err;
+}
 
 /**
  * Wrap load function with Sentry
@@ -16,7 +19,14 @@ export function wrapLoadWithSentry(origLoad: ServerLoad): ServerLoad {
       } catch (e) {
         // 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);
+        const objectifiedErr = objectify(e) as unknown;
+
+        // The error() helper is commonly used to throw errors in load functions: https://kit.svelte.dev/docs/modules#sveltejs-kit-error
+        // If we detect a thrown error that is an instance of HttpError, we don't want to capture 4xx errors as they
+        // could be noisy.
+        if (isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400) {
+          throw objectifiedErr;
+        }
 
         captureException(objectifiedErr, scope => {
           scope.addEventProcessor(event => {

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

@@ -1,8 +1,4 @@
 import { Scope } from '@sentry/svelte';
-// For now disable the import/no-unresolved rule, because we don't have a way to
-// tell eslint that we are only importing types from the @sveltejs/kit package without
-// adding a custom resolver, which will take too much time.
-// eslint-disable-next-line import/no-unresolved
 import type { HandleClientError, NavigationEvent } from '@sveltejs/kit';
 import { vi } from 'vitest';
 

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

@@ -1,8 +1,4 @@
 import { Scope } from '@sentry/node';
-// For now disable the import/no-unresolved rule, because we don't have a way to
-// tell eslint that we are only importing types from the @sveltejs/kit package without
-// adding a custom resolver, which will take too much time.
-// eslint-disable-next-line import/no-unresolved
 import type { HandleServerError, RequestEvent } from '@sveltejs/kit';
 import { vi } from 'vitest';
 
@@ -80,6 +76,6 @@ describe('handleError', () => {
 
     expect(addEventProcessorSpy).toBeCalledTimes(1);
     expect(mockAddExceptionMechanism).toBeCalledTimes(1);
-    expect(mockAddExceptionMechanism).toBeCalledWith({}, { handled: false, type: 'sveltekit' });
+    expect(mockAddExceptionMechanism).toBeCalledWith({}, { handled: false, type: 'instrument' });
   });
 });

packages/sveltekit/test/server/load.test.ts

@@ -1,14 +1,15 @@
 import { Scope } from '@sentry/node';
-// eslint-disable-next-line import/no-unresolved
 import type { ServerLoad } from '@sveltejs/kit';
+import { error } from '@sveltejs/kit';
+import { vi } from 'vitest';
 
 import { wrapLoadWithSentry } from '../../src/server/load';
 
-const mockCaptureException = jest.fn();
+const mockCaptureException = vi.fn();
 let mockScope = new Scope();
 
-jest.mock('@sentry/node', () => {
-  const original = jest.requireActual('@sentry/node');
+vi.mock('@sentry/node', async () => {
+  const original = (await vi.importActual('@sentry/node')) as any;
   return {
     ...original,
     captureException: (err: unknown, cb: (arg0: unknown) => unknown) => {
@@ -19,10 +20,10 @@ jest.mock('@sentry/node', () => {
   };
 });
 
-const mockAddExceptionMechanism = jest.fn();
+const mockAddExceptionMechanism = vi.fn();
 
-jest.mock('@sentry/utils', () => {
-  const original = jest.requireActual('@sentry/utils');
+vi.mock('@sentry/utils', async () => {
+  const original = (await vi.importActual('@sentry/utils')) as any;
   return {
     ...original,
     addExceptionMechanism: (...args: unknown[]) => mockAddExceptionMechanism(...args),
@@ -33,12 +34,6 @@ function getById(_id?: string) {
   throw new Error('error');
 }
 
-async function erroringLoad({ params }: Parameters<ServerLoad>[0]): Promise<ReturnType<ServerLoad>> {
-  return {
-    post: getById(params.id),
-  };
-}
-
 describe('wrapLoadWithSentry', () => {
   beforeEach(() => {
     mockCaptureException.mockClear();
@@ -47,20 +42,59 @@ describe('wrapLoadWithSentry', () => {
   });
 
   it('calls captureException', async () => {
-    const wrappedLoad = wrapLoadWithSentry(erroringLoad);
+    async function load({ params }: Parameters<ServerLoad>[0]): Promise<ReturnType<ServerLoad>> {
+      return {
+        post: getById(params.id),
+      };
+    }
+
+    const wrappedLoad = wrapLoadWithSentry(load);
     const res = wrappedLoad({ params: { id: '1' } } as any);
     await expect(res).rejects.toThrow();
 
     expect(mockCaptureException).toHaveBeenCalledTimes(1);
   });
 
+  describe('with error() helper', () => {
+    it.each([
+      // [statusCode, timesCalled]
+      [400, 0],
+      [401, 0],
+      [403, 0],
+      [404, 0],
+      [409, 0],
+      [429, 0],
+      [499, 0],
+      [500, 1],
+      [501, 1],
+      [503, 1],
+      [504, 1],
+    ])('error with status code %s calls captureException %s times', async (code, times) => {
+      async function load({ params }: Parameters<ServerLoad>[0]): Promise<ReturnType<ServerLoad>> {
+        throw error(code, params.id);
+      }
+
+      const wrappedLoad = wrapLoadWithSentry(load);
+      const res = wrappedLoad({ params: { id: '1' } } as any);
+      await expect(res).rejects.toThrow();
+
+      expect(mockCaptureException).toHaveBeenCalledTimes(times);
+    });
+  });
+
   it('adds an exception mechanism', async () => {
-    const addEventProcessorSpy = jest.spyOn(mockScope, 'addEventProcessor').mockImplementationOnce(callback => {
+    const addEventProcessorSpy = vi.spyOn(mockScope, 'addEventProcessor').mockImplementationOnce(callback => {
       void callback({}, { event_id: 'fake-event-id' });
       return mockScope;
     });
 
-    const wrappedLoad = wrapLoadWithSentry(erroringLoad);
+    async function load({ params }: Parameters<ServerLoad>[0]): Promise<ReturnType<ServerLoad>> {
+      return {
+        post: getById(params.id),
+      };
+    }
+
+    const wrappedLoad = wrapLoadWithSentry(load);
     const res = wrappedLoad({ params: { id: '1' } } as any);
     await expect(res).rejects.toThrow();