From 64565c0e242f322e4d669b45bf5f599bc418c478 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 13 Dec 2023 14:45:06 +0100 Subject: [PATCH 01/25] breaking: tighten up error handling Introduces a NonFatalError object that is used internally and that is user-detectable in handleError --- .changeset/real-pets-fix.md | 5 +++ documentation/docs/30-advanced/20-hooks.md | 2 +- documentation/docs/30-advanced/25-errors.md | 37 +++++++++++++++++++ .../30-migrating-to-sveltekit-2.md | 6 +++ packages/kit/src/exports/index.js | 3 +- packages/kit/src/exports/node/index.js | 25 ++++++------- packages/kit/src/exports/vite/dev/index.js | 5 ++- packages/kit/src/runtime/client/client.js | 23 ++++++------ packages/kit/src/runtime/control.js | 22 +++++++---- packages/kit/src/runtime/server/data/index.js | 7 +++- packages/kit/src/runtime/server/index.js | 7 ---- .../kit/src/runtime/server/page/actions.js | 22 +++++------ packages/kit/src/runtime/server/page/index.js | 9 ++--- .../runtime/server/page/respond_with_error.js | 5 ++- packages/kit/src/runtime/server/respond.js | 14 ++----- packages/kit/src/runtime/server/utils.js | 9 +++-- packages/kit/src/utils/error.js | 11 +++++- 17 files changed, 133 insertions(+), 79 deletions(-) create mode 100644 .changeset/real-pets-fix.md diff --git a/.changeset/real-pets-fix.md b/.changeset/real-pets-fix.md new file mode 100644 index 000000000000..336da95529b1 --- /dev/null +++ b/.changeset/real-pets-fix.md @@ -0,0 +1,5 @@ +--- +"@sveltejs/kit": major +--- + +breaking: tighten up error handling diff --git a/documentation/docs/30-advanced/20-hooks.md b/documentation/docs/30-advanced/20-hooks.md index 91603f1abbca..756b581239ec 100644 --- a/documentation/docs/30-advanced/20-hooks.md +++ b/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: diff --git a/documentation/docs/30-advanced/25-errors.md b/documentation/docs/30-advanced/25-errors.md index 81f216cd2ff3..9efdc65d7036 100644 --- a/documentation/docs/30-advanced/25-errors.md +++ b/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 diff --git a/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md b/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md index 6cda9848e788..70edf902e0b5 100644 --- a/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md +++ b/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. +## Error handling was slightly altered + +Some errors are handled internally by SvelteKit, but they are not unexpected. They were handled either by using the `error` function or throwing a regular `Error` on a case-by-case basis. This meant it's a) not easy to distinguish between real unexpected errors and others such as someone calling your action endpoint with the wrong content type and b) introduces a potential bug where properties that may be required due to a custom `App.Error` interface are missing. + +SvelteKit 2 cleans this up by introducing a new `NonFatalError` object which extends `Error` and is thrown by all internal code paths were applicable. As a consequence, `handleError` may be called more often now than before. You can distinguish these non-fatal errors by doing an `instanceof NonFatalError` check inside `handleError`. + ## Updated dependency requirements SvelteKit requires Node `18.13` or higher, Vite `^5.0`, vite-plugin-svelte `^3.0`, TypeScript `^5.0` and Svelte version 4 or higher. `svelte-migrate` will do the `package.json` bumps for you. diff --git a/packages/kit/src/exports/index.js b/packages/kit/src/exports/index.js index 283ba840c2ef..15957287c052 100644 --- a/packages/kit/src/exports/index.js +++ b/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 diff --git a/packages/kit/src/exports/node/index.js b/packages/kit/src/exports/node/index.js index 482e4ae8ac92..45813746ff5e 100644 --- a/packages/kit/src/exports/node/index.js +++ b/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; } @@ -124,7 +121,7 @@ export async function setResponse(res, response) { ? set_cookie_parser.splitCookiesString( // This is absurd but necessary, TODO: investigate why /** @type {string}*/ (response.headers.get(key)) - ) + ) : value ); } catch (error) { diff --git a/packages/kit/src/exports/vite/dev/index.js b/packages/kit/src/exports/vite/dev/index.js index 2a4293fff74b..2c59e66ec4ed 100644 --- a/packages/kit/src/exports/vite/dev/index.js +++ b/packages/kit/src/exports/vite/dev/index.js @@ -220,7 +220,7 @@ export async function dev(vite, vite_config, svelte_config) { ? async () => { const url = path.resolve(cwd, endpoint.file); return await loud_ssr_load_module(url); - } + } : null, endpoint_id: endpoint?.file }; @@ -354,7 +354,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(); diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 2ae4eacf0a14..b128d8691bb0 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/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; @@ -545,10 +546,10 @@ export function create_client(app, target) { typeof data !== 'object' ? `a ${typeof data}` : data instanceof Response - ? 'a Response object' - : Array.isArray(data) - ? 'an array' - : 'a non-plain object' + ? 'a Response object' + : Array.isArray(data) + ? 'an array' + : 'a non-plain object' }, but must return a plain object at the top level (i.e. \`return {...}\`)` ); } @@ -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 diff --git a/packages/kit/src/runtime/control.js b/packages/kit/src/runtime/control.js index 8d60e4741bf0..ce61152f26ef 100644 --- a/packages/kit/src/runtime/control.js +++ b/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 } diff --git a/packages/kit/src/runtime/server/data/index.js b/packages/kit/src/runtime/server/data/index.js index d8f22e57c404..ae8847cdb548 100644 --- a/packages/kit/src/runtime/server/data/index.js +++ b/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 }); }) ) diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js index 607f161b2742..90bc06f78f88 100644 --- a/packages/kit/src/runtime/server/index.js +++ b/packages/kit/src/runtime/server/index.js @@ -71,13 +71,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, diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index 8784d99a2bc8..534a818f04f8 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/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} 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,13 +218,14 @@ async function call_action(event, actions) { const action = actions[name]; if (!action) { - throw error(404, `No action with name '${name}' found`); + throw new NonFatalError(404, `No action with name '${name}' found`); } if (!is_form_content_type(event.request)) { - throw error( + throw new NonFatalError( 415, - `Actions expect form-encoded data (received ${event.request.headers.get('content-type')})` + `Actions expect form-encoded data`, + `Received ${event.request.headers.get('content-type')}` ); } diff --git a/packages/kit/src/runtime/server/page/index.js b/packages/kit/src/runtime/server/page/index.js index bcc316fbc323..067d60585cd9 100644 --- a/packages/kit/src/runtime/server/page/index.js +++ b/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--) { diff --git a/packages/kit/src/runtime/server/page/respond_with_error.js b/packages/kit/src/runtime/server/page/respond_with_error.js index ef7925d60f22..e320b01f04a4 100644 --- a/packages/kit/src/runtime/server/page/respond_with_error.js +++ b/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 ); } diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index 7637e5437fcc..8827cacdc3ea 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -18,7 +18,7 @@ import { exec } from '../../utils/routing.js'; import { redirect_json_response, render_data } from './data/index.js'; import { add_cookies_to_headers, get_cookies } from './cookie.js'; import { create_fetch } from './fetch.js'; -import { HttpError, Redirect, NotFound } from '../control.js'; +import { HttpError, Redirect, NonFatalError } from '../control.js'; import { validate_layout_exports, validate_layout_server_exports, @@ -344,8 +344,8 @@ export async function respond(request, options, manifest, state) { const response = is_data_request ? redirect_json_response(e) : route?.page && is_action_json_request(event) - ? action_json_redirect(e) - : redirect_response(e.status, e.location); + ? action_json_redirect(e) + : redirect_response(e.status, e.location); add_cookies_to_headers(response.headers, Object.values(cookies_to_add)); return response; } @@ -359,12 +359,6 @@ export async function respond(request, options, manifest, state) { async function resolve(event, opts) { try { if (opts) { - if ('ssr' in opts) { - throw new Error( - 'ssr has been removed, set it in the appropriate +layout.js instead. See the PR for more information: https://github.com/sveltejs/kit/pull/6197' - ); - } - resolve_opts = { transformPageChunk: opts.transformPageChunk || default_transform, filterSerializedResponseHeaders: opts.filterSerializedResponseHeaders || default_filter, @@ -483,7 +477,7 @@ export async function respond(request, options, manifest, state) { manifest, state, status: 404, - error: new NotFound(event.url.pathname), + error: new NonFatalError(404, 'Not Found', `Not found: ${event.url.pathname}`), resolve_opts }); } diff --git a/packages/kit/src/runtime/server/utils.js b/packages/kit/src/runtime/server/utils.js index ef8277efedcc..5e7e79d82ba6 100644 --- a/packages/kit/src/runtime/server/utils.js +++ b/packages/kit/src/runtime/server/utils.js @@ -1,8 +1,8 @@ import { DEV } from 'esm-env'; import { json, text } from '../../exports/index.js'; -import { coalesce_to_error } from '../../utils/error.js'; +import { coalesce_to_error, get_status } from '../../utils/error.js'; import { negotiate } from '../../utils/http.js'; -import { HttpError, NotFound } from '../control.js'; +import { HttpError, NonFatalError } from '../control.js'; import { fix_stack_trace } from '../shared-server.js'; import { ENDPOINT_METHODS } from '../../constants.js'; @@ -70,7 +70,7 @@ export function static_error_page(options, status, message) { */ export async function handle_fatal_error(event, options, error) { error = error instanceof HttpError ? error : coalesce_to_error(error); - const status = error instanceof HttpError ? error.status : 500; + const status = get_status(error); const body = await handle_error_and_jsonify(event, options, error); // ideally we'd use sec-fetch-dest instead, but Safari — quelle surprise — doesn't support it @@ -105,7 +105,8 @@ export async function handle_error_and_jsonify(event, options, error) { return ( (await options.hooks.handleError({ error, event })) ?? { - message: event.route.id === null && error instanceof NotFound ? 'Not Found' : 'Internal Error' + message: + event.route.id === null && error instanceof NonFatalError ? error.message : 'Internal Error' } ); } diff --git a/packages/kit/src/utils/error.js b/packages/kit/src/utils/error.js index d485dc75ca55..34c9a51f841a 100644 --- a/packages/kit/src/utils/error.js +++ b/packages/kit/src/utils/error.js @@ -1,3 +1,5 @@ +import { HttpError, NonFatalError } from '../runtime/control.js'; + /** * @param {unknown} err * @return {Error} @@ -16,7 +18,14 @@ export function coalesce_to_error(err) { * @param {unknown} error */ export function normalize_error(error) { - return /** @type {import('../runtime/control.js').Redirect | import('../runtime/control.js').HttpError | Error} */ ( + return /** @type {import('../runtime/control.js').Redirect | HttpError | NonFatalError | Error} */ ( error ); } + +/** + * @param {unknown} error + */ +export function get_status(error) { + return error instanceof HttpError || error instanceof NonFatalError ? error.status : 500; +} From fddc54fad1c41af49591d6bfef249a2abbab250b Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 13 Dec 2023 14:51:45 +0100 Subject: [PATCH 02/25] how did this happen --- packages/kit/src/exports/node/index.js | 2 +- packages/kit/src/exports/vite/dev/index.js | 2 +- packages/kit/src/runtime/client/client.js | 8 ++++---- packages/kit/src/runtime/server/respond.js | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/kit/src/exports/node/index.js b/packages/kit/src/exports/node/index.js index 45813746ff5e..1d43fd307dad 100644 --- a/packages/kit/src/exports/node/index.js +++ b/packages/kit/src/exports/node/index.js @@ -121,7 +121,7 @@ export async function setResponse(res, response) { ? set_cookie_parser.splitCookiesString( // This is absurd but necessary, TODO: investigate why /** @type {string}*/ (response.headers.get(key)) - ) + ) : value ); } catch (error) { diff --git a/packages/kit/src/exports/vite/dev/index.js b/packages/kit/src/exports/vite/dev/index.js index 2c59e66ec4ed..630c34dbab90 100644 --- a/packages/kit/src/exports/vite/dev/index.js +++ b/packages/kit/src/exports/vite/dev/index.js @@ -220,7 +220,7 @@ export async function dev(vite, vite_config, svelte_config) { ? async () => { const url = path.resolve(cwd, endpoint.file); return await loud_ssr_load_module(url); - } + } : null, endpoint_id: endpoint?.file }; diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index b128d8691bb0..5bc51a9d3d1f 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -546,10 +546,10 @@ export function create_client(app, target) { typeof data !== 'object' ? `a ${typeof data}` : data instanceof Response - ? 'a Response object' - : Array.isArray(data) - ? 'an array' - : 'a non-plain object' + ? 'a Response object' + : Array.isArray(data) + ? 'an array' + : 'a non-plain object' }, but must return a plain object at the top level (i.e. \`return {...}\`)` ); } diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index 8827cacdc3ea..d4bcdb18c7d8 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -344,8 +344,8 @@ export async function respond(request, options, manifest, state) { const response = is_data_request ? redirect_json_response(e) : route?.page && is_action_json_request(event) - ? action_json_redirect(e) - : redirect_response(e.status, e.location); + ? action_json_redirect(e) + : redirect_response(e.status, e.location); add_cookies_to_headers(response.headers, Object.values(cookies_to_add)); return response; } From 7613649cf98a6f13ce03ca9f9ccbac95547e1db3 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 13 Dec 2023 14:58:03 +0100 Subject: [PATCH 03/25] fix tests --- packages/kit/test/apps/basics/test/cross-platform/test.js | 2 +- packages/kit/test/apps/basics/test/test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/kit/test/apps/basics/test/cross-platform/test.js b/packages/kit/test/apps/basics/test/cross-platform/test.js index 217607d289f1..4cd9e723db06 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/test.js @@ -282,7 +282,7 @@ test.describe('Errors', () => { expect(await page.textContent('footer')).toBe('Custom layout'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Not found: /why/would/anyone/fetch/this/url"' + 'This is your custom error page saying: "Not Found"' ); expect(/** @type {Response} */ (response).status()).toBe(404); }); diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index cb6b4954e045..cfc1b5a483c3 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1249,7 +1249,7 @@ test.describe('Actions', () => { }); const { type, error } = await response.json(); expect(type).toBe('error'); - expect(error.message).toBe('Actions expect form-encoded data (received application/json)'); + expect(error.message).toBe('Actions expect form-encoded data'); expect(response.status()).toBe(415); }); From 5bf57fb94a4b84631ffd3d130de23ad8763d5915 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 13 Dec 2023 15:01:32 +0100 Subject: [PATCH 04/25] lint --- packages/kit/src/runtime/server/page/actions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index 534a818f04f8..aadeaca1a7ea 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -224,7 +224,7 @@ async function call_action(event, actions) { if (!is_form_content_type(event.request)) { throw new NonFatalError( 415, - `Actions expect form-encoded data`, + 'Actions expect form-encoded data', `Received ${event.request.headers.get('content-type')}` ); } From 067c3faf2213a8ef4437e079691524ccbc05a5cc Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 13 Dec 2023 15:35:02 +0100 Subject: [PATCH 05/25] types --- packages/kit/types/index.d.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index 79e925d0f4cc..a072c01b6437 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -1744,6 +1744,16 @@ declare module '@sveltejs/kit' { status: 301 | 302 | 303 | 307 | 308 | 300 | 304 | 305 | 306; location: string; } + /** + * 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 { + + constructor(status: number, message: string, context?: string | undefined); + status: number; + context: string | undefined; + } } declare module '@sveltejs/kit/hooks' { From a5da208323bf3a03f77442efe2620bdf1f11f3c6 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 13 Dec 2023 16:50:20 +0100 Subject: [PATCH 06/25] adjust wording (is this even a breaking change now?) --- .../docs/60-appendix/30-migrating-to-sveltekit-2.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md b/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md index 2ca1e89e362c..64340efc53e8 100644 --- a/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md +++ b/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md @@ -105,11 +105,11 @@ 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. -## Error handling was slightly altered +## Improved error handling -Some errors are handled internally by SvelteKit, but they are not unexpected. They were handled either by using the `error` function or throwing a regular `Error` on a case-by-case basis. This meant it's a) not easy to distinguish between real unexpected errors and others such as someone calling your action endpoint with the wrong content type and b) introduces a potential bug where properties that may be required due to a custom `App.Error` interface are missing. +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. As a consequence, `handleError` may be called more often now than before. You can distinguish these non-fatal errors by doing an `instanceof NonFatalError` check inside `handleError`. +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 From 8f0fbabce411d1ff7fa5717581065358b1572170 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 13 Dec 2023 16:53:22 +0100 Subject: [PATCH 07/25] adjust --- documentation/docs/30-advanced/20-hooks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/docs/30-advanced/20-hooks.md b/documentation/docs/30-advanced/20-hooks.md index 756b581239ec..034514f0a32d 100644 --- a/documentation/docs/30-advanced/20-hooks.md +++ b/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 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). +- 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: From 6d77aa5f6d744ce7acd9053900d4e9a78a41b139 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 13 Dec 2023 18:00:15 +0100 Subject: [PATCH 08/25] pass status and message to handleError --- documentation/docs/30-advanced/20-hooks.md | 4 +- documentation/docs/30-advanced/25-errors.md | 38 ++----------------- .../30-migrating-to-sveltekit-2.md | 4 +- packages/kit/src/exports/index.js | 3 +- packages/kit/src/exports/public.d.ts | 4 ++ packages/kit/src/exports/vite/dev/index.js | 2 +- packages/kit/src/runtime/client/client.js | 17 ++++++--- packages/kit/src/runtime/control.js | 8 ++-- packages/kit/src/runtime/server/data/index.js | 4 +- .../kit/src/runtime/server/page/actions.js | 12 +++--- packages/kit/src/runtime/server/respond.js | 4 +- packages/kit/src/runtime/server/utils.js | 11 +++--- packages/kit/src/utils/error.js | 13 +++++-- packages/kit/types/index.d.ts | 14 ++----- 14 files changed, 60 insertions(+), 78 deletions(-) diff --git a/documentation/docs/30-advanced/20-hooks.md b/documentation/docs/30-advanced/20-hooks.md index 034514f0a32d..6ceaa1eee67d 100644 --- a/documentation/docs/30-advanced/20-hooks.md +++ b/documentation/docs/30-advanced/20-hooks.md @@ -139,10 +139,10 @@ The following can be added to `src/hooks.server.js` _and_ `src/hooks.client.js`: ### handleError -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: +If an unexpected error is thrown during loading or rendering, this function will be called with the `error`, the `event`, a `status` code and a `message`. 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 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). +- 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` passed in to `handleError`, which doesn't reveal any sensitive information and will be `{ message: 'Internal Error' }` for most errors. 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: diff --git a/documentation/docs/30-advanced/25-errors.md b/documentation/docs/30-advanced/25-errors.md index 9efdc65d7036..a55099f63b1b 100644 --- a/documentation/docs/30-advanced/25-errors.md +++ b/documentation/docs/30-advanced/25-errors.md @@ -77,7 +77,7 @@ By default, unexpected errors are printed to the console (or, in production, you { "message": "Internal Error" } ``` -Unexpected errors will go through the [`handleError`](hooks#shared-hooks-handleerror) hook, where you can add your own error handling — for example, sending errors to a reporting service, or returning a custom error object. +Unexpected errors will go through the [`handleError`](hooks#shared-hooks-handleerror) hook, where you can add your own error handling — for example, sending errors to a reporting service, or returning a custom error object. `handleError` is passed the `error` along with the `event` and a `status` and `message`. `message` is just `"Internal Error"` for unforseen errors, in which case the `status` is 500. `status` may also contain other values such as 404 (page not found) or 415 for actions (wrong content-type), in which case a more specific but still safe `message` is provided. ```js /// file: src/hooks.server.js @@ -95,40 +95,10 @@ import * as Sentry from '@sentry/sveltekit'; Sentry.init({/*...*/}) /** @type {import('@sveltejs/kit').HandleServerError} */ -export function handleError({ error, event }) { - // example integration with https://sentry.io/ - Sentry.captureException(error, { extra: { event } }); - - return { - message: 'Whoops!', - code: error?.code ?? 'UNKNOWN' - }; -} -``` - -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) { +export function handleError({ error, event, status, message }) { + if (status !== 500) { return { - message: error.message, // safe to forward + message, // safe to forward code: 'UNKNOWN' }; } else { diff --git a/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md b/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md index 64340efc53e8..f869a0f1fccf 100644 --- a/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md +++ b/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md @@ -107,9 +107,9 @@ As such, SvelteKit 2 replaces `resolvePath` with a (slightly better named) funct ## 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. +Some errors are handled internally by SvelteKit, but they are not unexpected. They were handled either by using the `error` function or throwing a regular `Error` on a case-by-case basis. This meant it's a) not easy to distinguish between real unexpected errors and others such as someone calling your action endpoint with the wrong content type and b) introduces a potential bug where properties that may be required due to a custom `App.Error` interface are missing. -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`. +SvelteKit 2 cleans this up by providing two new properties to `handleError`: `status` and `message`. For unforseen errors, the `status` is 500 and the `message` just `"Internal Error"`, for internal but handleable errors a more specific status code and message is provided, one which is still safe to show to the user. ## Dynamic environment variables cannot be used during prerendering diff --git a/packages/kit/src/exports/index.js b/packages/kit/src/exports/index.js index 15957287c052..283ba840c2ef 100644 --- a/packages/kit/src/exports/index.js +++ b/packages/kit/src/exports/index.js @@ -1,8 +1,7 @@ -import { HttpError, Redirect, ActionFailure, NonFatalError } from '../runtime/control.js'; +import { HttpError, Redirect, ActionFailure } from '../runtime/control.js'; import { BROWSER, DEV } from 'esm-env'; export { VERSION } from '../version.js'; -export { NonFatalError }; /** * @template {number} TNumber diff --git a/packages/kit/src/exports/public.d.ts b/packages/kit/src/exports/public.d.ts index 8d9f432220fa..f9de82b1c393 100644 --- a/packages/kit/src/exports/public.d.ts +++ b/packages/kit/src/exports/public.d.ts @@ -657,6 +657,8 @@ export type Handle = (input: { export type HandleServerError = (input: { error: unknown; event: RequestEvent; + status: number; + message: string; }) => MaybePromise; /** @@ -668,6 +670,8 @@ export type HandleServerError = (input: { export type HandleClientError = (input: { error: unknown; event: NavigationEvent; + status: number; + message: string; }) => MaybePromise; /** diff --git a/packages/kit/src/exports/vite/dev/index.js b/packages/kit/src/exports/vite/dev/index.js index 539d978acd29..bfe2c089ecf8 100644 --- a/packages/kit/src/exports/vite/dev/index.js +++ b/packages/kit/src/exports/vite/dev/index.js @@ -356,7 +356,7 @@ export async function dev(vite, vite_config, svelte_config) { ActionFailure: control_module_vite.ActionFailure, HttpError: control_module_vite.HttpError, Redirect: control_module_vite.Redirect, - NonFatalError: control_module_vite.NonFatalError + SvelteKitError: control_module_vite.SvelteKitError }); } align_exports(); diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 5bc51a9d3d1f..1a7dd1b81a46 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -30,11 +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, NonFatalError } from '../control.js'; +import { HttpError, Redirect, SvelteKitError } 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'; +import { get_message, get_status } from '../../utils/error.js'; let errored = false; @@ -1060,7 +1060,7 @@ export function create_client(app, target) { navigation_result = await server_fallback( url, { id: null }, - await handle_error(new NonFatalError(404, 'Not Found', `Not found: ${url.pathname}`), { + await handle_error(new SvelteKitError(404, 'Not Found', `Not found: ${url.pathname}`), { url, params: {}, route: { id: null } @@ -1377,10 +1377,17 @@ export function create_client(app, target) { console.warn('The next HMR update will cause the page to reload'); } + const message = get_message(error); + return ( - app.hooks.handleError({ error, event }) ?? + app.hooks.handleError({ + error, + event, + status: get_status(error), + message + }) ?? /** @type {any} */ ({ - message: error instanceof NonFatalError ? error.message : 'Internal Error' + message }) ); } diff --git a/packages/kit/src/runtime/control.js b/packages/kit/src/runtime/control.js index ce61152f26ef..fdbfa73d18f2 100644 --- a/packages/kit/src/runtime/control.js +++ b/packages/kit/src/runtime/control.js @@ -32,9 +32,9 @@ export class Redirect { /** * 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`. + * `SvelteKitError` goes through `handleError`. */ -export class NonFatalError extends Error { +export class SvelteKitError extends Error { /** * @param {number} status * @param {string} message @@ -71,7 +71,7 @@ export class ActionFailure { * ActionFailure: typeof ActionFailure; * HttpError: typeof HttpError; * Redirect: typeof Redirect; - * NonFatalError: typeof NonFatalError; + * SvelteKitError: typeof SvelteKitError; * }} implementations */ export function replace_implementations(implementations) { @@ -82,5 +82,5 @@ export function replace_implementations(implementations) { // @ts-expect-error Redirect = implementations.Redirect; // eslint-disable-line no-class-assign // @ts-expect-error - NonFatalError = implementations.NonFatalError; // eslint-disable-line no-class-assign + SvelteKitError = implementations.SvelteKitError; // eslint-disable-line no-class-assign } diff --git a/packages/kit/src/runtime/server/data/index.js b/packages/kit/src/runtime/server/data/index.js index ae8847cdb548..c0316d278e8d 100644 --- a/packages/kit/src/runtime/server/data/index.js +++ b/packages/kit/src/runtime/server/data/index.js @@ -1,4 +1,4 @@ -import { HttpError, NonFatalError, Redirect } from '../../control.js'; +import { HttpError, SvelteKitError, 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'; @@ -110,7 +110,7 @@ export async function render_data( type: 'error', error: await handle_error_and_jsonify(event, options, error), status: - error instanceof HttpError || error instanceof NonFatalError + error instanceof HttpError || error instanceof SvelteKitError ? error.status : undefined }); diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index aadeaca1a7ea..719ba006543a 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -2,7 +2,7 @@ import * as devalue from 'devalue'; 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, NonFatalError } from '../../control.js'; +import { HttpError, Redirect, ActionFailure, SvelteKitError } from '../../control.js'; import { handle_error_and_jsonify } from '../utils.js'; /** @param {import('@sveltejs/kit').RequestEvent} event */ @@ -24,7 +24,7 @@ export async function handle_action_json_request(event, options, server) { const actions = server?.actions; if (!actions) { - const no_actions_error = new NonFatalError( + const no_actions_error = new SvelteKitError( 405, 'POST method not allowed. No actions exist for this page' ); @@ -141,7 +141,7 @@ export async function handle_action_request(event, server) { }); return { type: 'error', - error: new NonFatalError(405, 'POST method not allowed. No actions exist for this page') + error: new SvelteKitError(405, 'POST method not allowed. No actions exist for this page') }; } @@ -200,7 +200,7 @@ function check_named_default_separate(actions) { /** * @param {import('@sveltejs/kit').RequestEvent} event * @param {NonNullable} actions - * @throws {Redirect | HttpError | NonFatalError | Error} + * @throws {Redirect | HttpError | SvelteKitError | Error} */ async function call_action(event, actions) { const url = new URL(event.request.url); @@ -218,11 +218,11 @@ async function call_action(event, actions) { const action = actions[name]; if (!action) { - throw new NonFatalError(404, `No action with name '${name}' found`); + throw new SvelteKitError(404, `No action with name '${name}' found`); } if (!is_form_content_type(event.request)) { - throw new NonFatalError( + throw new SvelteKitError( 415, 'Actions expect form-encoded data', `Received ${event.request.headers.get('content-type')}` diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index 8d4dcac5cff5..567097184bf5 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -18,7 +18,7 @@ import { exec } from '../../utils/routing.js'; import { redirect_json_response, render_data } from './data/index.js'; import { add_cookies_to_headers, get_cookies } from './cookie.js'; import { create_fetch } from './fetch.js'; -import { HttpError, Redirect, NonFatalError } from '../control.js'; +import { HttpError, Redirect, SvelteKitError } from '../control.js'; import { validate_layout_exports, validate_layout_server_exports, @@ -482,7 +482,7 @@ export async function respond(request, options, manifest, state) { manifest, state, status: 404, - error: new NonFatalError(404, 'Not Found', `Not found: ${event.url.pathname}`), + error: new SvelteKitError(404, 'Not Found', `Not found: ${event.url.pathname}`), resolve_opts }); } diff --git a/packages/kit/src/runtime/server/utils.js b/packages/kit/src/runtime/server/utils.js index 5e7e79d82ba6..36ea81eb180c 100644 --- a/packages/kit/src/runtime/server/utils.js +++ b/packages/kit/src/runtime/server/utils.js @@ -1,8 +1,8 @@ import { DEV } from 'esm-env'; import { json, text } from '../../exports/index.js'; -import { coalesce_to_error, get_status } from '../../utils/error.js'; +import { coalesce_to_error, get_message, get_status } from '../../utils/error.js'; import { negotiate } from '../../utils/http.js'; -import { HttpError, NonFatalError } from '../control.js'; +import { HttpError } from '../control.js'; import { fix_stack_trace } from '../shared-server.js'; import { ENDPOINT_METHODS } from '../../constants.js'; @@ -103,10 +103,11 @@ export async function handle_error_and_jsonify(event, options, error) { fix_stack_trace(error); } + const message = get_message(error); + return ( - (await options.hooks.handleError({ error, event })) ?? { - message: - event.route.id === null && error instanceof NonFatalError ? error.message : 'Internal Error' + (await options.hooks.handleError({ error, event, status: get_status(error), message })) ?? { + message } ); } diff --git a/packages/kit/src/utils/error.js b/packages/kit/src/utils/error.js index 34c9a51f841a..69bfa2d2ed5b 100644 --- a/packages/kit/src/utils/error.js +++ b/packages/kit/src/utils/error.js @@ -1,4 +1,4 @@ -import { HttpError, NonFatalError } from '../runtime/control.js'; +import { HttpError, SvelteKitError } from '../runtime/control.js'; /** * @param {unknown} err @@ -18,7 +18,7 @@ export function coalesce_to_error(err) { * @param {unknown} error */ export function normalize_error(error) { - return /** @type {import('../runtime/control.js').Redirect | HttpError | NonFatalError | Error} */ ( + return /** @type {import('../runtime/control.js').Redirect | HttpError | SvelteKitError | Error} */ ( error ); } @@ -27,5 +27,12 @@ export function normalize_error(error) { * @param {unknown} error */ export function get_status(error) { - return error instanceof HttpError || error instanceof NonFatalError ? error.status : 500; + return error instanceof HttpError || error instanceof SvelteKitError ? error.status : 500; +} + +/** + * @param {unknown} error + */ +export function get_message(error) { + return error instanceof SvelteKitError ? error.message : 'Internal Error'; } diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index a072c01b6437..3769a377d816 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -639,6 +639,8 @@ declare module '@sveltejs/kit' { export type HandleServerError = (input: { error: unknown; event: RequestEvent; + status: number; + message: string; }) => MaybePromise; /** @@ -650,6 +652,8 @@ declare module '@sveltejs/kit' { export type HandleClientError = (input: { error: unknown; event: NavigationEvent; + status: number; + message: string; }) => MaybePromise; /** @@ -1744,16 +1748,6 @@ declare module '@sveltejs/kit' { status: 301 | 302 | 303 | 307 | 308 | 300 | 304 | 305 | 306; location: string; } - /** - * 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 { - - constructor(status: number, message: string, context?: string | undefined); - status: number; - context: string | undefined; - } } declare module '@sveltejs/kit/hooks' { From da812629ffc5dec465326ae6cab48ed3615d945f Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 13 Dec 2023 19:36:39 +0100 Subject: [PATCH 09/25] Apply suggestions from code review Co-authored-by: Rich Harris --- packages/kit/src/runtime/control.js | 6 +++--- packages/kit/src/runtime/server/page/actions.js | 8 ++++---- packages/kit/test/apps/basics/test/test.js | 7 +++---- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/kit/src/runtime/control.js b/packages/kit/src/runtime/control.js index fdbfa73d18f2..0e9aabba6ce6 100644 --- a/packages/kit/src/runtime/control.js +++ b/packages/kit/src/runtime/control.js @@ -38,12 +38,12 @@ export class SvelteKitError extends Error { /** * @param {number} status * @param {string} message - * @param {string} [context] + * @param {string} [details] */ - constructor(status, message, context) { + constructor(status, message, details) { super(message); this.status = status; - this.context = context; + this.details = details; } } diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index 719ba006543a..5371820ccbd5 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -141,7 +141,7 @@ export async function handle_action_request(event, server) { }); return { type: 'error', - error: new SvelteKitError(405, 'POST method not allowed. No actions exist for this page') + error: new SvelteKitError(405, 'Method Not Allowed', 'POST method not allowed. No actions exist for this page') }; } @@ -218,14 +218,14 @@ async function call_action(event, actions) { const action = actions[name]; if (!action) { - throw new SvelteKitError(404, `No action with name '${name}' found`); + throw new SvelteKitError(404, 'Not Found', `No action with name '${name}' found`); } if (!is_form_content_type(event.request)) { throw new SvelteKitError( 415, - 'Actions expect form-encoded data', - `Received ${event.request.headers.get('content-type')}` + 'Unsupported Media Type', + `Form actions expect form-encoded data — received ${event.request.headers.get('content-type')}` ); } diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index e1d5e1bc70bc..6a1b6894d347 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1201,7 +1201,7 @@ test.describe('Actions', () => { }); const { type, error } = await response.json(); expect(type).toBe('error'); - expect(error.message).toBe('Actions expect form-encoded data'); + expect(error.message).toBe('Unsupported Media Type'); expect(response.status()).toBe(415); }); @@ -1209,8 +1209,7 @@ test.describe('Actions', () => { baseURL, page }) => { - const randomActionName = 'some-random-action'; - const response = await page.request.fetch(`${baseURL}/actions/enhance?/${randomActionName}`, { + const response = await page.request.fetch(`${baseURL}/actions/enhance?/doesnt-exist`, { method: 'POST', body: 'irrelevant', headers: { @@ -1219,7 +1218,7 @@ test.describe('Actions', () => { }); const { type, error } = await response.json(); expect(type).toBe('error'); - expect(error.message).toBe(`No action with name '${randomActionName}' found`); + expect(error.message).toBe(`Not Found`); expect(response.status()).toBe(404); }); }); From e4c96b7d3c98a12a7e8328871a0b81898e7bedd0 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 13 Dec 2023 19:44:13 +0100 Subject: [PATCH 10/25] lint --- packages/kit/src/runtime/server/page/actions.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index 5371820ccbd5..f4bfb3fd13a0 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -141,7 +141,11 @@ export async function handle_action_request(event, server) { }); return { type: 'error', - error: new SvelteKitError(405, 'Method Not Allowed', 'POST method not allowed. No actions exist for this page') + error: new SvelteKitError( + 405, + 'Method Not Allowed', + 'POST method not allowed. No actions exist for this page' + ) }; } @@ -225,7 +229,9 @@ async function call_action(event, actions) { throw new SvelteKitError( 415, 'Unsupported Media Type', - `Form actions expect form-encoded data — received ${event.request.headers.get('content-type')}` + `Form actions expect form-encoded data — received ${event.request.headers.get( + 'content-type' + )}` ); } From e5ad6f14571a414e01bd59cd2c75c6c3ea33cbe7 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 13 Dec 2023 15:05:51 -0500 Subject: [PATCH 11/25] Update documentation/docs/30-advanced/20-hooks.md Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> --- documentation/docs/30-advanced/20-hooks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/docs/30-advanced/20-hooks.md b/documentation/docs/30-advanced/20-hooks.md index 6ceaa1eee67d..bf67a19c9629 100644 --- a/documentation/docs/30-advanced/20-hooks.md +++ b/documentation/docs/30-advanced/20-hooks.md @@ -139,7 +139,7 @@ The following can be added to `src/hooks.server.js` _and_ `src/hooks.client.js`: ### handleError -If an unexpected error is thrown during loading or rendering, this function will be called with the `error`, the `event`, a `status` code and a `message`. This allows for two things: +If an unexpected error is thrown during loading or rendering, this function will be called with the `error`, `event`, `status` code and `message`. 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 the `message` passed in to `handleError`, which doesn't reveal any sensitive information and will be `{ message: 'Internal Error' }` for most errors. 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). From f75abdca1c5d60b214939a20923a357556948e9c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 13 Dec 2023 15:07:32 -0500 Subject: [PATCH 12/25] lint --- packages/kit/test/apps/basics/test/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 6a1b6894d347..d153247f1335 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1218,7 +1218,7 @@ test.describe('Actions', () => { }); const { type, error } = await response.json(); expect(type).toBe('error'); - expect(error.message).toBe(`Not Found`); + expect(error.message).toBe('Not Found'); expect(response.status()).toBe(404); }); }); From bbe2c5df1495dd2440fc6ae644395f49dbe248bc Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 13 Dec 2023 15:21:48 -0500 Subject: [PATCH 13/25] simplify example --- documentation/docs/30-advanced/25-errors.md | 25 +++++++++------------ 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/documentation/docs/30-advanced/25-errors.md b/documentation/docs/30-advanced/25-errors.md index a55099f63b1b..344eb4677c36 100644 --- a/documentation/docs/30-advanced/25-errors.md +++ b/documentation/docs/30-advanced/25-errors.md @@ -96,20 +96,17 @@ Sentry.init({/*...*/}) /** @type {import('@sveltejs/kit').HandleServerError} */ export function handleError({ error, event, status, message }) { - if (status !== 500) { - return { - 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' - }; - } + // example integration with https://sentry.io/ + // `error.message` contains diagnostic information which, in general, + // should not be shown to users + Sentry.captureException(error, { + extra: { event, status } + }); + + return { + message, // safe to expose to users + code: error?.code ?? 'UNKNOWN' + }; } ``` From 7f5df13478e24a4ecf3ee2e1220b3e16dda62887 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 13 Dec 2023 15:30:22 -0500 Subject: [PATCH 14/25] tweak docs --- documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md b/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md index bdc52d3143e2..4e6854186a43 100644 --- a/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md +++ b/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md @@ -107,9 +107,9 @@ As such, SvelteKit 2 replaces `resolvePath` with a (slightly better named) funct ## Improved error handling -Some errors are handled internally by SvelteKit, but they are not unexpected. They were handled either by using the `error` function or throwing a regular `Error` on a case-by-case basis. This meant it's a) not easy to distinguish between real unexpected errors and others such as someone calling your action endpoint with the wrong content type and b) introduces a potential bug where properties that may be required due to a custom `App.Error` interface are missing. +Errors are handled inconsistently in SvelteKit 1. Some errors trigger the `handleError` hook but there is no good way to discern their status (for example, the only way to tell a 404 from a 500 is by seeing if `event.route.id` is `null`), while others (such as 405 errors for `POST` requests to pages without actions) don't trigger `handleError` at all, but should. In the latter case, the resulting `$page.error` will deviate from the [`App.Error`](/docs/types#app-error) type, if it is specified. -SvelteKit 2 cleans this up by providing two new properties to `handleError`: `status` and `message`. For unforseen errors, the `status` is 500 and the `message` just `"Internal Error"`, for internal but handleable errors a more specific status code and message is provided, one which is still safe to show to the user. +SvelteKit 2 cleans this up by calling `handleError` hooks with two new properties: `status` and `message`. For errors thrown from your code (or library code called by your code) the status will be `500` and the message will be `Internal Error`. While `error.message` may contain sensitive information that should not be exposed to users, `message` is safe. ## Dynamic environment variables cannot be used during prerendering From 877605f738ed40ace85ab1129b33e4e29aa2234c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 13 Dec 2023 15:30:53 -0500 Subject: [PATCH 15/25] Update documentation/docs/30-advanced/20-hooks.md --- documentation/docs/30-advanced/20-hooks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/docs/30-advanced/20-hooks.md b/documentation/docs/30-advanced/20-hooks.md index bf67a19c9629..3c80dd79f30d 100644 --- a/documentation/docs/30-advanced/20-hooks.md +++ b/documentation/docs/30-advanced/20-hooks.md @@ -139,7 +139,7 @@ The following can be added to `src/hooks.server.js` _and_ `src/hooks.client.js`: ### handleError -If an unexpected error is thrown during loading or rendering, this function will be called with the `error`, `event`, `status` code and `message`. This allows for two things: +If an [unexpected error](/docs/errors#unexpected-errors) is thrown during loading or rendering, this function will be called with the `error`, `event`, `status` code and `message`. 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 the `message` passed in to `handleError`, which doesn't reveal any sensitive information and will be `{ message: 'Internal Error' }` for most errors. 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). From 824198d4382b17807f26f0b6bec559a71c6e787b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 13 Dec 2023 15:45:37 -0500 Subject: [PATCH 16/25] various tweaks. we can be less duplicative i think --- documentation/docs/30-advanced/20-hooks.md | 21 ++++++++----- documentation/docs/30-advanced/25-errors.md | 35 +-------------------- 2 files changed, 15 insertions(+), 41 deletions(-) diff --git a/documentation/docs/30-advanced/20-hooks.md b/documentation/docs/30-advanced/20-hooks.md index 3c80dd79f30d..3176c1ac86b3 100644 --- a/documentation/docs/30-advanced/20-hooks.md +++ b/documentation/docs/30-advanced/20-hooks.md @@ -142,9 +142,11 @@ The following can be added to `src/hooks.server.js` _and_ `src/hooks.client.js`: If an [unexpected error](/docs/errors#unexpected-errors) is thrown during loading or rendering, this function will be called with the `error`, `event`, `status` code and `message`. 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 the `message` passed in to `handleError`, which doesn't reveal any sensitive information and will be `{ message: 'Internal Error' }` for most errors. 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, which defaults to `{ message }`, becomes the value of `$page.error`. -The following code shows an example of typing the error shape as `{ message: string; errorId: string }` and returning it accordingly from the `handleError` functions: +For errors thrown from your code (or library code called by your code) the status will be 500 and the message will be "Internal Error". While `error.message` may contain sensitive information that should not be exposed to users, `message` is safe (if meaningless to the average user). + +To add more information to the `$page.error` object in a type-safe way, you can customize the expected shape by declaring an `App.Error` interface (which must include `message: string`, to guarantee sensible fallback behavior). This allows you to — for example — append a tracking ID for users to quote in correspondence with your technical support staff: ```ts /// file: src/app.d.ts @@ -172,15 +174,17 @@ declare module '@sentry/sveltekit' { // @filename: index.js // ---cut--- import * as Sentry from '@sentry/sveltekit'; -import crypto from 'crypto'; Sentry.init({/*...*/}) /** @type {import('@sveltejs/kit').HandleServerError} */ -export async function handleError({ error, event }) { +export async function handleError({ error, event, status, message }) { const errorId = crypto.randomUUID(); + // example integration with https://sentry.io/ - Sentry.captureException(error, { extra: { event, errorId } }); + Sentry.captureException(error, { + extra: { event, errorId, status } + }); return { message: 'Whoops!', @@ -205,10 +209,13 @@ import * as Sentry from '@sentry/sveltekit'; Sentry.init({/*...*/}) /** @type {import('@sveltejs/kit').HandleClientError} */ -export async function handleError({ error, event }) { +export async function handleError({ error, event, status, message }) { const errorId = crypto.randomUUID(); + // example integration with https://sentry.io/ - Sentry.captureException(error, { extra: { event, errorId } }); + Sentry.captureException(error, { + extra: { event, errorId, status } + }); return { message: 'Whoops!', diff --git a/documentation/docs/30-advanced/25-errors.md b/documentation/docs/30-advanced/25-errors.md index 344eb4677c36..c68e5817a697 100644 --- a/documentation/docs/30-advanced/25-errors.md +++ b/documentation/docs/30-advanced/25-errors.md @@ -77,40 +77,7 @@ By default, unexpected errors are printed to the console (or, in production, you { "message": "Internal Error" } ``` -Unexpected errors will go through the [`handleError`](hooks#shared-hooks-handleerror) hook, where you can add your own error handling — for example, sending errors to a reporting service, or returning a custom error object. `handleError` is passed the `error` along with the `event` and a `status` and `message`. `message` is just `"Internal Error"` for unforseen errors, in which case the `status` is 500. `status` may also contain other values such as 404 (page not found) or 415 for actions (wrong content-type), in which case a more specific but still safe `message` is provided. - -```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'; - -Sentry.init({/*...*/}) - -/** @type {import('@sveltejs/kit').HandleServerError} */ -export function handleError({ error, event, status, message }) { - // example integration with https://sentry.io/ - // `error.message` contains diagnostic information which, in general, - // should not be shown to users - Sentry.captureException(error, { - extra: { event, status } - }); - - return { - message, // safe to expose to users - code: error?.code ?? 'UNKNOWN' - }; -} -``` - -> Make sure that `handleError` _never_ throws an error +Unexpected errors will go through the [`handleError`](hooks#shared-hooks-handleerror) hook, where you can add your own error handling — for example, sending errors to a reporting service, or returning a custom error object which becomes `$page.error`. ## Responses From 7ecbca5ab1d0c0e4f0ca6f25a614254e9ec7a665 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 13 Dec 2023 15:46:17 -0500 Subject: [PATCH 17/25] tweak --- documentation/docs/30-advanced/20-hooks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/docs/30-advanced/20-hooks.md b/documentation/docs/30-advanced/20-hooks.md index 3176c1ac86b3..d979ec88b1b1 100644 --- a/documentation/docs/30-advanced/20-hooks.md +++ b/documentation/docs/30-advanced/20-hooks.md @@ -144,7 +144,7 @@ If an [unexpected error](/docs/errors#unexpected-errors) is thrown during loadin - 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, which defaults to `{ message }`, becomes the value of `$page.error`. -For errors thrown from your code (or library code called by your code) the status will be 500 and the message will be "Internal Error". While `error.message` may contain sensitive information that should not be exposed to users, `message` is safe (if meaningless to the average user). +For errors thrown from your code (or library code called by your code) the status will be 500 and the message will be "Internal Error". While `error.message` may contain sensitive information that should not be exposed to users, `message` is safe (albeit meaningless to the average user). To add more information to the `$page.error` object in a type-safe way, you can customize the expected shape by declaring an `App.Error` interface (which must include `message: string`, to guarantee sensible fallback behavior). This allows you to — for example — append a tracking ID for users to quote in correspondence with your technical support staff: From 24bdfd9350ce084056bee1cab1f4521d3978afc5 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 13 Dec 2023 15:53:13 -0500 Subject: [PATCH 18/25] tweak --- packages/kit/src/runtime/client/client.js | 11 ++--------- packages/kit/src/runtime/control.js | 6 +++--- packages/kit/src/runtime/server/page/actions.js | 1 + packages/kit/src/runtime/server/utils.js | 7 ++----- packages/kit/src/utils/error.js | 2 +- 5 files changed, 9 insertions(+), 18 deletions(-) diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 1a7dd1b81a46..39b326dbc17d 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -1377,18 +1377,11 @@ export function create_client(app, target) { console.warn('The next HMR update will cause the page to reload'); } + const status = get_status(error); const message = get_message(error); return ( - app.hooks.handleError({ - error, - event, - status: get_status(error), - message - }) ?? - /** @type {any} */ ({ - message - }) + app.hooks.handleError({ error, event, status, message }) ?? /** @type {any} */ ({ message }) ); } diff --git a/packages/kit/src/runtime/control.js b/packages/kit/src/runtime/control.js index 0e9aabba6ce6..b55430e88820 100644 --- a/packages/kit/src/runtime/control.js +++ b/packages/kit/src/runtime/control.js @@ -37,13 +37,13 @@ export class Redirect { export class SvelteKitError extends Error { /** * @param {number} status + * @param {string} text * @param {string} message - * @param {string} [details] */ - constructor(status, message, details) { + constructor(status, text, message) { super(message); this.status = status; - this.details = details; + this.text = text; } } diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index f4bfb3fd13a0..64db810507a8 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -26,6 +26,7 @@ export async function handle_action_json_request(event, options, server) { if (!actions) { const no_actions_error = new SvelteKitError( 405, + 'Method Not Allowed', 'POST method not allowed. No actions exist for this page' ); return action_json( diff --git a/packages/kit/src/runtime/server/utils.js b/packages/kit/src/runtime/server/utils.js index 36ea81eb180c..12cb49288408 100644 --- a/packages/kit/src/runtime/server/utils.js +++ b/packages/kit/src/runtime/server/utils.js @@ -103,13 +103,10 @@ export async function handle_error_and_jsonify(event, options, error) { fix_stack_trace(error); } + const status = get_status(error); const message = get_message(error); - return ( - (await options.hooks.handleError({ error, event, status: get_status(error), message })) ?? { - message - } - ); + return (await options.hooks.handleError({ error, event, status, message })) ?? { message }; } /** diff --git a/packages/kit/src/utils/error.js b/packages/kit/src/utils/error.js index 69bfa2d2ed5b..1247b94096aa 100644 --- a/packages/kit/src/utils/error.js +++ b/packages/kit/src/utils/error.js @@ -34,5 +34,5 @@ export function get_status(error) { * @param {unknown} error */ export function get_message(error) { - return error instanceof SvelteKitError ? error.message : 'Internal Error'; + return error instanceof SvelteKitError ? error.text : 'Internal Error'; } From c1265d8069a1a13b5bace74622924d5a181f51e4 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 13 Dec 2023 16:23:31 -0500 Subject: [PATCH 19/25] handle too large body after streaming has started --- packages/kit/src/exports/node/index.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/kit/src/exports/node/index.js b/packages/kit/src/exports/node/index.js index 1d43fd307dad..163495ef0bf4 100644 --- a/packages/kit/src/exports/node/index.js +++ b/packages/kit/src/exports/node/index.js @@ -1,4 +1,5 @@ import * as set_cookie_parser from 'set-cookie-parser'; +import { SvelteKitError } from '../../runtime/control.js'; /** * @param {import('http').IncomingMessage} req @@ -61,12 +62,13 @@ function get_raw_body(req, body_size_limit) { size += chunk.length; if (size > length) { cancelled = true; - controller.error({ - status: 413, - message: `request body size exceeded ${ - content_length ? "'content-length'" : 'BODY_SIZE_LIMIT' - } of ${length}` - }); + + const constraint = content_length ? 'content-length' : 'BODY_SIZE_LIMIT'; + const message = `request body size exceeded ${constraint} of ${length}`; + + const error = new SvelteKitError(413, 'Payload Too Large', message); + controller.error(error); + return; } From 452e264075b24754a6f6985fdda228e18ecf7196 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 13 Dec 2023 16:45:11 -0500 Subject: [PATCH 20/25] cancel stream from the inside if content-length exceeds limit --- packages/kit/src/exports/node/index.js | 28 ++++++++++++-------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/kit/src/exports/node/index.js b/packages/kit/src/exports/node/index.js index 163495ef0bf4..ae3d57afc9cc 100644 --- a/packages/kit/src/exports/node/index.js +++ b/packages/kit/src/exports/node/index.js @@ -22,19 +22,6 @@ function get_raw_body(req, body_size_limit) { return null; } - let length = content_length; - - if (body_size_limit) { - if (!length) { - length = body_size_limit; - } else if (length > body_size_limit) { - throw { - status: 413, - message: `Received content-length of ${length}, but only accept up to ${body_size_limit} bytes.` - }; - } - } - if (req.destroyed) { const readable = new ReadableStream(); readable.cancel(); @@ -46,6 +33,17 @@ function get_raw_body(req, body_size_limit) { return new ReadableStream({ start(controller) { + if (body_size_limit !== undefined && content_length > body_size_limit) { + const error = new SvelteKitError( + 413, + 'Payload Too Large', + `Content-length of ${content_length} exceeds limit of ${body_size_limit} bytes.` + ); + + controller.error(error); + return; + } + req.on('error', (error) => { cancelled = true; controller.error(error); @@ -60,11 +58,11 @@ function get_raw_body(req, body_size_limit) { if (cancelled) return; size += chunk.length; - if (size > length) { + if (size > content_length) { cancelled = true; const constraint = content_length ? 'content-length' : 'BODY_SIZE_LIMIT'; - const message = `request body size exceeded ${constraint} of ${length}`; + const message = `request body size exceeded ${constraint} of ${content_length}`; const error = new SvelteKitError(413, 'Payload Too Large', message); controller.error(error); From 5a914a2fcf36f52f2df745537c6d4d4065ab7993 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 13 Dec 2023 16:48:59 -0500 Subject: [PATCH 21/25] remove unnecessary try-catch, bump adapter-node/adapter-vercel majors --- .changeset/fast-dolls-clean.md | 6 ++++++ packages/adapter-node/package.json | 2 +- packages/adapter-node/src/handler.js | 19 +++++-------------- packages/adapter-vercel/files/serverless.js | 10 +--------- packages/adapter-vercel/package.json | 2 +- packages/kit/src/exports/vite/dev/index.js | 15 ++++----------- .../kit/src/exports/vite/preview/index.js | 15 +++++---------- 7 files changed, 23 insertions(+), 46 deletions(-) create mode 100644 .changeset/fast-dolls-clean.md diff --git a/.changeset/fast-dolls-clean.md b/.changeset/fast-dolls-clean.md new file mode 100644 index 000000000000..824824b64d61 --- /dev/null +++ b/.changeset/fast-dolls-clean.md @@ -0,0 +1,6 @@ +--- +'@sveltejs/adapter-vercel': major +'@sveltejs/adapter-node': major +--- + +breaking: require SvelteKit 2 peer dependency diff --git a/packages/adapter-node/package.json b/packages/adapter-node/package.json index 7532c4c5a106..d3b5c1c7ce58 100644 --- a/packages/adapter-node/package.json +++ b/packages/adapter-node/package.json @@ -50,6 +50,6 @@ "rollup": "^4.8.0" }, "peerDependencies": { - "@sveltejs/kit": "^1.0.0 || ^2.0.0" + "@sveltejs/kit": "^2.0.0" } } diff --git a/packages/adapter-node/src/handler.js b/packages/adapter-node/src/handler.js index bae1854bdd7e..09bf98f63a11 100644 --- a/packages/adapter-node/src/handler.js +++ b/packages/adapter-node/src/handler.js @@ -76,20 +76,11 @@ function serve_prerendered() { /** @type {import('polka').Middleware} */ const ssr = async (req, res) => { - /** @type {Request | undefined} */ - let request; - - try { - request = await getRequest({ - base: origin || get_origin(req.headers), - request: req, - bodySizeLimit: body_size_limit - }); - } catch (err) { - res.statusCode = err.status || 400; - res.end('Invalid request body'); - return; - } + const request = await getRequest({ + base: origin || get_origin(req.headers), + request: req, + bodySizeLimit: body_size_limit + }); setResponse( res, diff --git a/packages/adapter-vercel/files/serverless.js b/packages/adapter-vercel/files/serverless.js index 5a6c2f64e2fd..d732371568c8 100644 --- a/packages/adapter-vercel/files/serverless.js +++ b/packages/adapter-vercel/files/serverless.js @@ -32,15 +32,7 @@ export default async (req, res) => { } } - /** @type {Request} */ - let request; - - try { - request = await getRequest({ base: `https://${req.headers.host}`, request: req }); - } catch (err) { - res.statusCode = /** @type {any} */ (err).status || 400; - return res.end('Invalid request body'); - } + const request = await getRequest({ base: `https://${req.headers.host}`, request: req }); setResponse( res, diff --git a/packages/adapter-vercel/package.json b/packages/adapter-vercel/package.json index e2df6706c99b..7da9c6843e32 100644 --- a/packages/adapter-vercel/package.json +++ b/packages/adapter-vercel/package.json @@ -42,6 +42,6 @@ "vitest": "^1.0.4" }, "peerDependencies": { - "@sveltejs/kit": "^1.5.0 || ^2.0.0" + "@sveltejs/kit": "^2.0.0" } } diff --git a/packages/kit/src/exports/vite/dev/index.js b/packages/kit/src/exports/vite/dev/index.js index bfe2c089ecf8..c1e008dbba50 100644 --- a/packages/kit/src/exports/vite/dev/index.js +++ b/packages/kit/src/exports/vite/dev/index.js @@ -472,17 +472,10 @@ export async function dev(vite, vite_config, svelte_config) { await server.init({ env }); - let request; - - try { - request = await getRequest({ - base, - request: req - }); - } catch (/** @type {any} */ err) { - res.statusCode = err.status || 400; - return res.end('Invalid request body'); - } + const request = await getRequest({ + base, + request: req + }); if (manifest_error) { console.error(colors.bold().red(manifest_error.message)); diff --git a/packages/kit/src/exports/vite/preview/index.js b/packages/kit/src/exports/vite/preview/index.js index a03b40dc2f48..fe44e4524deb 100644 --- a/packages/kit/src/exports/vite/preview/index.js +++ b/packages/kit/src/exports/vite/preview/index.js @@ -122,16 +122,11 @@ export async function preview(vite, vite_config, svelte_config) { vite.middlewares.use(async (req, res) => { const host = req.headers['host']; req.url = req.originalUrl; - let request; - try { - request = await getRequest({ - base: `${protocol}://${host}`, - request: req - }); - } catch (/** @type {any} */ err) { - res.statusCode = err.status || 400; - return res.end('Invalid request body'); - } + + const request = await getRequest({ + base: `${protocol}://${host}`, + request: req + }); setResponse( res, From 95bb3588090c0418cfc0b431bde967aa327b79e1 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 13 Dec 2023 16:52:15 -0500 Subject: [PATCH 22/25] migration docs --- .../docs/60-appendix/30-migrating-to-sveltekit-2.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md b/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md index 4e6854186a43..1ab03abdc277 100644 --- a/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md +++ b/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md @@ -133,6 +133,12 @@ If a form contains an `` but does not have an `enctype="multi Previously, the generated `tsconfig.json` was trying its best to still produce a somewhat valid config when your `tsconfig.json` included `paths` or `baseUrl`. In SvelteKit 2, the validation is more strict and will warn when you use either `paths` or `baseUrl` in your `tsconfig.json`. These settings are used to generate path aliases and you should use [the `alias` config](configuration#alias) option in your `svelte.config.js` instead, to also create a corresponding alias for the bundler. +## `getRequest` no longer throws errors + +The `@sveltejs/kit/node` module exports helper functions for use in Node environments, including `getRequest` which turns a Node [`ClientRequest`](https://nodejs.org/api/http.html#class-httpclientrequest) into a standard [`Request`](https://developer.mozilla.org/en-US/docs/Web/API/Request) object. + +In SvelteKit 1, `getRequest` could throw if the `Content-Length` header exceeded the specified size limit. In SvelteKit 2, the error will not be thrown until later, when the request body (if any) is being read. This enables better diagnostics and simpler code. + ## `vitePreprocess` is no longer exported from `@sveltejs/kit/vite` Since `@sveltejs/vite-plugin-svelte` is now a peer dependency, SvelteKit 2 no longer re-exports `vitePreprocess`. You should import it directly from `@svelte/vite-plugin-svelte`. From b24e8eba5490cb3068e3e7ac6758bbfc154fe35d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 13 Dec 2023 17:38:25 -0500 Subject: [PATCH 23/25] tweak/fix tests --- .../kit/test/apps/basics/src/hooks.server.js | 7 +++-- .../kit/test/apps/basics/test/client.test.js | 8 ++++-- .../basics/test/cross-platform/client.test.js | 2 +- .../apps/basics/test/cross-platform/test.js | 28 ++++++++++--------- .../kit/test/apps/basics/test/server.test.js | 8 +++--- packages/kit/test/apps/basics/test/test.js | 14 +++++++--- 6 files changed, 40 insertions(+), 27 deletions(-) diff --git a/packages/kit/test/apps/basics/src/hooks.server.js b/packages/kit/test/apps/basics/src/hooks.server.js index 22d2f9a79b13..1251601e35af 100644 --- a/packages/kit/test/apps/basics/src/hooks.server.js +++ b/packages/kit/test/apps/basics/src/hooks.server.js @@ -24,7 +24,7 @@ export function error_to_pojo(error) { } /** @type {import('@sveltejs/kit').HandleServerError} */ -export const handleError = ({ event, error: e }) => { +export const handleError = ({ event, error: e, status, message }) => { const error = /** @type {Error} */ (e); // TODO we do this because there's no other way (that i'm aware of) // to communicate errors back to the test suite. even if we could @@ -35,7 +35,10 @@ export const handleError = ({ event, error: e }) => { : {}; errors[event.url.pathname] = error_to_pojo(error); fs.writeFileSync('test/errors.json', JSON.stringify(errors)); - return event.url.pathname.endsWith('404-fallback') ? undefined : { message: error.message }; + + return event.url.pathname.endsWith('404-fallback') + ? undefined + : { message: `${error.message} (${status} ${message})` }; }; export const handle = sequence( diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js index 06a6561fd0b1..d579657e9a05 100644 --- a/packages/kit/test/apps/basics/test/client.test.js +++ b/packages/kit/test/apps/basics/test/client.test.js @@ -339,7 +339,7 @@ test.describe('SPA mode / no SSR', () => { }) => { await page.goto('/no-ssr/ssr-page-config/layout/overwrite'); await expect(page.locator('p')).toHaveText( - 'This is your custom error page saying: "document is not defined"' + 'This is your custom error page saying: "document is not defined (500 Internal Error)"' ); }); }); @@ -800,7 +800,9 @@ test.describe('Streaming', () => { expect(page.locator('p.loadingfail')).toBeVisible(); await expect(page.locator('p.success', { timeout: 15000 })).toHaveText('success'); - await expect(page.locator('p.fail', { timeout: 15000 })).toHaveText('fail'); + await expect(page.locator('p.fail', { timeout: 15000 })).toHaveText( + 'fail (500 Internal Error)' + ); expect(page.locator('p.loadingsuccess')).toBeHidden(); expect(page.locator('p.loadingfail')).toBeHidden(); }); @@ -850,7 +852,7 @@ test.describe('Streaming', () => { expect(page.locator('p.loadingfail')).toBeVisible(); await expect(page.locator('p.success')).toHaveText('success'); - await expect(page.locator('p.fail')).toHaveText('fail'); + await expect(page.locator('p.fail')).toHaveText('fail (500 Internal Error)'); expect(page.locator('p.loadingsuccess')).toBeHidden(); expect(page.locator('p.loadingfail')).toBeHidden(); }); diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index 7392c2465c41..56e2f71d2149 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -496,7 +496,7 @@ test.describe.serial('Errors', () => { expect(await page.textContent('h1')).toBe('Error - 500'); expect(await page.textContent('p')).toBe( - 'This is the static error page with the following message: Failed to load' + 'This is the static error page with the following message: Failed to load (500 Internal Error)' ); }); diff --git a/packages/kit/test/apps/basics/test/cross-platform/test.js b/packages/kit/test/apps/basics/test/cross-platform/test.js index 4cd9e723db06..004921e70a1a 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/test.js @@ -197,7 +197,7 @@ test.describe('Shadowed pages', () => { expect(await page.textContent('h1')).toBe('500'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Data returned from `load` while rendering /shadowed/serialization is not serializable: Cannot stringify arbitrary non-POJOs (data.nope)"' + 'This is your custom error page saying: "Data returned from `load` while rendering /shadowed/serialization is not serializable: Cannot stringify arbitrary non-POJOs (data.nope) (500 Internal Error)"' ); }); } @@ -212,7 +212,7 @@ test.describe('Errors', () => { expect(await page.textContent('footer')).toBe('Custom layout'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Crashing now"' + 'This is your custom error page saying: "Crashing now (500 Internal Error)"' ); }); @@ -242,7 +242,7 @@ test.describe('Errors', () => { : 'in src/routes/errors/invalid-load-response/+page.js'; expect(await page.textContent('#message')).toBe( - `This is your custom error page saying: "a load function ${details} returned an array, but must return a plain object at the top level (i.e. \`return {...}\`)"` + `This is your custom error page saying: "a load function ${details} returned an array, but must return a plain object at the top level (i.e. \`return {...}\`) (500 Internal Error)"` ); }); @@ -261,7 +261,7 @@ test.describe('Errors', () => { expect(await page.textContent('footer')).toBe('Custom layout'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "a load function in src/routes/errors/invalid-server-load-response/+page.server.js returned an array, but must return a plain object at the top level (i.e. `return {...}`)"' + 'This is your custom error page saying: "a load function in src/routes/errors/invalid-server-load-response/+page.server.js returned an array, but must return a plain object at the top level (i.e. `return {...}`) (500 Internal Error)"' ); }); } @@ -271,7 +271,7 @@ test.describe('Errors', () => { expect(await page.textContent('footer')).toBe('Custom layout'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Crashing now"' + 'This is your custom error page saying: "Crashing now (500 Internal Error)"' ); expect(await get_computed_style('h1', 'color')).toBe('rgb(255, 0, 0)'); @@ -282,7 +282,7 @@ test.describe('Errors', () => { expect(await page.textContent('footer')).toBe('Custom layout'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Not Found"' + 'This is your custom error page saying: "Not found: /why/would/anyone/fetch/this/url (404 Not Found)"' ); expect(/** @type {Response} */ (response).status()).toBe(404); }); @@ -319,7 +319,9 @@ test.describe('Errors', () => { } expect(res && res.status()).toBe(500); - expect(await page.textContent('#message')).toBe('This is your custom error page saying: "500"'); + expect(await page.textContent('#message')).toBe( + 'This is your custom error page saying: "500 (500 Internal Error)"' + ); }); test('error in shadow endpoint', async ({ page, read_errors }) => { @@ -335,7 +337,7 @@ test.describe('Errors', () => { expect(res && res.status()).toBe(500); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "nope"' + 'This is your custom error page saying: "nope (500 Internal Error)"' ); }); @@ -357,7 +359,7 @@ test.describe('Errors', () => { expect(await page.textContent('h1')).toBe('500'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Cannot prerender pages with actions"' + 'This is your custom error page saying: "Cannot prerender pages with actions (500 Internal Error)"' ); }); @@ -370,7 +372,7 @@ test.describe('Errors', () => { await clicknav('#get-implicit'); expect(await page.textContent('pre')).toBe( - JSON.stringify({ status: 500, message: 'oops' }, null, ' ') + JSON.stringify({ status: 500, message: 'oops (500 Internal Error)' }, null, ' ') ); const { status, name, message, stack, fancy } = read_errors( @@ -413,7 +415,7 @@ test.describe('Errors', () => { await clicknav('#post-implicit'); expect(await page.textContent('pre')).toBe( - JSON.stringify({ status: 500, message: 'oops' }, null, ' ') + JSON.stringify({ status: 500, message: 'oops (500 Internal Error)' }, null, ' ') ); const { status, name, message, stack, fancy } = read_errors( @@ -508,7 +510,7 @@ test.describe('Redirects', () => { expect(page.url()).toBe(`${baseURL}/redirect/missing-status/a`); expect(await page.textContent('h1')).toBe('500'); expect(await page.textContent('#message')).toBe( - `This is your custom error page saying: "${message}"` + `This is your custom error page saying: "${message} (500 Internal Error)"` ); if (!javaScriptEnabled) { @@ -528,7 +530,7 @@ test.describe('Redirects', () => { expect(page.url()).toBe(`${baseURL}/redirect/missing-status/b`); expect(await page.textContent('h1')).toBe('500'); expect(await page.textContent('#message')).toBe( - `This is your custom error page saying: "${message}"` + `This is your custom error page saying: "${message} (500 Internal Error)"` ); }); diff --git a/packages/kit/test/apps/basics/test/server.test.js b/packages/kit/test/apps/basics/test/server.test.js index c72416aa4f7d..6b11bd8634c3 100644 --- a/packages/kit/test/apps/basics/test/server.test.js +++ b/packages/kit/test/apps/basics/test/server.test.js @@ -289,13 +289,13 @@ test.describe('Errors', () => { test('stack traces are not fixed twice', async ({ page }) => { await page.goto('/errors/stack-trace'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Cannot read properties of undefined (reading \'toUpperCase\')"' + 'This is your custom error page saying: "Cannot read properties of undefined (reading \'toUpperCase\') (500 Internal Error)"' ); // check the stack wasn't mutated await page.goto('/errors/stack-trace'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Cannot read properties of undefined (reading \'toUpperCase\')"' + 'This is your custom error page saying: "Cannot read properties of undefined (reading \'toUpperCase\') (500 Internal Error)"' ); }); @@ -358,7 +358,7 @@ test.describe('Errors', () => { expect(await res_json.json()).toEqual({ type: 'error', error: { - message: 'POST method not allowed. No actions exist for this page' + message: 'POST method not allowed. No actions exist for this page (405 Method Not Allowed)' } }); }); @@ -389,7 +389,7 @@ test.describe('Errors', () => { expect(error.stack).toBe(undefined); expect(res.status()).toBe(500); expect(error).toEqual({ - message: 'Error in handle' + message: 'Error in handle (500 Internal Error)' }); } }); diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index d153247f1335..ccfdd3d6ca73 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -586,7 +586,9 @@ test.describe('Nested layouts', () => { expect(await page.$('p#nested')).not.toBeNull(); expect(await page.$('p#nested-foo')).not.toBeNull(); expect(await page.$('p#nested-bar')).not.toBeNull(); - expect(await page.textContent('#nested-error-message')).toBe('error.message: nope'); + expect(await page.textContent('#nested-error-message')).toBe( + 'error.message: nope (500 Internal Error)' + ); }); test('resets layout', async ({ page }) => { @@ -604,7 +606,9 @@ test.describe('Nested layouts', () => { expect(await page.textContent('h1')).toBe('Nested error page'); expect(await page.textContent('#nested-error-status')).toBe('status: 500'); - expect(await page.textContent('#nested-error-message')).toBe('error.message: nope'); + expect(await page.textContent('#nested-error-message')).toBe( + 'error.message: nope (500 Internal Error)' + ); }); }); @@ -1201,7 +1205,9 @@ test.describe('Actions', () => { }); const { type, error } = await response.json(); expect(type).toBe('error'); - expect(error.message).toBe('Unsupported Media Type'); + expect(error.message).toBe( + 'Form actions expect form-encoded data — received application/json (415 Unsupported Media Type)' + ); expect(response.status()).toBe(415); }); @@ -1218,7 +1224,7 @@ test.describe('Actions', () => { }); const { type, error } = await response.json(); expect(type).toBe('error'); - expect(error.message).toBe('Not Found'); + expect(error.message).toBe("No action with name 'doesnt-exist' found (404 Not Found)"); expect(response.status()).toBe(404); }); }); From d31afbbe82222dfb05c35a8310c9e8d48517f0e9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 13 Dec 2023 17:46:38 -0500 Subject: [PATCH 24/25] fix more --- packages/kit/test/apps/basics/src/hooks.client.js | 4 ++-- packages/kit/test/apps/basics/test/client.test.js | 2 +- .../kit/test/apps/basics/test/cross-platform/client.test.js | 2 +- packages/kit/test/apps/basics/test/cross-platform/test.js | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/kit/test/apps/basics/src/hooks.client.js b/packages/kit/test/apps/basics/src/hooks.client.js index 2dbe1020af0a..8e18bf084c06 100644 --- a/packages/kit/test/apps/basics/src/hooks.client.js +++ b/packages/kit/test/apps/basics/src/hooks.client.js @@ -3,8 +3,8 @@ import { env } from '$env/dynamic/public'; window.PUBLIC_DYNAMIC = env.PUBLIC_DYNAMIC; /** @type{import("@sveltejs/kit").HandleClientError} */ -export function handleError({ error, event }) { +export function handleError({ error, event, status, message }) { return event.url.pathname.endsWith('404-fallback') ? undefined - : { message: /** @type {Error} */ (error).message }; + : { message: `${/** @type {Error} */ (error).message} (${status} ${message})` }; } diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js index d579657e9a05..25c5d61f65fc 100644 --- a/packages/kit/test/apps/basics/test/client.test.js +++ b/packages/kit/test/apps/basics/test/client.test.js @@ -83,7 +83,7 @@ test.describe('Load', () => { test('accessing url.hash from load errors and suggests using page store', async ({ page }) => { await page.goto('/load/url-hash#please-dont-send-me-to-load'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Cannot access event.url.hash. Consider using `$page.url.hash` inside a component instead"' + 'This is your custom error page saying: "Cannot access event.url.hash. Consider using `$page.url.hash` inside a component instea (500 Internal Error)"' ); }); diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index 56e2f71d2149..c7cd0d0707c2 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -457,7 +457,7 @@ test.describe.serial('Errors', () => { expect(await page.textContent('footer')).toBe('Custom layout'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Crashing now"' + 'This is your custom error page saying: "Crashing now (500 Internal Error)"' ); }); diff --git a/packages/kit/test/apps/basics/test/cross-platform/test.js b/packages/kit/test/apps/basics/test/cross-platform/test.js index 004921e70a1a..93b38f17302b 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/test.js @@ -482,7 +482,7 @@ test.describe('Redirects', () => { expect(page.url()).toBe(`${baseURL}/redirect/loopy/a`); expect(await page.textContent('h1')).toBe('500'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Redirect loop"' + 'This is your custom error page saying: "Redirect loop (500 Internal Error)"' ); } else { // there's not a lot we can do to handle server-side redirect loops From 231f2120ba09aedd2fe0430ca5c5207394475dfb Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 13 Dec 2023 17:51:20 -0500 Subject: [PATCH 25/25] more --- packages/kit/test/apps/basics/test/client.test.js | 2 +- .../kit/test/apps/basics/test/cross-platform/client.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js index 25c5d61f65fc..a201b9cea9d8 100644 --- a/packages/kit/test/apps/basics/test/client.test.js +++ b/packages/kit/test/apps/basics/test/client.test.js @@ -83,7 +83,7 @@ test.describe('Load', () => { test('accessing url.hash from load errors and suggests using page store', async ({ page }) => { await page.goto('/load/url-hash#please-dont-send-me-to-load'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Cannot access event.url.hash. Consider using `$page.url.hash` inside a component instea (500 Internal Error)"' + 'This is your custom error page saying: "Cannot access event.url.hash. Consider using `$page.url.hash` inside a component instead (500 Internal Error)"' ); }); diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index c7cd0d0707c2..f32a42d80bf2 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -466,7 +466,7 @@ test.describe.serial('Errors', () => { expect(await page.textContent('footer')).toBe('Custom layout'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Crashing now"' + 'This is your custom error page saying: "Crashing now (500 Internal Error)"' ); });