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.

Sign up for GitHub

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

Jump to bottom

SPA Mode: Redirect from +layout.ts doesn't work on 404 #11099

Open
dimfeld opened this issue Nov 22, 2023 · 4 comments · May be fixed by #12005
Open

SPA Mode: Redirect from +layout.ts doesn't work on 404 #11099

dimfeld opened this issue Nov 22, 2023 · 4 comments · May be fixed by #12005
Labels
bug Something isn't working ready to implement please submit PRs for these issues!
Milestone
soon

Comments

@dimfeld
Copy link
Contributor

dimfeld commented Nov 22, 2023

Describe the bug

With the following conditions:

  • SPA mode enabled (ssr = false in +layout.ts)
  • A redirect from the load function in +layout.ts
  • An unknown route (e.g. '/bad_route`)

Expected Behavior: It either redirects or the error page shows up.
Actual Behavior: Neither the error page nor the redirect works. Instead nothing loads at all.

Notes

In the actual case where I encountered this, the redirect was to a login page when we detect that the user is not logged in.

This behavior happens either on initial load or from a client-side navigation, but only when SSR is disabled.

The redirect shows up in the console as an uncaught rejection, which hints at it maybe starting the layout load function but not actually awaiting it once it can't find a page? I haven't looked into it more yet though.

Workaround

Avoid throwing the redirect if route.id is empty. Might not work for unexpected errors though?

Reproduction

https://github.com/dimfeld/sveltekit-redirect-on-404

Load the app at http://localhost:5173/bad_route or click the bad route link from the login page, and see that nothing shows up at all.

Logs

No response

System Info

System:
    OS: macOS 14.1
    CPU: (16) arm64 Apple M3 Max
    Memory: 58.39 GB / 128.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.9.0 - /opt/homebrew/bin/node
    Yarn: 1.22.21 - ~/.pnpm/yarn
    npm: 10.1.0 - /opt/homebrew/bin/npm
    pnpm: 8.10.5 - ~/.pnpm/pnpm
    bun: 1.0.13 - /opt/homebrew/bin/bun
  Browsers:
    Brave Browser: 119.1.60.118
    Safari: 17.1
  npmPackages:
    @sveltejs/adapter-auto: ^1.0.0-next.90 => 1.0.3
    @sveltejs/kit: ^1.0.0-next.587 => 1.27.6
    svelte: ^3.54.0 => 3.59.2
    vite: ^4.0.0 => 4.5.0

Severity

serious, but I can work around it

Additional Information

No response
@eltigerchino eltigerchino added the bug Something isn't working label Nov 22, 2023
@eltigerchino eltigerchino added this to the soon milestone Nov 22, 2023
@eltigerchino
Copy link
Member

eltigerchino commented Nov 22, 2023 •
edited

The issue stems from not handling errors thrown while rendering the root error page.

If an error occurs during hydration, such as a 404 missing route, we load the root error page.

kit/packages/kit/src/runtime/client/client.js

Lines 1829 to 1834 in 07c76b1

result = await load_root_error_page({
status: error instanceof HttpError ? error.status : 500,
error: await handle_error(error, { url, params, route }),
url,
route
});

The root error page includes the root layout, so run those layout files at src/routes/+layout...

kit/packages/kit/src/runtime/client/client.js

Lines 864 to 871 in 07c76b1

const root_layout = await load_node({
loader: default_layout_loader,
url,
params,
route,
parent: () => Promise.resolve({}),
server_data_node: create_data_node(server_data_node)
});

We run the load function, which throws an error (in our case a redirect), and that error isn't caught anywhere.

kit/packages/kit/src/runtime/client/client.js

Line 542 in 07c76b1

data = (await node.universal.load.call(null, load_input)) ?? null;

Ideal solution

We need to catch and handle any redirects or errors thrown from the error helper. For all other errors, we need to display the static fallback error page (according to https://kit.svelte.dev/docs/errors#responses).

when the error occurs inside the root +layout.js or +layout.server.js, since the root layout would ordinarily contain the +error.svelte component. In this case, SvelteKit uses the fallback error page.

We should handle the error thrown by the load_root_error_page call

kit/packages/kit/src/runtime/client/client.js

Lines 1821 to 1835 in 07c76b1

} catch (error) {
if (error instanceof Redirect) {
// this is a real edge case — `load` would need to return
// a redirect but only in the browser
await native_navigation(new URL(error.location, location.href));
return;
}
result = await load_root_error_page({
status: error instanceof HttpError ? error.status : 500,
error: await handle_error(error, { url, params, route }),
url,
route
});
}

@eltigerchino eltigerchino added ready to implement please submit PRs for these issues! and removed ready to implement please submit PRs for these issues! labels Nov 22, 2023
@vocafeuvre
Copy link

vocafeuvre commented Dec 20, 2023 •
edited

I'm getting this error too. In my case, my app is an SPA, and I've implemented a check in load() which calls error(404) whenever a certain value is not retrieved. The page just shows a white screen and in the console:
image

After doing a step through in Dev Tools, I can confirm that the issue plays out just as eltigerchino narrated.

My workaround is to make an error route where I goto() every time I have an error. This won't work, it just threw the page into an infinite loop. What I did was nest my whole routes directory one level down via /(first-level)/, and place an +error.svelte file at the root.

@zommerberg
Copy link
Contributor

Seems to be related to #11494

@manhhungpc manhhungpc mentioned this issue Feb 1, 2024
Closed
6 tasks
@manhhungpc
Copy link

manhhungpc commented Feb 1, 2024 •
edited

I just opened the PR for fallback to default error page when navigating

@eltigerchino eltigerchino linked a pull request Mar 19, 2024 that will close this issue
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working ready to implement please submit PRs for these issues!
Projects
None yet
Milestone
soon
5 participants
@dimfeld @vocafeuvre @zommerberg @eltigerchino @manhhungpc