kit: `+error.svelte` needs to have a `load` function

The +layout.svelte example you gave is exactly what I had in mind. I just don’t agree that it’s a big problem — this…

<title>{$page.error ? 'Error occurred' : $page.data.meta.title}</title>

…seems preferable to having to have this for every single error boundary:

export function load() {
  return {
    meta: {
      title: 'Error occurred'
    }
  };
}

It strikes me as unlikely that you’d often need to vary the messages on a per-error-boundary rather than on a per-status or per-error basis, in which case having the logic centralised in the layout is better. But if you really do need to vary it based on the route, you have access to $page.params and $page.routeId.

I’d argue that the current approach makes common approaches easy and less-common approaches possible. What you’re arguing for makes the less-common approaches slightly easier, but at the cost of a more complex mental model.

The +layout.svelte example you gave is exactly what I had in mind. I just don’t agree that it’s a big problem

@Rich-Harris I must say I’m surprised to hear that, I was confident you’d share my sentiment. That specific example you provided was a one-liner and as such doesn’t really demonstrate the problem. Let’s take a more complex example: Do you genuinely find this… (please take a moment to actually parse each expression in your head)

+layout.svelte:

<svelte:head>
    <title>{($page.error ? `${$page.status} error occurred` : $page.data.meta.title) + $page.error ? titleSuffixes.short ? : $page.data.meta.titleSuffix}</title>
    <meta name="description" content={$page.error ? 'whatever' : $page.data.meta.description} />
</svelte:head>

<Header />

{#if !$page.error && !$page.data.footer.hide}
    <Footer />
{/if}

Header.svelte:

<div class="header {$page.error ? ($page.status == 404 ? 'wide' : 'normal') : $page.data.meta.header.layout}">
    ...
</div>

etc.

…to be cleaner/more scannable/better organized, than this:

+layout.svelte:

<svelte:head>
    <title>{$page.data.meta.title + $page.data.meta.titleSuffix}</title>
    <meta name="description" content={$page.data.meta.description} />
</svelte:head>

<Header />

{#if !$page.data.footer.hide}
    <Footer />
{/if}

Header.svelte:

<div class="header {$page.data.meta.header.layout}">
    ...
</div>

+error.js:

return {
    meta: {
        title: `${status} occured`,
    },
    header: {
        layout: status == 404 ? 'wide' : 'normal',
    },
    footer: {
        hide: true,
    }
};

Again, I feel like it shouldn’t really be controversial. Isn’t it clear which approach is saner? To have to declare a special case (through constant use of ternary operators and so on) in every single place, just to accommodate the lack of the ability to return page data from an error page, frankly feels preposterous to me. But definitely feel free to correct me if I’m missing something.

Well, I’d write the <title> like this for one thing — I wouldn’t have two ternaries when one will do:

<svelte:head>
-    <title>{($page.error ? `${$page.status} error occurred` : $page.data.meta.title) + $page.error ? titleSuffixes.short ? : $page.data.meta.titleSuffix}</title>
+    <title>{($page.error ? `${$page.status} error occurred${titleSuffixes.short}` : $page.data.meta.title + $page.data.meta.titleSuffix}</title>
    <meta name="description" content={$page.error ? 'whatever' : $page.data.meta.description} />
</svelte:head>

(I’m also not sure where titleSuffixes.short is expressed in the alternative code.)

Is the first example preferable to the second? No. Is it preferable to the second if you have multiple error boundaries? Yes, absolutely. The code in the first example isn’t going to make anyone swoon, but it’s fine, and if it really starts to get convoluted then you can just do <title>{getTitle($page)}</title>.

I’m not trying to be awkward here. The ability to run an asynchronous load function when trying to handle errors is a genuine source of complexity. If something goes wrong during that load (which is likely! If the conditions were right for an error to occur in the layout/page load functions, such as the user being offline, then there’s a better-than-normal chance that they’re right for the error load to fail as well), then we have to start making decisions: do we try and walk our way back up the layout tree, trying parent error boundaries? (Depending on what those load functions do, we could be waiting a while before we’re able to actually render something!) Which error do we use — the original error, or the error that was thrown from the error load? Do we bail altogether at that point and just display the root error page? What if we hit an error while running that load?

There’s no objectively right answer to any of those questions, but because we’ve created room for that ambiguity, and make the mental model that much harder to grasp, we have to document all this behaviour, and app developers need to read that documentation and organise their load functions around it. And that’s before people start doing throw redirect(...), creating even more opportunity for cascading failures.

Successfully handling error states requires the elimination of ambiguity.

(It’s not enough to say ‘you can’t fetch stuff in an error load function’ — you need to enforce it, and that means complicating the API. Nor can you ban people from throw error or throw redirect in an error load function — what are you going to do, throw a different error to tell them not to throw errors?)

Against all that, ‘I don’t want to write a ternary in my <title>’ just isn’t a persuasive argument unfortunately.

I think @harrylaulau’s proposal is intriguing. It allows data to be returned without any of the footguns that come with an error load function. We’d miss out on some type safety — assuming we move forward with the App.PageData proposal in #5951, the third argument would have to be Partial<App.PageData> & Record<string, any> rather than App.PageData & Record<string, any> because we wouldn’t have the contextual knowledge of which fields had already been supplied by parent layouts — but only a bit. It feels weird to include that sort of data when throwing an error, but not outrageously so.

The merged data is already available to the +error component.

(Folks might have missed this change: we recently changed how export let data works — instead of only containing the data that was returned from the adjacent load function, it contains shallowly-merged data from all load functions up to that point, so your +error.svelte component can access fetchedCampaigns from the root layout.)

The other reason to use export let data rather than $page.data everywhere is that it gives you a level of type safety that’s impossible with a global data object.

You can access $page.status and $page.error. As long as the errors are sufficiently informative, I believe you can do anything you would have done with a load, albeit with a bit more work.

Given how much of a footgun +error load can be, and how much implementation complexity it adds, I think this is the right trade-off. If the goal is to show a different layout in the case of an error, then +error@whatever.svelte feels like a more promising alternative.

@Rich-Harris I don’t find the justification convincing. If soon something like App.PageData is introduced (which you hinted at here) then we’re effectively defining a contract that “every” page must satisfy (e.g. if, for example, you have a required title: string property in your App.PageData, as per your own example in the comment I linked to), and an error page, is, in fact, a page as well. But currently it isn’t even “able to” conform to the contract, as it cannot have a load function, that’s kind of absurd in my opinion.

I would suggest at least making the load function of an error page more restrictive, like for example it cannot be async (i.e. return a Promise). Wouldn’t that solve the issues you’re concerned about? While also providing the ability to return data and such.

(Folks might have missed this change: we recently changed how export let data works — instead of only containing the data that was returned from the adjacent load function, it contains shallowly-merged data from all load functions up to that point, so your +error.svelte component can access fetchedCampaigns from the root layout.)

This is very good info and resolved why I found this thread! Would be great to have that on https://kit.svelte.dev/docs/errors.

Having to write $page.data everywhere would be cumbersome. If you had to do prop drilling only with export let data that would be cumbersome, too. Also, $page.data contains the whole data of the current page while each layout only has the merged data from itself and its parents accessible through export let data.

Oh wow, thanks! That indeed works great. Makes me kinda wonder why we even have 2 ways to access data? ($page.data and export let data)

I am not very sure if I comprehend everything correctly, but I think this is actually another problem. Please correct me if I misunderstood. I suppose the +error.js is for the return of load of +error.svelte, which then passes the returned value (meta header etc) to +layout.svelte.

The benefit of this is +error.svelte and other +page.svelte sharing the same +layout.svelte. What you need is really passing data from +error.svelte to +layout.svelte.

On first glance, +layout.svelte gets the data from load of other pages / layout, which makes it obvious for error to have load and do the same thing.

However, load in layout and pages could do much more stuff (fetching data, passing data to child etc), which I think is not the purpose of +error.svelte. In think the aim of error.svelte is just to act as a simple error page which other pages / endpoint “throws error at it”, such that it won’t fetch, pass data etc.

Thus I propose maybe we can add an object into throw error(_status_,_errorMessage_,_object_), which would be added / replace key value pairs in $page.data such that layout.svelte can consume the same way as is while no complicated load for error.svelte?

+page.server.js some hardcode

throw error(500,"Some server error",
  {
    meta: {
        title: `500 occured`, // may need hardcode
    },
    header: {
        layout: status == 404 ? 'wide' : 'normal',
    },
    footer: {
        hide: true,
    }
  }
)

+layout.svelte same as above comment

<svelte:head>
    <title>{$page.data.meta.title + $page.data.meta.titleSuffix}</title>
    <meta name="description" content={$page.data.meta.description} />
</svelte:head>


<Header />

{#if !$page.data.footer.hide}
    <Footer />
{/if}

Header.svelte: same as above comment

<div class="header {$page.data.meta.header.layout}">
    ...
</div>

@Rich-Harris

I believe you can do anything you would have done with a load, albeit with a bit more work.

Could you provide an example? I think that would be extremely awkward, especially if you have a bunch of different layout settings. With load you could simply do something like this:

return {
    meta: {
        title: 'Error occured',
    },
    header: {
        layout: 'wide',
    },
    footer: {
        hide: true,
    }
};

While on the other hand trying to achieve the same thing with $page.error entails having a bunch of ugly ternary operators and so on almost everywhere and scattering these configurations (which fundamentally belong to a single page, the error page) throughout different parts of the app:

+layout.svelte:

<svelte:head>
    <title>{$page.error ? 'Error occurred' : $page.data.meta.title}</title>
    <meta name="description" content="{$page.error ? 'whatever' : $page.data.meta.title}" />
</svelte:head>

{#if !$page.error && !$page.data.footer.hide}
    <Footer />
{/if}

Having special cases defined everywhere like this is the definition of ugly code (and could quickly turn into a maintenance nightmare) I think you would agree. This will get exponentially worse if you need to have specific page titles, for example, for specific errors (404, 403, etc.). This is just so unwieldy and unsveltety 😦 But let me know if you have a different workaround in mind. Thank you.

Given how much of a footgun +error load can be, and how much implementation complexity it adds, I think this is the right trade-off.

Don’t you think just mentioning that users need to be mindful about what they put inside an error page’s load function in the documentation would suffice? There’s a variety of things one might want to do in a load function that doesn’t involve potential screw-ups, and I provided the prime example of that. I would argue the framework shouldn’t take this capability away from the developer entirely just because it could cause problems if they do dumb things.