Simpler, more consistent error handling

[Rich-Harris]

Edited following discussion in #6499 (comment)

SvelteKit handles errors very inconsistently. If you throw an Error in a load function, $page.error will be that very same Error object during server-side rendering, but a POJO (created using somewhat opinionated/convoluted logic) in the client:

<script>
  import { page } from '$app/stores';

  console.log('$page.error', $page.error instanceof Error); // who knows?!
</script>

Another inconsistency: stack traces are included during development, but not in production (because stack traces aren't the sorts of things you should generally make public). But unless you're aware of that distinction, this can just lead to unexpected behaviour.

Meanwhile, error messages (in the case where the error was unexpected, perhaps emanating from the bowels of some library you're using) are also best kept hidden from public view, yet we send them to the client.

All of this confusion extends to JSON representations of these errors. An unexpected error will be an object with a name, message and (in dev) stack plus whatever additional properties happened to be lying around on the original error (e.g. code, frame etc) while expected errors (the kind thrown with throw error(status, message) have status (redundantly), message and __is_http_error (shudder).

Everything would get so much simpler (both implementation-wise and for users) if we eliminated all this ambiguity:

handleError returns a POJO

Unexpected errors already go through handleError, which by default prints the error (complete with sourcemapped stacktrace) to the terminal. That seems like plenty — we don't also need to print the stack trace to the +error.svelte page or include it in a JSON response, especially when doing so creates all the confusion described above.

handleError could be given the responsibility of creating a JSON-friendly non-sensitive-data-containing object representing the error. It would default to this, if no handleFile was specified or no return value was provided...

export function handleError({ event, error }) {
  console.error(error.stack);
  return {
    message: 'Internal Error'
  };
}

...but could include additional metadata:

export function handleError({ event, error }) {
  const { id, traceid } = sendToLoggingService(error);

  return {
    message: 'Something went wrong!',
    metadata: { id, traceid }
  };
}

Make $page.error that same POJO

Instead of playing 'is it an Error or HttpError or a POJO?', we can make $page.error just be a POJO:

<!-- src/routes/+error.svelte -->
<h1>{$page.error.message}</h1>
<p>Our team has been notified. Internal error ID: {$page.error.metadata?.id ?? 'unknown'}</p>

For unexpected errors, we default to replacing message with 'Internal Error' (not 'Internal Server Error', since the same code could throw an unexpected error when it runs on the server or in the browser). If no message is provided (e.g. throw error(404)), we default to just 'Error'.

Throw POJOs with error(...)

When you throw an expected error with throw error(...), the error doesn't go via handleError, because that exists primarily to help you track down unexpected errors. But you still might want to have a richer error object than just a message, for example containing a code that can be used for i18n:

throw error(404, {
  "message": "Not found",
  code: 'NOT_FOUND'
});

Simpler JSON representation

If you hit a route with fetch, the response should just be what was returned from handleError or thrown with throw error(...):

500
{
  "message": "Something went wrong",
  "metadata": {
    "id": "abc123",
    "traceid": "def456"
  }
}

518
{
  "message": "I'm a teapot",
  "code": "I_AM_A_TEAPOT"
}

Type safety

We could type handleError and $page.error by defining an App.PageError interface, as we do with App.Locals and so on:

declare namespace App {
  export interface PageError {
    message: string;
    metadata: {
      id: string;
      traceid: string;
    }
  }
}

One open question is whether message: string would always be a required property. I think it probably needs to be, otherwise the default behaviour of handleError would violate the type constraint. In other words, SvelteKit's types would include this declaration...

declare namespace App {
  export interface PageError {
    message: string;
  }
}

...and app-declared types wouldn't be able to unset it (because that's not how interface merging works).

In future: better client-side error handling

Unexpected errors that happened during client-side navigation would be printed to the console with console.error. Sentry and similar services can deal with this sort of thing, but it might be even nicer if we had some client-side error handling.

In the case of expected client-side errors like the NOT_FOUND one above, $page.error would just be that object:

console.log($page.error); // { message: 'Not found', code: 'NOT_FOUND' }

In the case of _un_expected errors, $page.error would just be a { message: 'Error' } object, unless we can find a good way to design a client-side version of handleError. My first thought was that it could be a lifecycle function, but there really needs to be a single one that's present for the life of the app, rather than something many components could add. (Update: some thoughts on this below: #6499 (comment))

[benmccann]

I agree that having error be an Error or HttpError or a POJO is confusing. I might choose to make it be a POJO rather than a string though. At the very least it would then be extensible in the future even if there were nothing else we decided to put there now

I used to use Play Framework a lot and one nice feature that had was a randomly generated error ID. You could include it in your error page, which was nice because support requests would often have details like "I keep getting error pages. I just got it with error ae02ef", which we could search the logs for and find rather easily.

status is nice on the initial page load, but doesn't really make sense for client-side navigations. I wouldn't mind having it populated only for server-side rendered pages, but won't complain too much if we leave it out. I also think including the stack is nice for server-rendered pages and will advocate for that, but know others feel differently about it and so I don't necessarily expect it to make the cut.

[Rich-Harris]

Yeah, $page.error.message works too. It's actually more consistent with the JSON representation too.

status is nice on the initial page load, but doesn't really make sense for client-side navigations. I wouldn't mind having it populated only for server-side rendered pages, but won't complain too much if we leave it out

It's definitely an abstraction leak, but stuff like this is too useful to disallow it on grounds of purity:

{#if $page.status === 401}
  <p>You must be <a href="/login?redirectTo={$page.url.pathname}">logged in</a> to view this page</p>
{:else if $page.status === 404}
  <p>This page doesn't exist! Try the <a href="/">homepage</a></p>
{:else}
  <p>Oh no! Something went wrong. Our team has been notified</p>
{/if}

[Rich-Harris]

(Also, $page.error.message means this change will be non-breaking for most people)

[Rich-Harris]

For consistency, %sveltekit.error% should be changed to %sveltekit.error.message% I think

[vekunz]

Returning just one string as an error is not really usable for us. We have fairly complex error handling and need more information than just an error message. More precisely, an error message is the only thing, we do NOT need. Because we do client-side localization, we return an error ID. For deeper debugging, we also need to return a Trace ID. We show this Trace ID in the UI in several cases (especially unexcepted errors), so that an end user can send us this trace id, and we can find the exact log lines of this call.
We already have to return a JSON string as an error message to return multiple information to our +error.svelte. And in +page.server.ts files we use the feature to return multiple errors to return multiple information of one error.

[AlexRMU]

An uncaught error and/or error() will be sent to handleError, then there it will be processed/changed and returned, then it will be sent to the client.

[Rich-Harris]

But what does 'sent to the client' mean? We have to serialize it, and since we can't do so with perfect fidelity (e.g. if the error uses a custom class, etc), that means we have to figure out a JSON-friendly (and non-sensitive-detail-leaking) representation. Turning an error into that representation is what handleError is for, under this proposal.

[AlexRMU]

Yes. I mean, handleError takes an uncaught error and/or error() and returns POJO, which will be in $page.error. That is, you can return from the handleError message, metadata and anything you want.

[danawoodman]

I really like the last proposal @Rich-Harris. It allows for flexibility and type safety which is really nice.

I'm basically doing a workaround now for that in my +server files where I have a custom error "thrower" then I catch them in handleError and strip out the metadata and return a formatted JSON error back to the user.

It works well but it would be clunky to do in every Kit app. Having most of this built-in would be the bees knees 🐝 🦵

[vekunz]

Yes. I mean, handleError takes an uncaught error and/or error() and returns POJO, which will be in $page.error. That is, you can return from the handleError message, metadata and anything you want.

I think this is exactly what Rich is talking about.

[wiverson]

I would just like to note the existence of RFC 7807 for error handling. I found implementations floating around for a number of other frameworks and it would be really nice to be able to point to an RFC for Svelte/SvelteKit error handling.

https://www.rfc-editor.org/rfc/rfc7807
https://blog.axway.com/learning-center/apis/api-design/introduction-to-rfc-7807

[wiverson]

I think it's basically a matter of having the fields of the error JSON object that gets thrown esp in an API route map to the fields in the RFC. In other words, when SvelteKit generates an error JSON the fields would just match the naming conventions in the RFC.

The RFC is only a few pages and I figured that standardizing on those for how errors are thrown and caught would be a nice, easy interop story.

Basically, it's a few fields at the top level (details in the RFC link):

type, title, status, detail, instance

So, from a high level I think it's just having the default error object match these fields and also avoiding naming conflicts.

If you would like me to write it up more completely I would be happy to do that as an issue or new discussion. Also happy to generate a PR if that would be helpful, tho I'm not an expert on SvelteKit internals/etc. FWIW having gone through a lot of IEEE, W3C RFCs this one is kind of hilariously simple, just tweaking the error json. Off the cuff the only thing that comes to mind is that it might(?) affect tests expecting the current error message format.

[vekunz]

I do not think this RFC would be a good decision for SvelteKit. It looks like it is primarily for backend systems (public APIs or distributed systems). But in SvelteKit we have only communication between the SvelteKit server and the browser. In my opinion, this RFC would be way too much for something like SvelteKit.

[einarpersson]

But in SvelteKit we have only communication between the SvelteKit server and the browser.

?? Maybe I’m missing your point, but that totally depends on the application. My SK app has a user interface with data visualizations and indeed a public api to expose the underlying data.

(If SK would go in a direction that discourage publishing apis beside UI I’d want to know that because then I’d have to abandon SK and go elsewhere)

(or maybe you are just talking about SK internals, idk. Ignore this then)

[vekunz]

Sure, but public API endpoints can only exposed through +server.ts endpoints, and these endpoints do not rely on the SvelteKits error handling.

[Rich-Harris]

Yeah, I've been struggling to get my head round what this would look like — it would be unfortunate if (for example) errors had to have a title instead of a message in order to be compatible with the RFC. But @vekunz nails it — you'd only really need to return these responses from a +server.js where you already have complete control, so it's probably easier if we don't have any tighter integration than that.

[Rich-Harris]

One possible solution to the client-side error handling question above would be to export a handleError hook from the root layout:

<!-- src/routes/+layout.svelte -->
<script>
  export function handleError({ event, error }) {
    // `event` is a `LoadEvent`, containing `url`, `params`, `routeId` etc
    // `error` is an unexpected error that was thrown

    return {
      message: `A custom message that isn't just 'Error'`,
      some_other_property: 'whatever'
    };
  }
</script>

In effect, +layout.svelte becomes a client-side version of src/hooks.js (only for handleError for now, but if there are other hooks that it makes sense to have client versions of, they could also go here).

The only wrinkle is it wouldn't be able to handle an error that occurred during the initial hydration — that would have to be replaced with a generic error. Otherwise, this seems better than a) introducing a new client-side hooks concept, or b) a lifecycle function with weird semantics

[dummdidumm]

I think it's weirder that having a client hooks file, even if it's only that one hook (for now?). Having a client hooks file could also make this work for the initial hydration, since it can be instantiated before the root layout.

[danawoodman]

src/hooks/client.ts and src/hooks/server.ts? I like that

[Rich-Harris]

Yeah, we decided to go with src/hooks/{server,client}.js

[syffs]

This sample kinda feels uncomplete and misleading:

• are we supposed to use @sentry/svelte or something else ?
• it's quite unclear where Sentry.init is supposed to be called. I'm calling it in hooks.server.js, but not really sure whether there's any impact.

Yeah, we decided to go with src/hooks/{server,client}.js

@Rich-Harris does this mean hooks.{server,client}.js will be soon deprecated ?

[danawoodman]

I think its the other way around, they decided to go with hooks.server/hooks.client instead of a directory