diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-3/src/routes/expected-error-4xx/+page.server.ts b/dev-packages/e2e-tests/test-applications/sveltekit-3/src/routes/expected-error-4xx/+page.server.ts new file mode 100644 index 000000000000..5ce63b13207e --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-3/src/routes/expected-error-4xx/+page.server.ts @@ -0,0 +1,7 @@ +import { error } from '@sveltejs/kit'; + +export const load = async () => { + // SvelteKit 3 passes expected errors to `handleError` as `kind: 'app'`. + // 4xx are expected, so the SDK must not capture them. + error(404, 'Expected 404 Error'); +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-3/src/routes/expected-error-4xx/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-3/src/routes/expected-error-4xx/+page.svelte new file mode 100644 index 000000000000..a16eaad1565d --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-3/src/routes/expected-error-4xx/+page.svelte @@ -0,0 +1 @@ +

Expected 4xx error

diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-3/src/routes/expected-error-5xx/+page.server.ts b/dev-packages/e2e-tests/test-applications/sveltekit-3/src/routes/expected-error-5xx/+page.server.ts new file mode 100644 index 000000000000..501fe7768d60 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-3/src/routes/expected-error-5xx/+page.server.ts @@ -0,0 +1,7 @@ +import { error } from '@sveltejs/kit'; + +export const load = async () => { + // SvelteKit 3 passes expected errors to `handleError` as `kind: 'app'`. + // 5xx are worth reporting, so the SDK captures them. + error(500, 'Expected 500 Error'); +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-3/src/routes/expected-error-5xx/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-3/src/routes/expected-error-5xx/+page.svelte new file mode 100644 index 000000000000..638d9577978b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-3/src/routes/expected-error-5xx/+page.svelte @@ -0,0 +1 @@ +

Expected 5xx error

diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-3/tests/errors.server.test.ts b/dev-packages/e2e-tests/test-applications/sveltekit-3/tests/errors.server.test.ts index 447f4fa07890..e00c5900e617 100644 --- a/dev-packages/e2e-tests/test-applications/sveltekit-3/tests/errors.server.test.ts +++ b/dev-packages/e2e-tests/test-applications/sveltekit-3/tests/errors.server.test.ts @@ -88,3 +88,48 @@ test.describe('server-side errors', () => { }); }); }); + +test.describe('expected errors thrown with `error()`', () => { + // SvelteKit 3 passes *every* error to `handleError`, discriminated by `kind` — including + // expected ones thrown with `error()`, which never reached the hook on SvelteKit 2. + // The SDK applies the same rule as everywhere else: 4xx are expected, 5xx are reported. + // + // These match on the request URL rather than the exception value: SvelteKit hands `handleError` + // the error *body* (a plain object), so the captured exception gets a synthesized message + // ("Object captured as exception with keys: ...") rather than the message passed to `error()`. + test("doesn't capture a 4xx error", async ({ page }) => { + let captured4xxError = false; + // Deliberately floating: this must never resolve, so it can't be awaited + void waitForError('sveltekit-3', errorEvent => { + return !!errorEvent?.request?.url?.endsWith('/expected-error-4xx'); + }).then(() => { + captured4xxError = true; + }); + + // The 5xx route *is* captured, so its error event is a concrete signal that the preceding + // 4xx request was fully processed - no sleeping on a timeout to prove a negative. + const signalErrorPromise = waitForError('sveltekit-3', errorEvent => { + return !!errorEvent?.request?.url?.endsWith('/expected-error-5xx'); + }); + + await page.goto('/expected-error-4xx'); + await page.goto('/expected-error-5xx'); + await signalErrorPromise; + + expect(captured4xxError).toBe(false); + }); + + test('captures a 5xx error', async ({ page }) => { + const errorEventPromise = waitForError('sveltekit-3', errorEvent => { + return !!errorEvent?.request?.url?.endsWith('/expected-error-5xx'); + }); + + await page.goto('/expected-error-5xx'); + + const errorEvent = await errorEventPromise; + + expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual( + expect.objectContaining({ type: 'auto.function.sveltekit.handle_error' }), + ); + }); +}); diff --git a/packages/sveltekit/src/client/handleError.ts b/packages/sveltekit/src/client/handleError.ts index f7869214d635..f48e44d35479 100644 --- a/packages/sveltekit/src/client/handleError.ts +++ b/packages/sveltekit/src/client/handleError.ts @@ -1,38 +1,41 @@ -import { isObjectLike, consoleSandbox } from '@sentry/core'; +import { consoleSandbox } from '@sentry/core'; import { captureException } from '@sentry/svelte'; -import type { HandleClientError } from '@sveltejs/kit'; +import type { AnyErrorHandler, SentryHandleClientErrorInput } from '../common/handleErrorTypes'; +import { getErrorStatus, shouldCaptureError } from '../common/handleErrorTypes'; + +type ClientErrorHandler = (input: SentryHandleClientErrorInput) => unknown; + +/** + * The default shape of the wrapped hook: structurally compatible with SvelteKit's + * `HandleClientError` on every supported major. + */ +type SentryHandleClientError = (input: SentryHandleClientErrorInput) => void | App.Error; + +// Mirrors SvelteKit's own default client error handler, which differs by major version: +// - SvelteKit 1.x/2.x log every error +// - SvelteKit 3 only logs unexpected errors +// see: https://github.com/sveltejs/kit/blob/49f0808f3e983d0cb5a4d586cf0d1678467431ed/packages/kit/src/core/sync/write_client_manifest.js#L157-L160 +function defaultErrorHandler({ kind, error }: SentryHandleClientErrorInput): void { + if (kind && kind !== 'unknown') { + return; + } -// 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 { consoleSandbox(() => { // eslint-disable-next-line no-console console.error(error); }); } -type HandleClientErrorInput = Parameters[0]; - -/** - * Backwards-compatible HandleServerError Input type for SvelteKit 1.x and 2.x - * `message` and `status` were added in 2.x. - * For backwards-compatibility, we make them optional - * - * @see https://kit.svelte.dev/docs/migrating-to-sveltekit-2#improved-error-handling - */ -type SafeHandleServerErrorInput = Omit & - Partial>; - /** * 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 { - const errorHandler = handleError ?? defaultErrorHandler; +export function handleErrorWithSentry(handleError?: T): T { + const errorHandler = (handleError ?? defaultErrorHandler) as ClientErrorHandler; - return (input: HandleClientErrorInput): ReturnType => { - if (is4xxError(input)) { + const sentryErrorHandler = (input: SentryHandleClientErrorInput): unknown => { + if (!shouldCaptureError(input, () => isExpectedLegacyError(input))) { return errorHandler(input); } @@ -45,10 +48,25 @@ export function handleErrorWithSentry(handleError?: HandleClientError): HandleCl return errorHandler(input); }; + + // Returning `T` (the caller's own hook type) is what keeps the result assignable to + // `HandleClientError` on both SvelteKit 2 and 3. The wrapper itself is written against our + // structural input type, which TS can't prove is identical to `T`, so it can't be narrowed + // without the double cast. + return sentryErrorHandler as unknown as T; } -// 4xx are expected errors and thus we don't want to capture them -function is4xxError(input: SafeHandleServerErrorInput): boolean { +/** + * Whether a SvelteKit 1.x/2.x error is an expected 4xx we don't want to capture. + * + * SvelteKit 3 errors are classified by `shouldCaptureError` instead. + */ +function isExpectedLegacyError(input: SentryHandleClientErrorInput): boolean { + if (input.kind) { + // Not a SvelteKit 1.x/2.x input - narrows the union so `status` below is readable + return false; + } + const { status } = input; if (status && status >= 400 && status < 500) { @@ -58,7 +76,7 @@ function is4xxError(input: SafeHandleServerErrorInput): boolean { // SvelteKit __data.json requests return HTTP 200 with errors embedded in JSON, // so get_status() may resolve to 500 for a deserialized plain error object. // Fall back to checking input.error.status directly. - const errorStatus = isObjectLike(input.error) ? (input.error as Record)['status'] : undefined; + const errorStatus = getErrorStatus(input.error); - return typeof errorStatus === 'number' && errorStatus >= 400 && errorStatus < 500; + return errorStatus !== undefined && errorStatus >= 400 && errorStatus < 500; } diff --git a/packages/sveltekit/src/common/handleErrorTypes.ts b/packages/sveltekit/src/common/handleErrorTypes.ts new file mode 100644 index 000000000000..0f761a5cb989 --- /dev/null +++ b/packages/sveltekit/src/common/handleErrorTypes.ts @@ -0,0 +1,119 @@ +/** + * Where an error passed to `handleError` came from. Added in SvelteKit 3; `undefined` on + * SvelteKit 1.x and 2.x. + * + * - `app`: thrown with the `error(...)` helper + * - `framework`: generated by SvelteKit itself (404s, 405s, 413s, ...) + * - `validation`: invalid remote function arguments (server only) + * - `unknown`: thrown by user code, or code it calls + * + * @see https://svelte.dev/docs/kit/hooks#handleError + */ +export type CaughtErrorKind = 'app' | 'framework' | 'validation' | 'unknown'; + +/** + * The `handleError` input as of SvelteKit 3, where errors are discriminated by `kind` and the + * status lives on the error instead of the input. + */ +export type CaughtErrorInput = { + kind: CaughtErrorKind; + error: unknown; + /** Only present for `kind: 'validation'` */ + issues?: unknown[]; +}; + +/** + * The `handleError` input on SvelteKit 1.x and 2.x, which had no `kind` and carried the status and + * message on the input itself. + * + * SvelteKit 3 keeps both alive in dev builds as deprecated getters that log a warning when read. + * Modelling the two shapes as a discriminated union is what stops us reading them on a SvelteKit 3 + * input: as far as the type system is concerned, `status` doesn't exist there. + */ +export type LegacyCaughtErrorInput = { + kind?: undefined; + error: unknown; + status?: number; + message?: string; +}; + +/** + * The input of a SvelteKit `handleError` hook, covering SvelteKit 1.x, 2.x and 3. + * + * We declare this structurally instead of importing SvelteKit's `HandleServerError`/ + * `HandleClientError`, because those types moved from `@sveltejs/kit` to `@sveltejs/kit/hooks` + * in SvelteKit 3 and neither import path type-checks against both majors. + */ +export type SentryHandleErrorInput = CaughtErrorInput | LegacyCaughtErrorInput; + +/** The `handleError` input on the server, where we also read from the request event. */ +export type SentryHandleServerErrorInput = SentryHandleErrorInput & { + event: { + route?: { id?: string | null }; + platform?: unknown; + }; +}; + +/** The `handleError` input on the client. */ +export type SentryHandleClientErrorInput = SentryHandleErrorInput & { + event: unknown; +}; + +/** + * Constrains the user-provided `handleError` hook without depending on SvelteKit's own types. + * `never` as the parameter type accepts any single-argument function (parameters are + * contravariant), so a SvelteKit 1.x, 2.x or 3 hook all satisfy it. + */ +export type AnyErrorHandler = (input: never) => unknown; + +/** + * Reads the HTTP status off an error. In SvelteKit 3, `app`, `framework` and `validation` errors + * all carry their status here. + */ +export function getErrorStatus(error: unknown): number | undefined { + if (error == null || typeof error !== 'object') { + return undefined; + } + + const { status } = error as { status?: unknown }; + + return typeof status === 'number' ? status : undefined; +} + +/** + * Whether an error passed to `handleError` should be sent to Sentry. + * + * @param isExpectedLegacyError checks whether a SvelteKit 1.x/2.x error is an expected one. Those + * versions have no `kind`, and what counts as expected differs between server and client. + */ +export function shouldCaptureError(input: SentryHandleErrorInput, isExpectedLegacyError: () => boolean): boolean { + if (input.kind) { + return shouldCaptureCaughtError(input); + } + + return !isExpectedLegacyError(); +} + +/** + * The SvelteKit 3+ rule. Every error reaches `handleError` there — including expected ones thrown + * with `error(...)` and framework errors like 404s, neither of which showed up here on SvelteKit 2. + * We apply the same rule the rest of the SDK uses for thrown `HttpError`s (see `sendErrorToSentry`): + * 4xx are expected and noisy, 5xx are worth reporting. + */ +function shouldCaptureCaughtError(input: CaughtErrorInput): boolean { + // Invalid remote function arguments are a caller mistake, not an app failure. SvelteKit always + // gives these a 400, but don't let that be the only reason we skip them. + if (input.kind === 'validation') { + return false; + } + + // Unexpected errors have no status of their own; SvelteKit reports them as 500s. + if (input.kind === 'unknown') { + return true; + } + + const status = getErrorStatus(input.error); + + // If we can't tell, err on the side of capturing. + return status === undefined || status >= 500; +} diff --git a/packages/sveltekit/src/index.types.ts b/packages/sveltekit/src/index.types.ts index ba597b6d40b4..e70bb94c5aed 100644 --- a/packages/sveltekit/src/index.types.ts +++ b/packages/sveltekit/src/index.types.ts @@ -6,7 +6,7 @@ // Some of the exports collide, which is not allowed, unless we redefine the colliding // exports in this file - which we do below. import type { Client, Integration, Options, StackParser } from '@sentry/core'; -import type { HandleClientError, HandleServerError } from '@sveltejs/kit'; +import type { AnyErrorHandler } from './common/handleErrorTypes'; import type * as clientSdk from './client'; import type * as serverSdk from './server'; @@ -22,7 +22,7 @@ export { initCloudflareSentryHandle } from './worker'; /** Initializes Sentry SvelteKit SDK */ export declare function init(options: Options | clientSdk.BrowserOptions | serverSdk.NodeOptions): Client | undefined; -export declare function handleErrorWithSentry(handleError?: T): T; +export declare function handleErrorWithSentry(handleError?: T): T; /** * Wrap a universal load function (e.g. +page.js or +layout.js) with Sentry functionality diff --git a/packages/sveltekit/src/server-common/handleError.ts b/packages/sveltekit/src/server-common/handleError.ts index 21e13899ee87..c1a9be07f3bc 100644 --- a/packages/sveltekit/src/server-common/handleError.ts +++ b/packages/sveltekit/src/server-common/handleError.ts @@ -1,37 +1,72 @@ import { captureException, consoleSandbox, flushIfServerless } from '@sentry/core'; -import type { HandleServerError } from '@sveltejs/kit'; +import type { AnyErrorHandler, SentryHandleServerErrorInput } from '../common/handleErrorTypes'; +import { shouldCaptureError } from '../common/handleErrorTypes'; import { getCloudflareExecutionContext } from './utils'; -// 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 - consoleSandbox(() => console.error(error?.stack)); -} - -type HandleServerErrorInput = Parameters[0]; +type ServerErrorHandler = (input: SentryHandleServerErrorInput) => unknown; /** - * Backwards-compatible HandleServerError Input type for SvelteKit 1.x and 2.x - * `message` and `status` were added in 2.x. - * For backwards-compatibility, we make them optional - * - * @see https://kit.svelte.dev/docs/migrating-to-sveltekit-2#improved-error-handling + * The default shape of the wrapped hook: structurally compatible with SvelteKit's + * `HandleServerError` on every supported major. */ -type SafeHandleServerErrorInput = Omit & - Partial>; +type SentryHandleServerError = (input: SentryHandleServerErrorInput) => Promise; + +// Mirrors SvelteKit's own default error handler, which differs by major version: +// - SvelteKit 1.x/2.x log the error's stack trace +// - SvelteKit 3 only logs unexpected errors (walking the `cause` chain), and logs the issues of +// remote function validation errors +// see: https://github.com/sveltejs/kit/blob/49f0808f3e983d0cb5a4d586cf0d1678467431ed/packages/kit/src/runtime/server/index.js#L132-L156 +function defaultErrorHandler(input: SentryHandleServerErrorInput): void { + if (input.kind === 'validation') { + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.error('Remote function schema validation failed:', input.issues); + }); + return; + } + + const { kind, error } = input; + + if (kind && kind !== 'unknown') { + // Don't log stack traces for expected app errors or framework errors like 404s + return; + } + + consoleSandbox(() => { + if (!kind) { + // SvelteKit 1.x/2.x + // eslint-disable-next-line no-console + console.error((error as Error | undefined)?.stack); + return; + } + + let e = error; + while (e instanceof Error) { + if (e.stack) { + // eslint-disable-next-line no-console + console.error(e.stack); + } + // `Error.cause` needs a lib newer than the one we compile against + e = (e as Error & { cause?: unknown }).cause; + } + + if (e) { + // eslint-disable-next-line no-console + console.error(String(e)); + } + }); +} /** * 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 { - const errorHandler = handleError ?? defaultErrorHandler; +export function handleErrorWithSentry(handleError?: T): T { + const errorHandler = (handleError ?? defaultErrorHandler) as ServerErrorHandler; - return async (input: HandleServerErrorInput): Promise => { - if (is4xxError(input)) { + const sentryErrorHandler = async (input: SentryHandleServerErrorInput): Promise => { + if (!shouldCaptureError(input, () => isExpectedLegacyError(input))) { return errorHandler(input); } @@ -42,7 +77,7 @@ export function handleErrorWithSentry(handleError?: HandleServerError): HandleSe }, }); - const cloudflareCtx = getCloudflareExecutionContext(input.event.platform); + const cloudflareCtx = getCloudflareExecutionContext(input.event?.platform); // Cloudflare workers have a `waitUntil` method on `ctx` that we can use to flush the event queue // We already call this in `wrapRequestHandler` from `sentryHandleInitCloudflare` @@ -56,12 +91,26 @@ export function handleErrorWithSentry(handleError?: HandleServerError): HandleSe return errorHandler(input); }; + + // Returning `T` (the caller's own hook type) is what keeps the result assignable to + // `HandleServerError` on both SvelteKit 2 and 3. The wrapper itself is written against our + // structural input type, which TS can't prove is identical to `T`, so it can't be narrowed + // without the double cast. + return sentryErrorHandler as unknown as T; } /** - * When a page request fails because the page is not found, SvelteKit throws a "Not found" error. + * Whether a SvelteKit 1.x/2.x error is an expected one we don't want to capture: a "Not found" + * error for an unmatched route, or any other 4xx. + * + * SvelteKit 3 errors are classified by `shouldCaptureError` instead. */ -function is4xxError(input: SafeHandleServerErrorInput): boolean { +function isExpectedLegacyError(input: SentryHandleServerErrorInput): boolean { + if (input.kind) { + // Not a SvelteKit 1.x/2.x input - narrows the union so `status` below is readable + return false; + } + const { error, event, status } = input; // SvelteKit 2.0 offers a reliable way to check for a Not Found error: @@ -72,7 +121,7 @@ function is4xxError(input: SafeHandleServerErrorInput): boolean { // SvelteKit 1.x doesn't offer a reliable way to check for a Not Found error. // So we check the route id (shouldn't exist) and the raw stack trace // We can delete all of this below whenever we drop Kit 1.x support - const hasNoRouteId = !event.route?.id; + const hasNoRouteId = !event?.route?.id; const rawStack: string = (error != null && diff --git a/packages/sveltekit/test/client/handleError.test.ts b/packages/sveltekit/test/client/handleError.test.ts index 76de90a55a4a..4f746b4d3456 100644 --- a/packages/sveltekit/test/client/handleError.test.ts +++ b/packages/sveltekit/test/client/handleError.test.ts @@ -5,7 +5,7 @@ import { handleErrorWithSentry } from '../../src/client/handleError'; const mockCaptureException = vi.spyOn(SentrySvelte, 'captureException').mockImplementation(() => 'xx'); -function handleError(_input: { error: unknown; event: NavigationEvent }): ReturnType { +function handleError(_input: Parameters[0]): ReturnType { return { message: 'Whoops!', }; @@ -33,7 +33,7 @@ describe('handleError (client)', () => { it('invokes the default handler if no handleError func is provided', async () => { const wrappedHandleError = handleErrorWithSentry(); const mockError = new Error('test'); - // @ts-expect-error - purposefully omitting status and message to cover SvelteKit 1.x compatibility + // purposefully omitting status and message to cover SvelteKit 1.x compatibility const returnVal = await wrappedHandleError({ error: mockError, event: navigationEvent }); expect(returnVal).not.toBeDefined(); @@ -93,3 +93,98 @@ describe('handleError (client)', () => { }, ); }); + +describe('handleError (client) [Kit 3.x]', () => { + // SvelteKit 3 passes *every* error to `handleError`, discriminated by `kind`, and moved the + // status from the input onto the error itself. + // see: https://svelte.dev/docs/kit/hooks#handleError + beforeEach(() => { + mockCaptureException.mockClear(); + consoleErrorSpy.mockClear(); + }); + + it('captures unexpected errors', async () => { + const wrappedHandleError = handleErrorWithSentry(); + const mockError = new Error('boom'); + + await wrappedHandleError({ kind: 'unknown', error: mockError, event: navigationEvent }); + + expect(mockCaptureException).toHaveBeenCalledWith(mockError, { + mechanism: { type: 'auto.function.sveltekit.handle_error', handled: false }, + }); + }); + + it('captures unexpected errors that happen to carry a 4xx `status` property', async () => { + // The SvelteKit 2 heuristic looked at `error.status` and would have skipped this one + const wrappedHandleError = handleErrorWithSentry(); + const mockError = { message: 'a failed fetch response', status: 404 }; + + await wrappedHandleError({ kind: 'unknown', error: mockError, event: navigationEvent }); + + expect(mockCaptureException).toHaveBeenCalledTimes(1); + }); + + it.each(['app', 'framework'] as const)("doesn't capture 4xx %s errors", async kind => { + const wrappedHandleError = handleErrorWithSentry(); + + await wrappedHandleError({ kind, error: { status: 404, message: 'Not Found' }, event: navigationEvent }); + + expect(mockCaptureException).not.toHaveBeenCalled(); + }); + + it.each(['app', 'framework'] as const)('captures 5xx %s errors', async kind => { + const wrappedHandleError = handleErrorWithSentry(); + const mockError = { status: 500, message: 'Internal Error' }; + + await wrappedHandleError({ kind, error: mockError, event: navigationEvent }); + + expect(mockCaptureException).toHaveBeenCalledTimes(1); + }); + + it('never reads the deprecated `status` and `message` input properties', async () => { + // In SvelteKit 3 dev builds these only exist as deprecated getters that log a warning when read + const statusGetter = vi.fn().mockReturnValue(500); + const messageGetter = vi.fn().mockReturnValue('Internal Error'); + + const input = { kind: 'framework' as const, error: { status: 404, message: 'Not Found' }, event: navigationEvent }; + Object.defineProperties(input, { + status: { get: statusGetter }, + message: { get: messageGetter }, + }); + + await handleErrorWithSentry()(input); + + expect(statusGetter).not.toHaveBeenCalled(); + expect(messageGetter).not.toHaveBeenCalled(); + }); + + describe('default error handler', () => { + it('logs unexpected errors', async () => { + const error = new Error('boom'); + + await handleErrorWithSentry()({ kind: 'unknown', error, event: navigationEvent }); + + expect(consoleErrorSpy).toHaveBeenCalledWith(error); + }); + + it.each(['app', 'framework'] as const)("doesn't log %s errors", async kind => { + await handleErrorWithSentry()({ + kind, + error: { status: 500, message: 'Internal Error' }, + event: navigationEvent, + }); + + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + }); +}); + +describe('handleErrorWithSentry (client) types', () => { + it("stays assignable to SvelteKit's `HandleClientError`", () => { + const withDefaultHandler: HandleClientError = handleErrorWithSentry(); + const withCustomHandler: HandleClientError = handleErrorWithSentry(handleError); + + expect(withDefaultHandler).toBeTypeOf('function'); + expect(withCustomHandler).toBeTypeOf('function'); + }); +}); diff --git a/packages/sveltekit/test/server-common/handleError.test.ts b/packages/sveltekit/test/server-common/handleError.test.ts index 928d3fbe61f0..6b3ee06c17c1 100644 --- a/packages/sveltekit/test/server-common/handleError.test.ts +++ b/packages/sveltekit/test/server-common/handleError.test.ts @@ -5,7 +5,7 @@ import { handleErrorWithSentry } from '../../src/server-common/handleError'; const mockCaptureException = vi.spyOn(SentryCore, 'captureException').mockImplementation(() => 'xx'); -function handleError(_input: { error: unknown; event: RequestEvent }): ReturnType { +function handleError(_input: Parameters[0]): ReturnType { return { message: 'Whoops!', }; @@ -30,7 +30,7 @@ describe('handleError (server)', () => { // ... } as RequestEvent; - // @ts-expect-error - purposefully omitting status and message to cover SvelteKit 1.x compatibility + // purposefully omitting status and message to cover SvelteKit 1.x compatibility const returnVal = await wrappedHandleError({ error: mockError, event: mockEvent }); expect(returnVal).not.toBeDefined(); @@ -160,3 +160,153 @@ describe('handleError (server)', () => { }); }); }); + +describe('handleError (server) [Kit 3.x]', () => { + // SvelteKit 3 passes *every* error to `handleError`, discriminated by `kind`, and moved the + // status from the input onto the error itself. + // see: https://svelte.dev/docs/kit/hooks#handleError + beforeEach(() => { + mockCaptureException.mockClear(); + consoleErrorSpy.mockClear(); + }); + + it('captures unexpected errors', async () => { + const wrappedHandleError = handleErrorWithSentry(); + const mockError = new Error('boom'); + + await wrappedHandleError({ kind: 'unknown', error: mockError, event: requestEvent }); + + expect(mockCaptureException).toHaveBeenCalledWith(mockError, { + mechanism: { type: 'auto.function.sveltekit.handle_error', handled: false }, + }); + }); + + it.each(['app', 'framework'] as const)("doesn't capture 4xx %s errors", async kind => { + const wrappedHandleError = handleErrorWithSentry(); + + await wrappedHandleError({ kind, error: { status: 404, message: 'Not Found' }, event: requestEvent }); + + expect(mockCaptureException).not.toHaveBeenCalled(); + }); + + it.each(['app', 'framework'] as const)('captures 5xx %s errors', async kind => { + const wrappedHandleError = handleErrorWithSentry(); + const mockError = { status: 500, message: 'Internal Error' }; + + await wrappedHandleError({ kind, error: mockError, event: requestEvent }); + + expect(mockCaptureException).toHaveBeenCalledWith(mockError, { + mechanism: { type: 'auto.function.sveltekit.handle_error', handled: false }, + }); + }); + + it("doesn't capture remote function validation errors", async () => { + const wrappedHandleError = handleErrorWithSentry(); + + await wrappedHandleError({ + kind: 'validation', + error: { status: 400, message: 'Bad Request' }, + issues: [{ message: 'Expected string' }], + event: requestEvent, + }); + + expect(mockCaptureException).not.toHaveBeenCalled(); + expect(consoleErrorSpy).toHaveBeenCalledWith('Remote function schema validation failed:', [ + { message: 'Expected string' }, + ]); + }); + + it('captures errors whose status we cannot determine', async () => { + const wrappedHandleError = handleErrorWithSentry(); + const mockError = { message: 'no status here' }; + + await wrappedHandleError({ kind: 'app', error: mockError, event: requestEvent }); + + expect(mockCaptureException).toHaveBeenCalledTimes(1); + }); + + it('never reads the deprecated `status` and `message` input properties', async () => { + // In SvelteKit 3 dev builds these only exist as deprecated getters that log a warning when read + const statusGetter = vi.fn().mockReturnValue(500); + const messageGetter = vi.fn().mockReturnValue('Internal Error'); + + const input = { kind: 'framework' as const, error: { status: 404, message: 'Not Found' }, event: requestEvent }; + Object.defineProperties(input, { + status: { get: statusGetter }, + message: { get: messageGetter }, + }); + + await handleErrorWithSentry()(input); + + expect(statusGetter).not.toHaveBeenCalled(); + expect(messageGetter).not.toHaveBeenCalled(); + }); + + it('calls the user-provided handler for both captured and skipped errors', async () => { + const userHandler = vi.fn().mockReturnValue({ message: 'Whoops!' }); + const wrappedHandleError = handleErrorWithSentry(userHandler); + + const captured = { kind: 'unknown' as const, error: new Error('boom'), event: requestEvent }; + const skipped = { kind: 'app' as const, error: { status: 404, message: 'Not Found' }, event: requestEvent }; + + expect(await wrappedHandleError(captured)).toEqual({ message: 'Whoops!' }); + expect(await wrappedHandleError(skipped)).toEqual({ message: 'Whoops!' }); + + expect(userHandler).toHaveBeenNthCalledWith(1, captured); + expect(userHandler).toHaveBeenNthCalledWith(2, skipped); + expect(mockCaptureException).toHaveBeenCalledTimes(1); + }); + + describe('default error handler', () => { + it('logs the stack traces of an unexpected error and its causes', async () => { + const cause = new Error('the cause'); + const error = new Error('boom'); + // set explicitly: the `ErrorOptions` constructor overload needs a newer lib than we compile against + (error as Error & { cause?: unknown }).cause = cause; + + await handleErrorWithSentry()({ kind: 'unknown', error, event: requestEvent }); + + expect(consoleErrorSpy).toHaveBeenCalledTimes(2); + expect(consoleErrorSpy).toHaveBeenNthCalledWith(1, error.stack); + expect(consoleErrorSpy).toHaveBeenNthCalledWith(2, cause.stack); + }); + + it.each(['app', 'framework'] as const)("doesn't log %s errors", async kind => { + await handleErrorWithSentry()({ kind, error: { status: 500, message: 'Internal Error' }, event: requestEvent }); + + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + }); +}); + +describe('handleErrorWithSentry (server) types', () => { + it("stays assignable to SvelteKit's `HandleServerError`", () => { + const withDefaultHandler: HandleServerError = handleErrorWithSentry(); + const withCustomHandler: HandleServerError = handleErrorWithSentry(handleError); + + expect(withDefaultHandler).toBeTypeOf('function'); + expect(withCustomHandler).toBeTypeOf('function'); + }); +}); + +describe('handleError (server) validation errors', () => { + beforeEach(() => { + mockCaptureException.mockClear(); + consoleErrorSpy.mockClear(); + }); + + // SvelteKit always gives validation errors a 400, so the status rule alone would cover them. + // These pin the explicit `kind` check, so the behaviour doesn't depend on that Kit internal. + it.each([undefined, 500, 503])("doesn't capture a validation error with status %s", async status => { + const wrappedHandleError = handleErrorWithSentry(); + + await wrappedHandleError({ + kind: 'validation', + error: { status, message: 'Bad Request' }, + issues: [{ message: 'Expected string' }], + event: requestEvent, + }); + + expect(mockCaptureException).not.toHaveBeenCalled(); + }); +});