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/.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..d979ec88b1b1 100644 --- a/documentation/docs/30-advanced/20-hooks.md +++ b/documentation/docs/30-advanced/20-hooks.md @@ -139,12 +139,14 @@ 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](/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 `{ 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, 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 (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: ```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 81f216cd2ff3..c68e5817a697 100644 --- a/documentation/docs/30-advanced/25-errors.md +++ b/documentation/docs/30-advanced/25-errors.md @@ -77,36 +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. - -```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 }) { - // 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 +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 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 c6f304b2cc1b..1ab03abdc277 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. +## Improved error handling + +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 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 The `$env/dynamic/public` and `$env/dynamic/private` modules provide access to _run time_ environment variables, as opposed to the _build time_ environment variables exposed by `$env/static/public` and `$env/static/private`. @@ -127,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`. 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/node/index.js b/packages/kit/src/exports/node/index.js index 482e4ae8ac92..ae3d57afc9cc 100644 --- a/packages/kit/src/exports/node/index.js +++ b/packages/kit/src/exports/node/index.js @@ -1,5 +1,5 @@ import * as set_cookie_parser from 'set-cookie-parser'; -import { error } from '../index.js'; +import { SvelteKitError } from '../../runtime/control.js'; /** * @param {import('http').IncomingMessage} req @@ -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) { - error( - 413, - `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,16 +58,15 @@ function get_raw_body(req, body_size_limit) { if (cancelled) return; size += chunk.length; - if (size > length) { + if (size > content_length) { cancelled = true; - controller.error( - error( - 413, - `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 ${content_length}`; + + const error = new SvelteKitError(413, 'Payload Too Large', message); + controller.error(error); + return; } 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 82e662ec7a98..c1e008dbba50 100644 --- a/packages/kit/src/exports/vite/dev/index.js +++ b/packages/kit/src/exports/vite/dev/index.js @@ -355,7 +355,8 @@ export async function dev(vite, vite_config, svelte_config) { control_module_node.replace_implementations({ ActionFailure: control_module_vite.ActionFailure, HttpError: control_module_vite.HttpError, - Redirect: control_module_vite.Redirect + Redirect: control_module_vite.Redirect, + SvelteKitError: control_module_vite.SvelteKitError }); } align_exports(); @@ -471,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, diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 2ae4eacf0a14..39b326dbc17d 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, 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_message, get_status } from '../../utils/error.js'; let errored = false; @@ -704,7 +705,7 @@ export function create_client(app, target) { server_data = await load_data(url, invalid_server_nodes); } catch (error) { return load_root_error_page({ - status: error instanceof HttpError ? error.status : 500, + status: get_status(error), error: await handle_error(error, { url, params, route: { id: route.id } }), url, route @@ -788,7 +789,7 @@ export function create_client(app, target) { }; } - let status = 500; + let status = get_status(err); /** @type {App.Error} */ let error; @@ -798,7 +799,6 @@ export function create_client(app, target) { status = /** @type {import('types').ServerErrorNode} */ (err).status ?? status; error = /** @type {import('types').ServerErrorNode} */ (err).error; } else if (err instanceof HttpError) { - status = err.status; error = err.body; } else { // Referenced node could have been removed due to redeploy, check @@ -1060,7 +1060,7 @@ export function create_client(app, target) { navigation_result = await server_fallback( url, { id: null }, - await handle_error(new Error(`Not found: ${url.pathname}`), { + await handle_error(new SvelteKitError(404, 'Not Found', `Not found: ${url.pathname}`), { url, params: {}, route: { id: null } @@ -1377,12 +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 }) ?? - /** @type {any} */ ({ - message: - event.route.id === null && error instanceof NotFound ? 'Not Found' : 'Internal Error' - }) + app.hooks.handleError({ error, event, status, message }) ?? /** @type {any} */ ({ message }) ); } @@ -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..b55430e88820 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. + * `SvelteKitError` goes through `handleError`. + */ +export class SvelteKitError extends Error { /** - * @param {string} pathname + * @param {number} status + * @param {string} text + * @param {string} message */ - constructor(pathname) { - super(); - - this.status = 404; - this.message = `Not found: ${pathname}`; + constructor(status, text, message) { + super(message); + this.status = status; + this.text = text; } } @@ -66,6 +71,7 @@ export class ActionFailure { * ActionFailure: typeof ActionFailure; * HttpError: typeof HttpError; * Redirect: typeof Redirect; + * SvelteKitError: typeof SvelteKitError; * }} 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 + 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 d8f22e57c404..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, 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'; @@ -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 SvelteKitError + ? error.status + : undefined }); }) ) diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js index bf14575b3438..fde088f714b6 100644 --- a/packages/kit/src/runtime/server/index.js +++ b/packages/kit/src/runtime/server/index.js @@ -81,13 +81,6 @@ export class Server { * @param {import('types').RequestOptions} options */ async respond(request, options) { - // TODO this should probably have been removed for 1.0 — i think we can get rid of it? - if (!(request instanceof Request)) { - throw new Error( - 'The first argument to server.respond must be a Request object. See https://github.com/sveltejs/kit/pull/3384 for details' - ); - } - return respond(request, this.#options, this.#manifest, { ...options, error: false, diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index daf3452098b0..64db810507a8 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 { json } from '../../../exports/index.js'; -import { normalize_error } from '../../../utils/error.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, SvelteKitError } from '../../control.js'; import { handle_error_and_jsonify } from '../utils.js'; /** @param {import('@sveltejs/kit').RequestEvent} event */ @@ -24,9 +24,9 @@ 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 SvelteKitError( 405, + 'Method Not Allowed', 'POST method not allowed. No actions exist for this page' ); return action_json( @@ -84,7 +84,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 +142,11 @@ 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 SvelteKitError( + 405, + 'Method Not Allowed', + 'POST method not allowed. No actions exist for this page' + ) }; } @@ -201,7 +205,7 @@ function check_named_default_separate(actions) { /** * @param {import('@sveltejs/kit').RequestEvent} event * @param {NonNullable} actions - * @throws {Redirect | ActionFailure | HttpError | Error} + * @throws {Redirect | HttpError | SvelteKitError | Error} */ async function call_action(event, actions) { const url = new URL(event.request.url); @@ -219,12 +223,16 @@ async function call_action(event, actions) { const action = actions[name]; if (!action) { - throw new Error(`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 Error( - `Actions expect form-encoded data (received ${event.request.headers.get('content-type')})` + throw new SvelteKitError( + 415, + 'Unsupported Media Type', + `Form 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 f58a0d8fa103..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, NotFound } from '../control.js'; +import { HttpError, Redirect, SvelteKitError } from '../control.js'; import { validate_layout_exports, validate_layout_server_exports, @@ -364,12 +364,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, @@ -488,7 +482,7 @@ export async function respond(request, options, manifest, state) { manifest, state, status: 404, - error: new NotFound(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 ef8277efedcc..12cb49288408 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_message, get_status } from '../../utils/error.js'; import { negotiate } from '../../utils/http.js'; -import { HttpError, NotFound } from '../control.js'; +import { HttpError } 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 @@ -103,11 +103,10 @@ export async function handle_error_and_jsonify(event, options, error) { fix_stack_trace(error); } - return ( - (await options.hooks.handleError({ error, event })) ?? { - message: event.route.id === null && error instanceof NotFound ? 'Not Found' : 'Internal Error' - } - ); + const status = get_status(error); + const message = get_message(error); + + 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 d485dc75ca55..1247b94096aa 100644 --- a/packages/kit/src/utils/error.js +++ b/packages/kit/src/utils/error.js @@ -1,3 +1,5 @@ +import { HttpError, SvelteKitError } from '../runtime/control.js'; + /** * @param {unknown} err * @return {Error} @@ -16,7 +18,21 @@ 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 | SvelteKitError | Error} */ ( error ); } + +/** + * @param {unknown} error + */ +export function get_status(error) { + return error instanceof HttpError || error instanceof SvelteKitError ? error.status : 500; +} + +/** + * @param {unknown} error + */ +export function get_message(error) { + return error instanceof SvelteKitError ? error.text : 'Internal Error'; +} 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/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..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 instead"' + 'This is your custom error page saying: "Cannot access event.url.hash. Consider using `$page.url.hash` inside a component instead (500 Internal Error)"' ); }); @@ -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..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 @@ -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)"' ); }); @@ -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)"' ); }); @@ -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 217607d289f1..93b38f17302b 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: /why/would/anyone/fetch/this/url"' + '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( @@ -480,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 @@ -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 cd3b6934e382..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)' + ); }); }); @@ -1186,6 +1190,43 @@ test.describe('Actions', () => { await expect(page.locator('pre')).toHaveText('something went wrong'); }); + + test('submitting application/json should return http status code 415', async ({ + baseURL, + page + }) => { + const response = await page.request.fetch(`${baseURL}/actions/form-errors`, { + method: 'POST', + body: JSON.stringify({ foo: 'bar' }), + headers: { + 'Content-Type': 'application/json', + Origin: `${baseURL}` + } + }); + const { type, error } = await response.json(); + expect(type).toBe('error'); + expect(error.message).toBe( + 'Form actions expect form-encoded data — received application/json (415 Unsupported Media Type)' + ); + expect(response.status()).toBe(415); + }); + + test('submitting to a form action that does not exists, should return http status code 404', async ({ + baseURL, + page + }) => { + const response = await page.request.fetch(`${baseURL}/actions/enhance?/doesnt-exist`, { + method: 'POST', + body: 'irrelevant', + headers: { + Origin: `${baseURL}` + } + }); + const { type, error } = await response.json(); + expect(type).toBe('error'); + expect(error.message).toBe("No action with name 'doesnt-exist' found (404 Not Found)"); + expect(response.status()).toBe(404); + }); }); // Run in serial to not pollute the log with (correct) cookie warnings diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index b8653b5f179d..59c18f3cf923 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; /**