From d21dd18bf23998e9cadf097c97a9377270946102 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 10 Mar 2023 15:15:13 +0100 Subject: [PATCH 1/3] feat(sveltekit): Add server-side handleError wrapper --- packages/sveltekit/src/client/handleError.ts | 2 +- packages/sveltekit/src/client/index.ts | 2 +- packages/sveltekit/src/index.types.ts | 6 ++ packages/sveltekit/src/server/handleError.ts | 28 +++++++ packages/sveltekit/src/server/index.ts | 1 + .../sveltekit/test/client/handleError.test.ts | 6 +- .../sveltekit/test/server/handleError.test.ts | 74 +++++++++++++++++++ 7 files changed, 114 insertions(+), 5 deletions(-) create mode 100644 packages/sveltekit/src/server/handleError.ts create mode 100644 packages/sveltekit/test/server/handleError.test.ts diff --git a/packages/sveltekit/src/client/handleError.ts b/packages/sveltekit/src/client/handleError.ts index a4c20b895537..5c6fd649fe17 100644 --- a/packages/sveltekit/src/client/handleError.ts +++ b/packages/sveltekit/src/client/handleError.ts @@ -11,7 +11,7 @@ import type { HandleClientError, NavigationEvent } from '@sveltejs/kit'; * * @param handleError The original SvelteKit error handler. */ -export function wrapHandleError(handleError: HandleClientError): HandleClientError { +export function wrapHandleErrorWithSentry(handleError: HandleClientError): HandleClientError { return (input: { error: unknown; event: NavigationEvent }): ReturnType => { captureException(input.error, scope => { scope.addEventProcessor(event => { diff --git a/packages/sveltekit/src/client/index.ts b/packages/sveltekit/src/client/index.ts index e49e83c9ce96..693cf829ce30 100644 --- a/packages/sveltekit/src/client/index.ts +++ b/packages/sveltekit/src/client/index.ts @@ -1,7 +1,7 @@ export * from '@sentry/svelte'; export { init } from './sdk'; -export { wrapHandleError } from './handleError'; +export { wrapHandleErrorWithSentry } from './handleError'; // Just here so that eslint is happy until we export more stuff here export const PLACEHOLDER_CLIENT = 'PLACEHOLDER'; diff --git a/packages/sveltekit/src/index.types.ts b/packages/sveltekit/src/index.types.ts index d4c7a9e52c25..ec408505c063 100644 --- a/packages/sveltekit/src/index.types.ts +++ b/packages/sveltekit/src/index.types.ts @@ -8,6 +8,8 @@ export * from './config'; export * from './server'; import type { Integration, Options, StackParser } from '@sentry/types'; +// eslint-disable-next-line import/no-unresolved +import type { HandleClientError, HandleServerError } from '@sveltejs/kit'; import type * as clientSdk from './client'; import type * as serverSdk from './server'; @@ -15,6 +17,10 @@ import type * as serverSdk from './server'; /** Initializes Sentry SvelteKit SDK */ export declare function init(options: Options | clientSdk.BrowserOptions | serverSdk.NodeOptions): void; +export declare function wrapHandleErrorWithSentry( + handleError: T, +): ReturnType; + // We export a merged Integrations object so that users can (at least typing-wise) use all integrations everywhere. export declare const Integrations: typeof clientSdk.Integrations & typeof serverSdk.Integrations; diff --git a/packages/sveltekit/src/server/handleError.ts b/packages/sveltekit/src/server/handleError.ts new file mode 100644 index 000000000000..ca09e173dbf4 --- /dev/null +++ b/packages/sveltekit/src/server/handleError.ts @@ -0,0 +1,28 @@ +import { captureException } from '@sentry/node'; +import { addExceptionMechanism } from '@sentry/utils'; +// 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'; + +/** + * Wrapper for the SvelteKit error handler that sends the error to Sentry. + * + * @param handleError The original SvelteKit error handler. + */ +export function wrapHandleErrorWithSentry(handleError: HandleServerError): HandleServerError { + return (input: { error: unknown; event: RequestEvent }): ReturnType => { + captureException(input.error, scope => { + scope.addEventProcessor(event => { + addExceptionMechanism(event, { + type: 'sveltekit', + handled: false, + }); + return event; + }); + return scope; + }); + return handleError(input); + }; +} diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index 6ac8d97b4241..5dd114d9f1fb 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -1,3 +1,4 @@ export * from '@sentry/node'; export { init } from './sdk'; +export { wrapHandleErrorWithSentry } from './handleError'; diff --git a/packages/sveltekit/test/client/handleError.test.ts b/packages/sveltekit/test/client/handleError.test.ts index a075ac611c1e..bf58675ada09 100644 --- a/packages/sveltekit/test/client/handleError.test.ts +++ b/packages/sveltekit/test/client/handleError.test.ts @@ -5,7 +5,7 @@ import { Scope } from '@sentry/svelte'; // eslint-disable-next-line import/no-unresolved import type { HandleClientError, NavigationEvent } from '@sveltejs/kit'; -import { wrapHandleError } from '../../src/client/handleError'; +import { wrapHandleErrorWithSentry } from '../../src/client/handleError'; const mockCaptureException = jest.fn(); let mockScope = new Scope(); @@ -56,7 +56,7 @@ describe('handleError', () => { }); it('calls captureException', async () => { - const wrappedHandleError = wrapHandleError(handleError); + const wrappedHandleError = wrapHandleErrorWithSentry(handleError); const mockError = new Error('test'); const returnVal = await wrappedHandleError({ error: mockError, event: navigationEvent }); @@ -71,7 +71,7 @@ describe('handleError', () => { return mockScope; }); - const wrappedHandleError = wrapHandleError(handleError); + const wrappedHandleError = wrapHandleErrorWithSentry(handleError); const mockError = new Error('test'); await wrappedHandleError({ error: mockError, event: navigationEvent }); diff --git a/packages/sveltekit/test/server/handleError.test.ts b/packages/sveltekit/test/server/handleError.test.ts new file mode 100644 index 000000000000..60cdad856601 --- /dev/null +++ b/packages/sveltekit/test/server/handleError.test.ts @@ -0,0 +1,74 @@ +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 { wrapHandleErrorWithSentry } from '../../src/server/handleError'; + +const mockCaptureException = jest.fn(); +let mockScope = new Scope(); + +jest.mock('@sentry/node', () => { + const original = jest.requireActual('@sentry/core'); + return { + ...original, + captureException: (err: unknown, cb: (arg0: unknown) => unknown) => { + cb(mockScope); + mockCaptureException(err, cb); + return original.captureException(err, cb); + }, + }; +}); + +const mockAddExceptionMechanism = jest.fn(); + +jest.mock('@sentry/utils', () => { + const original = jest.requireActual('@sentry/utils'); + return { + ...original, + addExceptionMechanism: (...args: unknown[]) => mockAddExceptionMechanism(...args), + }; +}); + +function handleError(_input: { error: unknown; event: RequestEvent }): ReturnType { + return { + message: 'Whoops!', + }; +} + +const requestEvent = {} as RequestEvent; + +describe('handleError', () => { + beforeEach(() => { + mockCaptureException.mockClear(); + mockAddExceptionMechanism.mockClear(); + mockScope = new Scope(); + }); + + it('calls captureException', async () => { + const wrappedHandleError = wrapHandleErrorWithSentry(handleError); + const mockError = new Error('test'); + const returnVal = await wrappedHandleError({ error: mockError, event: requestEvent }); + + expect(returnVal!.message).toEqual('Whoops!'); + expect(mockCaptureException).toHaveBeenCalledTimes(1); + expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function)); + }); + + it('adds an exception mechanism', async () => { + const addEventProcessorSpy = jest.spyOn(mockScope, 'addEventProcessor').mockImplementationOnce(callback => { + void callback({}, { event_id: 'fake-event-id' }); + return mockScope; + }); + + const wrappedHandleError = wrapHandleErrorWithSentry(handleError); + const mockError = new Error('test'); + await wrappedHandleError({ error: mockError, event: requestEvent }); + + expect(addEventProcessorSpy).toBeCalledTimes(1); + expect(mockAddExceptionMechanism).toBeCalledTimes(1); + expect(mockAddExceptionMechanism).toBeCalledWith({}, { handled: false, type: 'sveltekit' }); + }); +}); From e9d89040602744cf0add176a47e47f42e82b746d Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 10 Mar 2023 15:23:59 +0100 Subject: [PATCH 2/3] rename to `handleErrorWithSentry` --- packages/sveltekit/src/client/handleError.ts | 6 ++++-- packages/sveltekit/src/client/index.ts | 2 +- packages/sveltekit/src/index.types.ts | 2 +- packages/sveltekit/src/server/handleError.ts | 6 ++++-- packages/sveltekit/src/server/index.ts | 2 +- packages/sveltekit/test/client/handleError.test.ts | 10 ++++++++++ packages/sveltekit/test/server/handleError.test.ts | 10 ++++++++++ 7 files changed, 31 insertions(+), 7 deletions(-) diff --git a/packages/sveltekit/src/client/handleError.ts b/packages/sveltekit/src/client/handleError.ts index 5c6fd649fe17..5490a3e20980 100644 --- a/packages/sveltekit/src/client/handleError.ts +++ b/packages/sveltekit/src/client/handleError.ts @@ -11,7 +11,7 @@ import type { HandleClientError, NavigationEvent } from '@sveltejs/kit'; * * @param handleError The original SvelteKit error handler. */ -export function wrapHandleErrorWithSentry(handleError: HandleClientError): HandleClientError { +export function handleErrorWithSentry(handleError?: HandleClientError): HandleClientError { return (input: { error: unknown; event: NavigationEvent }): ReturnType => { captureException(input.error, scope => { scope.addEventProcessor(event => { @@ -23,6 +23,8 @@ export function wrapHandleErrorWithSentry(handleError: HandleClientError): Handl }); return scope; }); - return handleError(input); + if (handleError) { + return handleError(input); + } }; } diff --git a/packages/sveltekit/src/client/index.ts b/packages/sveltekit/src/client/index.ts index 693cf829ce30..dc6ad3407264 100644 --- a/packages/sveltekit/src/client/index.ts +++ b/packages/sveltekit/src/client/index.ts @@ -1,7 +1,7 @@ export * from '@sentry/svelte'; export { init } from './sdk'; -export { wrapHandleErrorWithSentry } from './handleError'; +export { handleErrorWithSentry } from './handleError'; // Just here so that eslint is happy until we export more stuff here export const PLACEHOLDER_CLIENT = 'PLACEHOLDER'; diff --git a/packages/sveltekit/src/index.types.ts b/packages/sveltekit/src/index.types.ts index ec408505c063..06ff806a8377 100644 --- a/packages/sveltekit/src/index.types.ts +++ b/packages/sveltekit/src/index.types.ts @@ -17,7 +17,7 @@ import type * as serverSdk from './server'; /** Initializes Sentry SvelteKit SDK */ export declare function init(options: Options | clientSdk.BrowserOptions | serverSdk.NodeOptions): void; -export declare function wrapHandleErrorWithSentry( +export declare function handleErrorWithSentry( handleError: T, ): ReturnType; diff --git a/packages/sveltekit/src/server/handleError.ts b/packages/sveltekit/src/server/handleError.ts index ca09e173dbf4..449ca65e19ce 100644 --- a/packages/sveltekit/src/server/handleError.ts +++ b/packages/sveltekit/src/server/handleError.ts @@ -11,7 +11,7 @@ import type { HandleServerError, RequestEvent } from '@sveltejs/kit'; * * @param handleError The original SvelteKit error handler. */ -export function wrapHandleErrorWithSentry(handleError: HandleServerError): HandleServerError { +export function handleErrorWithSentry(handleError?: HandleServerError): HandleServerError { return (input: { error: unknown; event: RequestEvent }): ReturnType => { captureException(input.error, scope => { scope.addEventProcessor(event => { @@ -23,6 +23,8 @@ export function wrapHandleErrorWithSentry(handleError: HandleServerError): Handl }); return scope; }); - return handleError(input); + if (handleError) { + return handleError(input); + } }; } diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index 5dd114d9f1fb..1e3aed034f1e 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -1,4 +1,4 @@ export * from '@sentry/node'; export { init } from './sdk'; -export { wrapHandleErrorWithSentry } from './handleError'; +export { handleErrorWithSentry } from './handleError'; diff --git a/packages/sveltekit/test/client/handleError.test.ts b/packages/sveltekit/test/client/handleError.test.ts index bf58675ada09..6f224902c7de 100644 --- a/packages/sveltekit/test/client/handleError.test.ts +++ b/packages/sveltekit/test/client/handleError.test.ts @@ -55,6 +55,16 @@ describe('handleError', () => { mockScope = new Scope(); }); + it('works when a handleError func is not provided', async () => { + const wrappedHandleError = wrapHandleErrorWithSentry(); + const mockError = new Error('test'); + const returnVal = await wrappedHandleError({ error: mockError, event: navigationEvent }); + + expect(returnVal).not.toBeDefined(); + expect(mockCaptureException).toHaveBeenCalledTimes(1); + expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function)); + }); + it('calls captureException', async () => { const wrappedHandleError = wrapHandleErrorWithSentry(handleError); const mockError = new Error('test'); diff --git a/packages/sveltekit/test/server/handleError.test.ts b/packages/sveltekit/test/server/handleError.test.ts index 60cdad856601..7e7e0467c353 100644 --- a/packages/sveltekit/test/server/handleError.test.ts +++ b/packages/sveltekit/test/server/handleError.test.ts @@ -47,6 +47,16 @@ describe('handleError', () => { mockScope = new Scope(); }); + it('works when a handleError func is not provided', async () => { + const wrappedHandleError = wrapHandleErrorWithSentry(); + const mockError = new Error('test'); + const returnVal = await wrappedHandleError({ error: mockError, event: requestEvent }); + + expect(returnVal).not.toBeDefined(); + expect(mockCaptureException).toHaveBeenCalledTimes(1); + expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function)); + }); + it('calls captureException', async () => { const wrappedHandleError = wrapHandleErrorWithSentry(handleError); const mockError = new Error('test'); From 2adf5d30577a3748b35b590c9e7844a458196f88 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 10 Mar 2023 15:32:11 +0100 Subject: [PATCH 3/3] rename --- packages/sveltekit/test/client/handleError.test.ts | 8 ++++---- packages/sveltekit/test/server/handleError.test.ts | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/sveltekit/test/client/handleError.test.ts b/packages/sveltekit/test/client/handleError.test.ts index 6f224902c7de..49bcb6a0c57f 100644 --- a/packages/sveltekit/test/client/handleError.test.ts +++ b/packages/sveltekit/test/client/handleError.test.ts @@ -5,7 +5,7 @@ import { Scope } from '@sentry/svelte'; // eslint-disable-next-line import/no-unresolved import type { HandleClientError, NavigationEvent } from '@sveltejs/kit'; -import { wrapHandleErrorWithSentry } from '../../src/client/handleError'; +import { handleErrorWithSentry } from '../../src/client/handleError'; const mockCaptureException = jest.fn(); let mockScope = new Scope(); @@ -56,7 +56,7 @@ describe('handleError', () => { }); it('works when a handleError func is not provided', async () => { - const wrappedHandleError = wrapHandleErrorWithSentry(); + const wrappedHandleError = handleErrorWithSentry(); const mockError = new Error('test'); const returnVal = await wrappedHandleError({ error: mockError, event: navigationEvent }); @@ -66,7 +66,7 @@ describe('handleError', () => { }); it('calls captureException', async () => { - const wrappedHandleError = wrapHandleErrorWithSentry(handleError); + const wrappedHandleError = handleErrorWithSentry(handleError); const mockError = new Error('test'); const returnVal = await wrappedHandleError({ error: mockError, event: navigationEvent }); @@ -81,7 +81,7 @@ describe('handleError', () => { return mockScope; }); - const wrappedHandleError = wrapHandleErrorWithSentry(handleError); + const wrappedHandleError = handleErrorWithSentry(handleError); const mockError = new Error('test'); await wrappedHandleError({ error: mockError, event: navigationEvent }); diff --git a/packages/sveltekit/test/server/handleError.test.ts b/packages/sveltekit/test/server/handleError.test.ts index 7e7e0467c353..41e587dfa94c 100644 --- a/packages/sveltekit/test/server/handleError.test.ts +++ b/packages/sveltekit/test/server/handleError.test.ts @@ -5,7 +5,7 @@ import { Scope } from '@sentry/node'; // eslint-disable-next-line import/no-unresolved import type { HandleServerError, RequestEvent } from '@sveltejs/kit'; -import { wrapHandleErrorWithSentry } from '../../src/server/handleError'; +import { handleErrorWithSentry } from '../../src/server/handleError'; const mockCaptureException = jest.fn(); let mockScope = new Scope(); @@ -48,7 +48,7 @@ describe('handleError', () => { }); it('works when a handleError func is not provided', async () => { - const wrappedHandleError = wrapHandleErrorWithSentry(); + const wrappedHandleError = handleErrorWithSentry(); const mockError = new Error('test'); const returnVal = await wrappedHandleError({ error: mockError, event: requestEvent }); @@ -58,7 +58,7 @@ describe('handleError', () => { }); it('calls captureException', async () => { - const wrappedHandleError = wrapHandleErrorWithSentry(handleError); + const wrappedHandleError = handleErrorWithSentry(handleError); const mockError = new Error('test'); const returnVal = await wrappedHandleError({ error: mockError, event: requestEvent }); @@ -73,7 +73,7 @@ describe('handleError', () => { return mockScope; }); - const wrappedHandleError = wrapHandleErrorWithSentry(handleError); + const wrappedHandleError = handleErrorWithSentry(handleError); const mockError = new Error('test'); await wrappedHandleError({ error: mockError, event: requestEvent });