diff --git a/packages/sveltekit/src/client/handleError.ts b/packages/sveltekit/src/client/handleError.ts index 5490a3e20980..5b24c2ef9357 100644 --- a/packages/sveltekit/src/client/handleError.ts +++ b/packages/sveltekit/src/client/handleError.ts @@ -6,12 +6,19 @@ import { addExceptionMechanism } from '@sentry/utils'; // eslint-disable-next-line import/no-unresolved import type { HandleClientError, NavigationEvent } from '@sveltejs/kit'; +// The SvelteKit default error handler just logs the error to the console +// see: https://github.com/sveltejs/kit/blob/369e7d6851f543a40c947e033bfc4a9506fdc0a8/packages/kit/src/core/sync/write_client_manifest.js#LL127C2-L127C2 +function defaultErrorHandler({ error }: Parameters[0]): ReturnType { + // eslint-disable-next-line no-console + console.error(error); +} + /** * Wrapper for the SvelteKit error handler that sends the error to Sentry. * * @param handleError The original SvelteKit error handler. */ -export function handleErrorWithSentry(handleError?: HandleClientError): HandleClientError { +export function handleErrorWithSentry(handleError: HandleClientError = defaultErrorHandler): HandleClientError { return (input: { error: unknown; event: NavigationEvent }): ReturnType => { captureException(input.error, scope => { scope.addEventProcessor(event => { @@ -23,8 +30,7 @@ export function handleErrorWithSentry(handleError?: HandleClientError): HandleCl }); return scope; }); - if (handleError) { - return handleError(input); - } + + return handleError(input); }; } diff --git a/packages/sveltekit/src/server/handleError.ts b/packages/sveltekit/src/server/handleError.ts index 449ca65e19ce..4d886fc80224 100644 --- a/packages/sveltekit/src/server/handleError.ts +++ b/packages/sveltekit/src/server/handleError.ts @@ -6,12 +6,20 @@ import { addExceptionMechanism } from '@sentry/utils'; // eslint-disable-next-line import/no-unresolved import type { HandleServerError, RequestEvent } from '@sveltejs/kit'; +// The SvelteKit default error handler just logs the error's stack trace to the console +// see: https://github.com/sveltejs/kit/blob/369e7d6851f543a40c947e033bfc4a9506fdc0a8/packages/kit/src/runtime/server/index.js#L43 +function defaultErrorHandler({ error }: Parameters[0]): ReturnType { + // @ts-expect-error this conforms to the default implementation (including this ts-expect-error) + // eslint-disable-next-line no-console + console.error(error && error.stack); +} + /** * Wrapper for the SvelteKit error handler that sends the error to Sentry. * * @param handleError The original SvelteKit error handler. */ -export function handleErrorWithSentry(handleError?: HandleServerError): HandleServerError { +export function handleErrorWithSentry(handleError: HandleServerError = defaultErrorHandler): HandleServerError { return (input: { error: unknown; event: RequestEvent }): ReturnType => { captureException(input.error, scope => { scope.addEventProcessor(event => { @@ -23,8 +31,7 @@ export function handleErrorWithSentry(handleError?: HandleServerError): HandleSe }); return scope; }); - if (handleError) { - return handleError(input); - } + + return handleError(input); }; } diff --git a/packages/sveltekit/test/client/handleError.test.ts b/packages/sveltekit/test/client/handleError.test.ts index 170322555308..4dc2e6658af5 100644 --- a/packages/sveltekit/test/client/handleError.test.ts +++ b/packages/sveltekit/test/client/handleError.test.ts @@ -45,31 +45,40 @@ const navigationEvent: NavigationEvent = { url: new URL('http://example.org/users/123'), }; +const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(_ => {}); + describe('handleError', () => { beforeEach(() => { mockCaptureException.mockClear(); mockAddExceptionMechanism.mockClear(); + consoleErrorSpy.mockClear(); mockScope = new Scope(); }); - it('works when a handleError func is not provided', async () => { - const wrappedHandleError = handleErrorWithSentry(); - const mockError = new Error('test'); - const returnVal = await wrappedHandleError({ error: mockError, event: navigationEvent }); + describe('calls captureException', () => { + it('invokes the default handler if no handleError func is provided', async () => { + const wrappedHandleError = handleErrorWithSentry(); + 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)); - }); + expect(returnVal).not.toBeDefined(); + expect(mockCaptureException).toHaveBeenCalledTimes(1); + expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function)); + // The default handler logs the error to the console + expect(consoleErrorSpy).toHaveBeenCalledTimes(1); + }); - it('calls captureException', async () => { - const wrappedHandleError = handleErrorWithSentry(handleError); - const mockError = new Error('test'); - const returnVal = (await wrappedHandleError({ error: mockError, event: navigationEvent })) as any; + it('invokes the user-provided error handler', async () => { + const wrappedHandleError = handleErrorWithSentry(handleError); + const mockError = new Error('test'); + const returnVal = (await wrappedHandleError({ error: mockError, event: navigationEvent })) as any; - expect(returnVal.message).toEqual('Whoops!'); - expect(mockCaptureException).toHaveBeenCalledTimes(1); - expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function)); + expect(returnVal.message).toEqual('Whoops!'); + expect(mockCaptureException).toHaveBeenCalledTimes(1); + expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function)); + // Check that the default handler wasn't invoked + expect(consoleErrorSpy).toHaveBeenCalledTimes(0); + }); }); it('adds an exception mechanism', async () => { diff --git a/packages/sveltekit/test/server/handleError.test.ts b/packages/sveltekit/test/server/handleError.test.ts index afe7f5cbe6df..e44d5553f928 100644 --- a/packages/sveltekit/test/server/handleError.test.ts +++ b/packages/sveltekit/test/server/handleError.test.ts @@ -37,31 +37,40 @@ function handleError(_input: { error: unknown; event: RequestEvent }): ReturnTyp const requestEvent = {} as RequestEvent; +const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(_ => {}); + describe('handleError', () => { beforeEach(() => { mockCaptureException.mockClear(); mockAddExceptionMechanism.mockClear(); + consoleErrorSpy.mockClear(); mockScope = new Scope(); }); - it('works when a handleError func is not provided', async () => { - const wrappedHandleError = handleErrorWithSentry(); - const mockError = new Error('test'); - const returnVal = await wrappedHandleError({ error: mockError, event: requestEvent }); + describe('calls captureException', () => { + it('invokes the default handler if no handleError func is provided', async () => { + const wrappedHandleError = handleErrorWithSentry(); + 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)); - }); + expect(returnVal).not.toBeDefined(); + expect(mockCaptureException).toHaveBeenCalledTimes(1); + expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function)); + // The default handler logs the error to the console + expect(consoleErrorSpy).toHaveBeenCalledTimes(1); + }); - it('calls captureException', async () => { - const wrappedHandleError = handleErrorWithSentry(handleError); - const mockError = new Error('test'); - const returnVal = (await wrappedHandleError({ error: mockError, event: requestEvent })) as any; + it('invokes the user-provided error handler', async () => { + const wrappedHandleError = handleErrorWithSentry(handleError); + const mockError = new Error('test'); + const returnVal = (await wrappedHandleError({ error: mockError, event: requestEvent })) as any; - expect(returnVal.message).toEqual('Whoops!'); - expect(mockCaptureException).toHaveBeenCalledTimes(1); - expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function)); + expect(returnVal.message).toEqual('Whoops!'); + expect(mockCaptureException).toHaveBeenCalledTimes(1); + expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function)); + // Check that the default handler wasn't invoked + expect(consoleErrorSpy).toHaveBeenCalledTimes(0); + }); }); it('adds an exception mechanism', async () => {