Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(sveltekit): Log error to console by default in handleErrorWithSentry #7674

Merged
merged 3 commits into from
Mar 30, 2023
Merged

fix(sveltekit): Log error to console by default in handleErrorWithSentry #7674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cb3b6e5
fix(sveltekit): Log console to error by default in `handleErrorWithSe…
Lms24 Mar 30, 2023
5afd8cc
add server-side fallback
Lms24 Mar 30, 2023
0ecac4e
fix typo
Lms24 Mar 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 10 additions & 4 deletions packages/sveltekit/src/client/handleError.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<HandleClientError>[0]): ReturnType<HandleClientError> {
// 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<HandleClientError> => {
captureException(input.error, scope => {
scope.addEventProcessor(event => {
Expand All @@ -23,8 +30,7 @@ export function handleErrorWithSentry(handleError?: HandleClientError): HandleCl
});
return scope;
});
if (handleError) {
return handleError(input);
}

return handleError(input);
};
}
15 changes: 11 additions & 4 deletions packages/sveltekit/src/server/handleError.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<HandleServerError>[0]): ReturnType<HandleServerError> {
// @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<HandleServerError> => {
captureException(input.error, scope => {
scope.addEventProcessor(event => {
Expand All @@ -23,8 +31,7 @@ export function handleErrorWithSentry(handleError?: HandleServerError): HandleSe
});
return scope;
});
if (handleError) {
return handleError(input);
}

return handleError(input);
};
}
39 changes: 24 additions & 15 deletions packages/sveltekit/test/client/handleError.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
39 changes: 24 additions & 15 deletions packages/sveltekit/test/server/handleError.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down