Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Returning non-200 status from load in __layout.svelte causes a Uncaught (in Promise) Error #4815

Closed
poppa opened this issue May 4, 2022 · 3 comments
Labels
error handling load / layout p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc.
Milestone

Comments

@poppa
Copy link

poppa commented May 4, 2022

Describe the bug

Returning a non-200 status from load in __layout.svelte in a Svelte-Kit app results in an uncaught promise error.

It seems to me said function is called recursively when returning a non-200 status. If you track the number of calls and return 200 after the first call to load everything seems to be fine.

Reproduction

https://github.com/poppa/svelte-kit-uncaught-in-promise-sample

Logs

__layout.svelte? [sm]:26 Uncaught (in promise) Error: Authentication Required
    at load (__layout.svelte? [sm]:26:14)
    at load_node (start.js:946:37)
    at Object._hydrate (start.js:1575:25)
    at async start (start.js:1676:3)

System Info

System:
    OS: macOS 11.6
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 53.85 MB / 32.00 GB
    Shell: 3.3.1 - /usr/local/bin/fish
  Binaries:
    Node: 16.13.0 - /usr/local/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 8.1.0 - /usr/local/bin/npm
  Browsers:
    Brave Browser: 101.1.38.111
    Chrome: 101.0.4951.54
    Firefox: 98.0.1
    Safari: 15.0
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.40 
    @sveltejs/kit: next => 1.0.0-next.324 
    svelte: ^3.44.0 => 3.48.0

Severity

serious, but I can work around it

Additional Information

You guys are doing a great job, and Svelte and Svelte-Kit is just awesome.

@elliott-with-the-longest-name-on-github
Copy link
Contributor

Okay, I've been toying with this and I just want to provide some observations.

It appears that the current behavior of non-root __error.svelte and __layout.svelte files is a little confusing. Let's say I have the following folder structure:

.
└── src/
└── routes/
├── test/
│ ├── __error.svelte
│ ├── __layout.svelte
│ └── index.svelte
├── __layout.svelte
└── index.svelte

If an error occurs in the load for /src/routes/test/__layout.svelte, the default SvelteKit error page is displayed. If it occurs in the load for /src/routes/test/index.svelte, the neighboring /src/routes/test/__error.svelte is rendered. Based on what the docs say, this is intended behavior:

For example, if src/routes/settings/notifications/index.svelte failed to load, SvelteKit would render src/routes/settings/notifications/__error.svelte in the same layout, if it existed. If not, it would render src/routes/settings/__error.svelte in the parent layout, or src/routes/__error.svelte in the root layout.

"In the x layout" says to me that the layout has to have finished loading (otherwise, how would you render inside of it)?

With that behavior explained, this bug makes a lot of sense. Of course src/routes/__layout.svelte is being called recursively. By the definitions in the documentation, the root __error.svelte shouldn't ever be able to catch its errors, because the root __error.svelte tries to render "in the root layout". Yuck.

I can think of two solutions:

  1. If src/routes/__layout.svelte fails to load the first time, render src/routes/__error.svelte by itself, outside of the layout.
  2. Default to a SvelteKit-provided "catastrophic failure" page?

However, I could also see 1 being a security vulnerability or a "gotcha" for new developers. For example, I check authentication status in session from the load of my root __layout.svelte to decide what content to display. If that failed, I probably wouldn't want my __error.svelte to break out of my __layout.svelte... we'd have to make sure that __layout.svelte's load did run again on client-side navigation to any other route.

@benmccann benmccann added this to the 1.0 milestone May 4, 2022
@poppa
Copy link
Author

poppa commented May 5, 2022

Thanks for a quick reply 👍

Personally I'm not a fan at all of the error page being rendered inside the main layout. In my world __error.svelte should be its own totally separate layout file, but that's just my opinion.

I personally just didn't expect __error.svelte to issue a call to load in __layout.svelte (which seems to be the case) since it was said function that issued the render of __error.svelte in the first place.

Anyhow, keep up the good work 🥇

@Rich-Harris
Copy link
Member

Since it's now possible to render an error page outside the 'main' layout by moving your routes inside a route group...

src/routes/
├ (main)
│ ├ foo/
│ ├ bar/
│ ├ etc/
│ ├ +layout.svelte # can safely throw errors in here
│ └ +page.svelte
├ +error.svelte   
└ +layout.svelte   # optional

...I'll close this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error handling load / layout p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants