feat(sveltekit): Add wrapper for server load function (#7416)

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/index.ts

@@ -2,3 +2,4 @@ export * from '@sentry/node';
 
 export { init } from './sdk';
 export { handleErrorWithSentry } from './handleError';
+export { wrapLoadWithSentry } from './load';

packages/sveltekit/src/server/load.ts

@@ -0,0 +1,64 @@
+import { captureException } from '@sentry/node';
+import { addExceptionMechanism, isThenable, objectify } from '@sentry/utils';
+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;
+}
+
+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);
+
+  // 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) {
+    return objectifiedErr;
+  }
+
+  captureException(objectifiedErr, scope => {
+    scope.addEventProcessor(event => {
+      addExceptionMechanism(event, {
+        type: 'sveltekit',
+        handled: false,
+        data: {
+          function: 'load',
+        },
+      });
+      return event;
+    });
+
+    return scope;
+  });
+
+  return objectifiedErr;
+}
+
+/**
+ * Wrap load function with Sentry
+ *
+ * @param origLoad SvelteKit user defined load function
+ */
+export function wrapLoadWithSentry(origLoad: ServerLoad): ServerLoad {
+  return new Proxy(origLoad, {
+    apply: (wrappingTarget, thisArg, args: Parameters<ServerLoad>) => {
+      let maybePromiseResult;
+
+      try {
+        maybePromiseResult = wrappingTarget.apply(thisArg, args);
+      } catch (e) {
+        throw sendErrorToSentry(e);
+      }
+
+      if (isThenable(maybePromiseResult)) {
+        Promise.resolve(maybePromiseResult).then(null, e => {
+          sendErrorToSentry(e);
+        });
+      }
+
+      return maybePromiseResult;
+    },
+  });
+}

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';
 
@@ -12,7 +8,7 @@ const mockCaptureException = vi.fn();
 let mockScope = new Scope();
 
 vi.mock('@sentry/node', async () => {
-  const original = (await vi.importActual('@sentry/core')) as any;
+  const original = (await vi.importActual('@sentry/node')) as any;
   return {
     ...original,
     captureException: (err: unknown, cb: (arg0: unknown) => unknown) => {

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

@@ -0,0 +1,108 @@
+import { Scope } from '@sentry/node';
+import type { ServerLoad } from '@sveltejs/kit';
+import { error } from '@sveltejs/kit';
+import { vi } from 'vitest';
+
+import { wrapLoadWithSentry } from '../../src/server/load';
+
+const mockCaptureException = vi.fn();
+let mockScope = new Scope();
+
+vi.mock('@sentry/node', async () => {
+  const original = (await vi.importActual('@sentry/node')) as any;
+  return {
+    ...original,
+    captureException: (err: unknown, cb: (arg0: unknown) => unknown) => {
+      cb(mockScope);
+      mockCaptureException(err, cb);
+      return original.captureException(err, cb);
+    },
+  };
+});
+
+const mockAddExceptionMechanism = vi.fn();
+
+vi.mock('@sentry/utils', async () => {
+  const original = (await vi.importActual('@sentry/utils')) as any;
+  return {
+    ...original,
+    addExceptionMechanism: (...args: unknown[]) => mockAddExceptionMechanism(...args),
+  };
+});
+
+function getById(_id?: string) {
+  throw new Error('error');
+}
+
+describe('wrapLoadWithSentry', () => {
+  beforeEach(() => {
+    mockCaptureException.mockClear();
+    mockAddExceptionMechanism.mockClear();
+    mockScope = new Scope();
+  });
+
+  it('calls captureException', async () => {
+    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 = vi.spyOn(mockScope, 'addEventProcessor').mockImplementationOnce(callback => {
+      void callback({}, { event_id: 'fake-event-id' });
+      return mockScope;
+    });
+
+    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(addEventProcessorSpy).toBeCalledTimes(1);
+    expect(mockAddExceptionMechanism).toBeCalledTimes(1);
+    expect(mockAddExceptionMechanism).toBeCalledWith(
+      {},
+      { handled: false, type: 'sveltekit', data: { function: 'load' } },
+    );
+  });
+});