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

Error page still loaded when redirecting #3051

Closed
LorenzoLeonardini opened this issue Dec 15, 2021 · 5 comments
Closed

Error page still loaded when redirecting #3051

LorenzoLeonardini opened this issue Dec 15, 2021 · 5 comments
Labels
Milestone

Comments

@LorenzoLeonardini
Copy link
Contributor

Describe the bug

This is the scenario: I have a conditional redirect in __layout.svelte which checks if the current path starts with a certain prefix. Let's say "foo". If I go to /foo/bar and routes/foo/bar.svelte exists, I correctly get redirected and the bar page does not get loaded. If I go to /foo/baz and that page does not exist, the __error.svelte page still gets ssr-ed and loaded and sent to the browser.

In my specific project, this becomes a problem because with /foo/baz the error page fails to load because it tries to access unavailable stuff, the server returns an error 500, which overrides my 302 redirect and the user gets stuck on an error page. This wasn't happening in Sapper (I migrated yesterday), so I don't know if it's a bug or a design change.

Reproduction

To reproduce I set up a very simple repo: https://github.com/LorenzoLeonardini/sveltekit-redirect-reproduction
Just visit /foo[whatever] and in the console you get proof the error page gets loaded:

Error: Not found: /fooasd
    at resolve (file:///tmp/my-app/node_modules/@sveltejs/kit/dist/ssr.js:1754:13)
    at async respond (file:///tmp/my-app/node_modules/@sveltejs/kit/dist/ssr.js:1696:10)
    at async svelteKitMiddleware (file:///tmp/my-app/node_modules/@sveltejs/kit/dist/chunks/index.js:4558:22)

If the browser is slow enough you can also see the error page before you get redirected: https://i.imgur.com/5exhl6C.gif

Logs

No response

System Info

System:
    OS: Linux 5.15 Arch Linux
    CPU: (8) x64 Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
    Memory: 635.48 MB / 11.57 GB
    Container: Yes
    Shell: 5.1.12 - /bin/bash
  Binaries:
    Node: 17.1.0 - /usr/bin/node
    Yarn: 1.22.17 - /usr/bin/yarn
    npm: 8.1.3 - /usr/bin/npm
  Browsers:
    Firefox: 95.0
  npmPackages:
    svelte: ^3.44.0 => 3.44.3

Severity

annoyance

@dummdidumm
Copy link
Member

The problem is that the route is matched using the manifest with all possible routes, and matching is done using a regex containing the whole route url. In this case, no route matches, so nothing (no layouts or page) is loaded and instead the root 404 case kicks in. The GIF behavior happens because the root layout contains the redirect logic, and the root layout is always loaded, so the redirect happens on hydration.

For this case to be supported we would need to adjust the route matching logic to instead check each part of the url (for foo/bar, check foo, then bar), use the longest match and run the nodes until you hit a redirect or then backtrack to render the neareset error page.

@LorenzoLeonardini
Copy link
Contributor Author

I think the route matching is fine, and redirecting is the intended behaviour.

What I would expect, though, would be the redirect in the load function to short circuit the current page load and load the new one. Don't know if that's the right thing to do in a general case, though.

@LorenzoLeonardini
Copy link
Contributor Author

LorenzoLeonardini commented Jul 26, 2022

I don't recall the exact problem I was dealing with when I opened the issue, this is my current interpretation. Phrasing wasn't great

@dummdidumm
Copy link
Member

Possibly related: #1575

@Rich-Harris
Copy link
Member

Going to close this as it's intended behaviour. You'd need to have a foo/[...rest] route for the root layout to load outside of the not-found-error-page scenario

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants