Skip to content

Commit

Permalink
breaking: tighten up error handling (#11289)
Browse files Browse the repository at this point in the history
* breaking: tighten up error handling

Introduces a NonFatalError object that is used internally and that is user-detectable in handleError

* how did this happen

* fix tests

* lint

* types

* adjust wording (is this even a breaking change now?)

* adjust

* pass status and message to handleError

* Apply suggestions from code review

Co-authored-by: Rich Harris <richard.a.harris@gmail.com>

* lint

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

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

* lint

* simplify example

* tweak docs

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

* various tweaks. we can be less duplicative i think

* tweak

* tweak

* handle too large body after streaming has started

* cancel stream from the inside if content-length exceeds limit

* remove unnecessary try-catch, bump adapter-node/adapter-vercel majors

* migration docs

* tweak/fix tests

* fix more

* more

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
Co-authored-by: Rich Harris <richard.a.harris@gmail.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
  • Loading branch information
4 people committed Dec 13, 2023
1 parent 57be35a commit 7718d0f
Show file tree
Hide file tree
Showing 31 changed files with 246 additions and 200 deletions.
6 changes: 6 additions & 0 deletions .changeset/fast-dolls-clean.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@sveltejs/adapter-vercel': major
'@sveltejs/adapter-node': major
---

breaking: require SvelteKit 2 peer dependency
5 changes: 5 additions & 0 deletions .changeset/real-pets-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@sveltejs/kit": major
---

breaking: tighten up error handling
23 changes: 15 additions & 8 deletions documentation/docs/30-advanced/20-hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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!',
Expand All @@ -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!',
Expand Down
31 changes: 1 addition & 30 deletions documentation/docs/30-advanced/25-errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 12 additions & 0 deletions documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -127,6 +133,12 @@ If a form contains an `<input type="file">` 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`.
Expand Down
2 changes: 1 addition & 1 deletion packages/adapter-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,6 @@
"rollup": "^4.8.0"
},
"peerDependencies": {
"@sveltejs/kit": "^1.0.0 || ^2.0.0"
"@sveltejs/kit": "^2.0.0"
}
}
19 changes: 5 additions & 14 deletions packages/adapter-node/src/handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 1 addition & 9 deletions packages/adapter-vercel/files/serverless.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion packages/adapter-vercel/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,6 @@
"vitest": "^1.0.4"
},
"peerDependencies": {
"@sveltejs/kit": "^1.5.0 || ^2.0.0"
"@sveltejs/kit": "^2.0.0"
}
}
43 changes: 20 additions & 23 deletions packages/kit/src/exports/node/index.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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();
Expand All @@ -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);
Expand All @@ -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;
}

Expand Down
4 changes: 4 additions & 0 deletions packages/kit/src/exports/public.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,8 @@ export type Handle = (input: {
export type HandleServerError = (input: {
error: unknown;
event: RequestEvent;
status: number;
message: string;
}) => MaybePromise<void | App.Error>;

/**
Expand All @@ -668,6 +670,8 @@ export type HandleServerError = (input: {
export type HandleClientError = (input: {
error: unknown;
event: NavigationEvent;
status: number;
message: string;
}) => MaybePromise<void | App.Error>;

/**
Expand Down
18 changes: 6 additions & 12 deletions packages/kit/src/exports/vite/dev/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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));
Expand Down
15 changes: 5 additions & 10 deletions packages/kit/src/exports/vite/preview/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 7718d0f

Please sign in to comment.