breaking: tighten up error handling

.changeset/real-pets-fix.md

@@ -0,0 +1,5 @@
+---
+"@sveltejs/kit": major
+---
+
+breaking: tighten up error handling

documentation/docs/30-advanced/20-hooks.md

@@ -142,7 +142,7 @@ The following can be added to `src/hooks.server.js` _and_ `src/hooks.client.js`:
 If an unexpected error is thrown during loading or rendering, this function will be called with the `error` and the `event`. This allows for two things:
 
 - you can log the error
-- you can generate a custom representation of the error that is safe to show to users, omitting sensitive details like messages and stack traces. The returned value becomes the value of `$page.error`. It defaults to `{ message: 'Not Found' }` in case of a 404 (you can detect them through `event.route.id` being `null`) and to `{ message: 'Internal Error' }` for everything else. To make this type-safe, you can customize the expected shape by declaring an `App.Error` interface (which must include `message: string`, to guarantee sensible fallback behavior).
+- you can generate a custom representation of the error that is safe to show to users, omitting sensitive details like messages and stack traces. The returned value becomes the value of `$page.error`. It defaults to the message of `NonFatalError` for non-fatal errors within SvelteKit (such as `{ message: 'Not found: ..' }` in case of a 404) and to `{ message: 'Internal Error' }` for everything else. To make this type-safe, you can customize the expected shape by declaring an `App.Error` interface (which must include `message: string`, to guarantee sensible fallback behavior).
 
 The following code shows an example of typing the error shape as `{ message: string; errorId: string }` and returning it accordingly from the `handleError` functions:
 

documentation/docs/30-advanced/25-errors.md

@@ -106,6 +106,43 @@ export function handleError({ error, event }) {
 }
 ```
 
+SvelteKit may handle certain errors as non-fatal. In these cases, it throws a `NonFatalError` which contains a non-sensitive message and the status code. Examples for this are 404s (page not found) or 415s for actions (wrong content-type). These go through `handleError` as well, and you can distinguish them from other unexpected errors via an `instanceof` check.
+
+```js
+/// file: src/hooks.server.js
+// @errors: 2322 1360 2571 2339 2353
+// @filename: ambient.d.ts
+declare module '@sentry/sveltekit' {
+	export const init: (opts: any) => void;
+	export const captureException: (error: any, opts: any) => void;
+}
+
+// @filename: index.js
+// ---cut---
+import * as Sentry from '@sentry/sveltekit';
+import { NonFatalError } from '@sveltejs/kit';
+
+Sentry.init({/*...*/})
+
+/** @type {import('@sveltejs/kit').HandleServerError} */
+export function handleError({ error, event }) {
+	if (error instanceof NonFatalError) {
+		return {
+			message: error.message, // safe to forward
+			code: 'UNKNOWN'
+		};
+	} else {
+		// example integration with https://sentry.io/
+		Sentry.captureException(error, { extra: { event } });
+
+		return {
+			message: 'Whoops!',
+			code: error?.code ?? 'UNKNOWN'
+		};
+	}
+}
+```
+
 > Make sure that `handleError` _never_ throws an error
 
 ## Responses

documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md

@@ -105,6 +105,12 @@ As such, SvelteKit 2 replaces `resolvePath` with a (slightly better named) funct
 
 `svelte-migrate` will do the method replacement for you, though if you later prepend the result with `base`, you need to remove that yourself.
 
+## Improved error handling
+
+Some errors are handled internally by SvelteKit, but they are not unexpected. In SvelteKit 1 they were all handled by throwing a regular `Error` on a case-by-case basis. This meant it's not easy to distinguish between real unexpected errors and others such as someone calling your action endpoint with the wrong content type.
+
+SvelteKit 2 cleans this up by introducing a new `NonFatalError` object which extends `Error` and is thrown by all internal code paths were applicable. You can distinguish these non-fatal errors by doing an `instanceof NonFatalError` check inside `handleError`.
+
 ## Dynamic environment variables cannot be used during prerendering
 
 The `$env/dynamic/public` and `$env/dynamic/private` modules provide access to _run time_ environment variables, as opposed to the _build time_ environment variables exposed by `$env/static/public` and `$env/static/private`.

packages/kit/src/exports/index.js

@@ -1,7 +1,8 @@
-import { HttpError, Redirect, ActionFailure } from '../runtime/control.js';
+import { HttpError, Redirect, ActionFailure, NonFatalError } from '../runtime/control.js';
 import { BROWSER, DEV } from 'esm-env';
 
 export { VERSION } from '../version.js';
+export { NonFatalError };
 
 /**
  * @template {number} TNumber

packages/kit/src/exports/node/index.js

@@ -1,5 +1,4 @@
 import * as set_cookie_parser from 'set-cookie-parser';
-import { error } from '../index.js';
 
 /**
  * @param {import('http').IncomingMessage} req
@@ -28,10 +27,10 @@ function get_raw_body(req, body_size_limit) {
 		if (!length) {
 			length = body_size_limit;
 		} else if (length > body_size_limit) {
-			error(
-				413,
-				`Received content-length of ${length}, but only accept up to ${body_size_limit} bytes.`
-			);
+			throw {
+				status: 413,
+				message: `Received content-length of ${length}, but only accept up to ${body_size_limit} bytes.`
+			};
 		}
 	}
 
@@ -62,14 +61,12 @@ function get_raw_body(req, body_size_limit) {
 				size += chunk.length;
 				if (size > length) {
 					cancelled = true;
-					controller.error(
-						error(
-							413,
-							`request body size exceeded ${
-								content_length ? "'content-length'" : 'BODY_SIZE_LIMIT'
-							} of ${length}`
-						)
-					);
+					controller.error({
+						status: 413,
+						message: `request body size exceeded ${
+							content_length ? "'content-length'" : 'BODY_SIZE_LIMIT'
+						} of ${length}`
+					});
 					return;
 				}
 

packages/kit/src/exports/vite/dev/index.js

@@ -355,7 +355,8 @@ export async function dev(vite, vite_config, svelte_config) {
 		control_module_node.replace_implementations({
 			ActionFailure: control_module_vite.ActionFailure,
 			HttpError: control_module_vite.HttpError,
-			Redirect: control_module_vite.Redirect
+			Redirect: control_module_vite.Redirect,
+			NonFatalError: control_module_vite.NonFatalError
 		});
 	}
 	align_exports();

packages/kit/src/runtime/client/client.js

@@ -30,10 +30,11 @@ import { base } from '__sveltekit/paths';
 import * as devalue from 'devalue';
 import { compact } from '../../utils/array.js';
 import { validate_page_exports } from '../../utils/exports.js';
-import { HttpError, Redirect, NotFound } from '../control.js';
+import { HttpError, Redirect, NonFatalError } from '../control.js';
 import { INVALIDATED_PARAM, TRAILING_SLASH_PARAM, validate_depends } from '../shared.js';
 import { INDEX_KEY, PRELOAD_PRIORITIES, SCROLL_KEY, SNAPSHOT_KEY } from './constants.js';
 import { stores } from './singletons.js';
+import { get_status } from '../../utils/error.js';
 
 let errored = false;
 
@@ -704,7 +705,7 @@ export function create_client(app, target) {
 				server_data = await load_data(url, invalid_server_nodes);
 			} catch (error) {
 				return load_root_error_page({
-					status: error instanceof HttpError ? error.status : 500,
+					status: get_status(error),
 					error: await handle_error(error, { url, params, route: { id: route.id } }),
 					url,
 					route
@@ -788,7 +789,7 @@ export function create_client(app, target) {
 						};
 					}
 
-					let status = 500;
+					let status = get_status(err);
 					/** @type {App.Error} */
 					let error;
 
@@ -798,7 +799,6 @@ export function create_client(app, target) {
 						status = /** @type {import('types').ServerErrorNode} */ (err).status ?? status;
 						error = /** @type {import('types').ServerErrorNode} */ (err).error;
 					} else if (err instanceof HttpError) {
-						status = err.status;
 						error = err.body;
 					} else {
 						// Referenced node could have been removed due to redeploy, check
@@ -1060,7 +1060,7 @@ export function create_client(app, target) {
 			navigation_result = await server_fallback(
 				url,
 				{ id: null },
-				await handle_error(new Error(`Not found: ${url.pathname}`), {
+				await handle_error(new NonFatalError(404, 'Not Found', `Not found: ${url.pathname}`), {
 					url,
 					params: {},
 					route: { id: null }
@@ -1380,8 +1380,7 @@ export function create_client(app, target) {
 		return (
 			app.hooks.handleError({ error, event }) ??
 			/** @type {any} */ ({
-				message:
-					event.route.id === null && error instanceof NotFound ? 'Not Found' : 'Internal Error'
+				message: error instanceof NonFatalError ? error.message : 'Internal Error'
 			})
 		);
 	}
@@ -1908,7 +1907,7 @@ export function create_client(app, target) {
 				}
 
 				result = await load_root_error_page({
-					status: error instanceof HttpError ? error.status : 500,
+					status: get_status(error),
 					error: await handle_error(error, { url, params, route }),
 					url,
 					route

packages/kit/src/runtime/control.js

@@ -30,15 +30,20 @@ export class Redirect {
 	}
 }
 
-export class NotFound extends Error {
+/**
+ * An error that was thrown from within the SvelteKit runtime that is not fatal and doesn't result in a 500, such as a 404.
+ * `NonFatalError` goes through `handleError` and you can distinguish it from other errors using `instanceof NonFatalError`.
+ */
+export class NonFatalError extends Error {
 	/**
-	 * @param {string} pathname
+	 * @param {number} status
+	 * @param {string} message
+	 * @param {string} [context]
 	 */
-	constructor(pathname) {
-		super();
-
-		this.status = 404;
-		this.message = `Not found: ${pathname}`;
+	constructor(status, message, context) {
+		super(message);
+		this.status = status;
+		this.context = context;
 	}
 }
 
@@ -66,6 +71,7 @@ export class ActionFailure {
  *   ActionFailure: typeof ActionFailure;
  *   HttpError: typeof HttpError;
  *   Redirect: typeof Redirect;
+ *   NonFatalError: typeof NonFatalError;
  * }} implementations
  */
 export function replace_implementations(implementations) {
@@ -75,4 +81,6 @@ export function replace_implementations(implementations) {
 	HttpError = implementations.HttpError; // eslint-disable-line no-class-assign
 	// @ts-expect-error
 	Redirect = implementations.Redirect; // eslint-disable-line no-class-assign
+	// @ts-expect-error
+	NonFatalError = implementations.NonFatalError; // eslint-disable-line no-class-assign
 }

packages/kit/src/runtime/server/data/index.js

@@ -1,4 +1,4 @@
-import { HttpError, Redirect } from '../../control.js';
+import { HttpError, NonFatalError, Redirect } from '../../control.js';
 import { normalize_error } from '../../../utils/error.js';
 import { once } from '../../../utils/functions.js';
 import { load_server_data } from '../page/load_data.js';
@@ -109,7 +109,10 @@ export async function render_data(
 					return /** @type {import('types').ServerErrorNode} */ ({
 						type: 'error',
 						error: await handle_error_and_jsonify(event, options, error),
-						status: error instanceof HttpError ? error.status : undefined
+						status:
+							error instanceof HttpError || error instanceof NonFatalError
+								? error.status
+								: undefined
 					});
 				})
 			)

packages/kit/src/runtime/server/index.js

@@ -81,13 +81,6 @@ export class Server {
 	 * @param {import('types').RequestOptions} options
 	 */
 	async respond(request, options) {
-		// TODO this should probably have been removed for 1.0 — i think we can get rid of it?
-		if (!(request instanceof Request)) {
-			throw new Error(
-				'The first argument to server.respond must be a Request object. See https://github.com/sveltejs/kit/pull/3384 for details'
-			);
-		}
-
 		return respond(request, this.#options, this.#manifest, {
 			...options,
 			error: false,

packages/kit/src/runtime/server/page/actions.js

@@ -1,8 +1,8 @@
 import * as devalue from 'devalue';
-import { error, json } from '../../../exports/index.js';
-import { normalize_error } from '../../../utils/error.js';
+import { json } from '../../../exports/index.js';
+import { get_status, normalize_error } from '../../../utils/error.js';
 import { is_form_content_type, negotiate } from '../../../utils/http.js';
-import { HttpError, Redirect, ActionFailure } from '../../control.js';
+import { HttpError, Redirect, ActionFailure, NonFatalError } from '../../control.js';
 import { handle_error_and_jsonify } from '../utils.js';
 
 /** @param {import('@sveltejs/kit').RequestEvent} event */
@@ -24,8 +24,7 @@ export async function handle_action_json_request(event, options, server) {
 	const actions = server?.actions;
 
 	if (!actions) {
-		// TODO should this be a different error altogether?
-		const no_actions_error = new HttpError(
+		const no_actions_error = new NonFatalError(
 			405,
 			'POST method not allowed. No actions exist for this page'
 		);
@@ -84,7 +83,7 @@ export async function handle_action_json_request(event, options, server) {
 				error: await handle_error_and_jsonify(event, options, check_incorrect_fail_use(err))
 			},
 			{
-				status: err instanceof HttpError ? err.status : 500
+				status: get_status(err)
 			}
 		);
 	}
@@ -142,7 +141,7 @@ export async function handle_action_request(event, server) {
 		});
 		return {
 			type: 'error',
-			error: new HttpError(405, 'POST method not allowed. No actions exist for this page')
+			error: new NonFatalError(405, 'POST method not allowed. No actions exist for this page')
 		};
 	}
 
@@ -201,7 +200,7 @@ function check_named_default_separate(actions) {
 /**
  * @param {import('@sveltejs/kit').RequestEvent} event
  * @param {NonNullable<import('types').SSRNode['server']['actions']>} actions
- * @throws {Redirect | ActionFailure | HttpError | Error}
+ * @throws {Redirect | HttpError | NonFatalError | Error}
  */
 async function call_action(event, actions) {
 	const url = new URL(event.request.url);
@@ -219,12 +218,14 @@ async function call_action(event, actions) {
 
 	const action = actions[name];
 	if (!action) {
-		throw new Error(`No action with name '${name}' found`);
+		throw new NonFatalError(404, `No action with name '${name}' found`);
 	}
 
 	if (!is_form_content_type(event.request)) {
-		throw new Error(
-			`Actions expect form-encoded data (received ${event.request.headers.get('content-type')})`
+		throw new NonFatalError(
+			415,
+			'Actions expect form-encoded data',
+			`Received ${event.request.headers.get('content-type')}`
 		);
 	}
 

packages/kit/src/runtime/server/page/index.js

@@ -1,8 +1,8 @@
 import { text } from '../../../exports/index.js';
 import { compact } from '../../../utils/array.js';
-import { normalize_error } from '../../../utils/error.js';
+import { get_status, normalize_error } from '../../../utils/error.js';
 import { add_data_suffix } from '../../../utils/url.js';
-import { HttpError, Redirect } from '../../control.js';
+import { Redirect } from '../../control.js';
 import { redirect_response, static_error_page, handle_error_and_jsonify } from '../utils.js';
 import {
 	handle_action_json_request,
@@ -65,8 +65,7 @@ export async function render_page(event, page, options, manifest, state, resolve
 				return redirect_response(action_result.status, action_result.location);
 			}
 			if (action_result?.type === 'error') {
-				const error = action_result.error;
-				status = error instanceof HttpError ? error.status : 500;
+				status = get_status(action_result.error);
 			}
 			if (action_result?.type === 'failure') {
 				status = action_result.status;
@@ -221,7 +220,7 @@ export async function render_page(event, page, options, manifest, state, resolve
 						return redirect_response(err.status, err.location);
 					}
 
-					const status = err instanceof HttpError ? err.status : 500;
+					const status = get_status(err);
 					const error = await handle_error_and_jsonify(event, options, err);
 
 					while (i--) {

packages/kit/src/runtime/server/page/respond_with_error.js

@@ -2,7 +2,8 @@ import { render_response } from './render.js';
 import { load_data, load_server_data } from './load_data.js';
 import { handle_error_and_jsonify, static_error_page, redirect_response } from '../utils.js';
 import { get_option } from '../../../utils/options.js';
-import { HttpError, Redirect } from '../../control.js';
+import { Redirect } from '../../control.js';
+import { get_status } from '../../../utils/error.js';
 
 /**
  * @typedef {import('./types.js').Loaded} Loaded
@@ -103,7 +104,7 @@ export async function respond_with_error({
 
 		return static_error_page(
 			options,
-			e instanceof HttpError ? e.status : 500,
+			get_status(e),
 			(await handle_error_and_jsonify(event, options, e)).message
 		);
 	}