diff --git a/packages/sveltekit/.eslintrc.js b/packages/sveltekit/.eslintrc.js index 2d614f46733b..8fcc9ae58f9a 100644 --- a/packages/sveltekit/.eslintrc.js +++ b/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'], }; diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index 1e3aed034f1e..c7784d870c56 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/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'; diff --git a/packages/sveltekit/src/server/load.ts b/packages/sveltekit/src/server/load.ts new file mode 100644 index 000000000000..ef0433091a9e --- /dev/null +++ b/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) => { + 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; + }, + }); +} diff --git a/packages/sveltekit/test/client/handleError.test.ts b/packages/sveltekit/test/client/handleError.test.ts index 6260d3dc4827..170322555308 100644 --- a/packages/sveltekit/test/client/handleError.test.ts +++ b/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'; diff --git a/packages/sveltekit/test/server/handleError.test.ts b/packages/sveltekit/test/server/handleError.test.ts index da73ca0eef5d..afe7f5cbe6df 100644 --- a/packages/sveltekit/test/server/handleError.test.ts +++ b/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) => { diff --git a/packages/sveltekit/test/server/load.test.ts b/packages/sveltekit/test/server/load.test.ts new file mode 100644 index 000000000000..ec2503b945c4 --- /dev/null +++ b/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[0]): Promise> { + 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[0]): Promise> { + 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[0]): Promise> { + 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' } }, + ); + }); +});