Skip to content

Commit 79ca4a7

Browse files
Lms24Lms24
authored
fix(sveltekit): Log error to console by default in handleErrorWithSentry (#7674)
Previously, our `handleErrorWithSentry` wrapper did nothing if users didn't provide a custom error handler as an argument. While IMO it's good that the error handler is an optional parameter, this swallowed errors and didn't log them to the console anymore. This patch fixes that by adding a default error handler which our wrapper invokes if no custom handler was provided. The client- and server default handlers log the error to the console, exactly like the default handlers by SvelteKit ([client](https://github.com/sveltejs/kit/blob/369e7d6851f543a40c947e033bfc4a9506fdc0a8/packages/kit/src/core/sync/write_client_manifest.js#LL127C2-L127C2), [server](https://github.com/sveltejs/kit/blob/369e7d6851f543a40c947e033bfc4a9506fdc0a8/packages/kit/src/runtime/server/index.js#L43)).
1 parent 016d3fc commit 79ca4a7

File tree

4 files changed

+69
-38
lines changed

4 files changed

+69
-38
lines changed

‎packages/sveltekit/src/client/handleError.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,19 @@ import { addExceptionMechanism } from '@sentry/utils';
66
// eslint-disable-next-line import/no-unresolved
77
import type { HandleClientError, NavigationEvent } from '@sveltejs/kit';
88

9+
// The SvelteKit default error handler just logs the error to the console
10+
// see: https://github.com/sveltejs/kit/blob/369e7d6851f543a40c947e033bfc4a9506fdc0a8/packages/kit/src/core/sync/write_client_manifest.js#LL127C2-L127C2
11+
function defaultErrorHandler({ error }: Parameters<HandleClientError>[0]): ReturnType<HandleClientError> {
12+
// eslint-disable-next-line no-console
13+
console.error(error);
14+
}
15+
916
/**
1017
* Wrapper for the SvelteKit error handler that sends the error to Sentry.
1118
*
1219
* @param handleError The original SvelteKit error handler.
1320
*/
14-
export function handleErrorWithSentry(handleError?: HandleClientError): HandleClientError {
21+
export function handleErrorWithSentry(handleError: HandleClientError = defaultErrorHandler): HandleClientError {
1522
return (input: { error: unknown; event: NavigationEvent }): ReturnType<HandleClientError> => {
1623
captureException(input.error, scope => {
1724
scope.addEventProcessor(event => {
@@ -23,8 +30,7 @@ export function handleErrorWithSentry(handleError?: HandleClientError): HandleCl
2330
});
2431
return scope;
2532
});
26-
if (handleError) {
27-
return handleError(input);
28-
}
33+
34+
return handleError(input);
2935
};
3036
}

‎packages/sveltekit/src/server/handleError.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,20 @@ import { addExceptionMechanism } from '@sentry/utils';
66
// eslint-disable-next-line import/no-unresolved
77
import type { HandleServerError, RequestEvent } from '@sveltejs/kit';
88

9+
// The SvelteKit default error handler just logs the error's stack trace to the console
10+
// see: https://github.com/sveltejs/kit/blob/369e7d6851f543a40c947e033bfc4a9506fdc0a8/packages/kit/src/runtime/server/index.js#L43
11+
function defaultErrorHandler({ error }: Parameters<HandleServerError>[0]): ReturnType<HandleServerError> {
12+
// @ts-expect-error this conforms to the default implementation (including this ts-expect-error)
13+
// eslint-disable-next-line no-console
14+
console.error(error && error.stack);
15+
}
16+
917
/**
1018
* Wrapper for the SvelteKit error handler that sends the error to Sentry.
1119
*
1220
* @param handleError The original SvelteKit error handler.
1321
*/
14-
export function handleErrorWithSentry(handleError?: HandleServerError): HandleServerError {
22+
export function handleErrorWithSentry(handleError: HandleServerError = defaultErrorHandler): HandleServerError {
1523
return (input: { error: unknown; event: RequestEvent }): ReturnType<HandleServerError> => {
1624
captureException(input.error, scope => {
1725
scope.addEventProcessor(event => {
@@ -23,8 +31,7 @@ export function handleErrorWithSentry(handleError?: HandleServerError): HandleSe
2331
});
2432
return scope;
2533
});
26-
if (handleError) {
27-
return handleError(input);
28-
}
34+
35+
return handleError(input);
2936
};
3037
}

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

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -45,31 +45,40 @@ const navigationEvent: NavigationEvent = {
4545
url: new URL('http://example.org/users/123'),
4646
};
4747

48+
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(_ => {});
49+
4850
describe('handleError', () => {
4951
beforeEach(() => {
5052
mockCaptureException.mockClear();
5153
mockAddExceptionMechanism.mockClear();
54+
consoleErrorSpy.mockClear();
5255
mockScope = new Scope();
5356
});
5457

55-
it('works when a handleError func is not provided', async () => {
56-
const wrappedHandleError = handleErrorWithSentry();
57-
const mockError = new Error('test');
58-
const returnVal = await wrappedHandleError({ error: mockError, event: navigationEvent });
58+
describe('calls captureException', () => {
59+
it('invokes the default handler if no handleError func is provided', async () => {
60+
const wrappedHandleError = handleErrorWithSentry();
61+
const mockError = new Error('test');
62+
const returnVal = await wrappedHandleError({ error: mockError, event: navigationEvent });
5963

60-
expect(returnVal).not.toBeDefined();
61-
expect(mockCaptureException).toHaveBeenCalledTimes(1);
62-
expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function));
63-
});
64+
expect(returnVal).not.toBeDefined();
65+
expect(mockCaptureException).toHaveBeenCalledTimes(1);
66+
expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function));
67+
// The default handler logs the error to the console
68+
expect(consoleErrorSpy).toHaveBeenCalledTimes(1);
69+
});
6470

65-
it('calls captureException', async () => {
66-
const wrappedHandleError = handleErrorWithSentry(handleError);
67-
const mockError = new Error('test');
68-
const returnVal = (await wrappedHandleError({ error: mockError, event: navigationEvent })) as any;
71+
it('invokes the user-provided error handler', async () => {
72+
const wrappedHandleError = handleErrorWithSentry(handleError);
73+
const mockError = new Error('test');
74+
const returnVal = (await wrappedHandleError({ error: mockError, event: navigationEvent })) as any;
6975

70-
expect(returnVal.message).toEqual('Whoops!');
71-
expect(mockCaptureException).toHaveBeenCalledTimes(1);
72-
expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function));
76+
expect(returnVal.message).toEqual('Whoops!');
77+
expect(mockCaptureException).toHaveBeenCalledTimes(1);
78+
expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function));
79+
// Check that the default handler wasn't invoked
80+
expect(consoleErrorSpy).toHaveBeenCalledTimes(0);
81+
});
7382
});
7483

7584
it('adds an exception mechanism', async () => {

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

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,31 +37,40 @@ function handleError(_input: { error: unknown; event: RequestEvent }): ReturnTyp
3737

3838
const requestEvent = {} as RequestEvent;
3939

40+
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(_ => {});
41+
4042
describe('handleError', () => {
4143
beforeEach(() => {
4244
mockCaptureException.mockClear();
4345
mockAddExceptionMechanism.mockClear();
46+
consoleErrorSpy.mockClear();
4447
mockScope = new Scope();
4548
});
4649

47-
it('works when a handleError func is not provided', async () => {
48-
const wrappedHandleError = handleErrorWithSentry();
49-
const mockError = new Error('test');
50-
const returnVal = await wrappedHandleError({ error: mockError, event: requestEvent });
50+
describe('calls captureException', () => {
51+
it('invokes the default handler if no handleError func is provided', async () => {
52+
const wrappedHandleError = handleErrorWithSentry();
53+
const mockError = new Error('test');
54+
const returnVal = await wrappedHandleError({ error: mockError, event: requestEvent });
5155

52-
expect(returnVal).not.toBeDefined();
53-
expect(mockCaptureException).toHaveBeenCalledTimes(1);
54-
expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function));
55-
});
56+
expect(returnVal).not.toBeDefined();
57+
expect(mockCaptureException).toHaveBeenCalledTimes(1);
58+
expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function));
59+
// The default handler logs the error to the console
60+
expect(consoleErrorSpy).toHaveBeenCalledTimes(1);
61+
});
5662

57-
it('calls captureException', async () => {
58-
const wrappedHandleError = handleErrorWithSentry(handleError);
59-
const mockError = new Error('test');
60-
const returnVal = (await wrappedHandleError({ error: mockError, event: requestEvent })) as any;
63+
it('invokes the user-provided error handler', async () => {
64+
const wrappedHandleError = handleErrorWithSentry(handleError);
65+
const mockError = new Error('test');
66+
const returnVal = (await wrappedHandleError({ error: mockError, event: requestEvent })) as any;
6167

62-
expect(returnVal.message).toEqual('Whoops!');
63-
expect(mockCaptureException).toHaveBeenCalledTimes(1);
64-
expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function));
68+
expect(returnVal.message).toEqual('Whoops!');
69+
expect(mockCaptureException).toHaveBeenCalledTimes(1);
70+
expect(mockCaptureException).toHaveBeenCalledWith(mockError, expect.any(Function));
71+
// Check that the default handler wasn't invoked
72+
expect(consoleErrorSpy).toHaveBeenCalledTimes(0);
73+
});
6574
});
6675

6776
it('adds an exception mechanism', async () => {

0 commit comments

Comments
 (0)