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 pages in SPA mode adapter-static #2454

Closed
gyurielf opened this issue Sep 18, 2021 · 11 comments
Closed

Error pages in SPA mode adapter-static #2454

gyurielf opened this issue Sep 18, 2021 · 11 comments
Labels
bug Something isn't working
Milestone

Comments

@gyurielf
Copy link
Contributor

gyurielf commented Sep 18, 2021

Describe the bug

Hello Svelte community!

I ran into a problem with the error pages.
First of all, I use sveltkit with adapter-static and with fallback: '404.html' (So in SPA mode) and I prerender non-dynamic pages with export const prerender = true.

This app served from a cdn, which is simply serve static files.

When I get a 404 error, because the path (and the prerendere file ofc.) isn't exist, if I know correctly, it will be falling back to the fallback 404.html which I set in the adapter-static and the it will handle the things hereinafter.

But the 404 error is always rendered with the root _error.svelte and __layout.svelte.

The SvelteKit ignore the actual path __error.svelte page.
https://kit.svelte.dev/docs#layouts-error-pages

Here is my tree:

my-app
└── src
    └── routes
        ├── app
        │   ├── auth
        │   │   ├── __layout.reset.svelte
        │   │   └── __index.svelte
        │   ├── __error.svelte
        │   ├── __layout.reset.svelte
        │   └── index.svelte
        ├── __error.svelte
        ├── __layout.svelte
        └── index.svelte

So when I hit myurl.com/app/3216836281.
The root error will be rendered for me which is in the routes, not the correct one, which is in the /app.
I misunderstand something?

Reproduction

Set up a project and use it in SPA mode.
Set prerender=true at least one page.
Set a file structure like in the description.
Hit an url, which isn't exist.

Logs

No response

System Info

System:
    OS: Linux 5.11 Ubuntu 20.04.3 LTS (Focal Fossa)
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 3.20 GB / 15.49 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 16.9.1 - /usr/bin/node
    npm: 7.21.1 - /usr/bin/npm
  Browsers:
    Chrome: 93.0.4577.82
    Firefox: 92.0
  npmPackages:
    @sveltejs/adapter-static: 1.0.0-next.19 
    @sveltejs/kit: next => 1.0.0-next.169 
    svelte: 3.42.6

Severity

blocking an upgrade

Additional Information

No response

@benmccann benmccann added this to the 1.0 milestone Sep 19, 2021
@benmccann benmccann added bug Something isn't working help wanted labels Sep 19, 2021
@benmccann
Copy link
Member

@gyurielf could you create a repository that reproduces this? We've got a bunch of open issues and we wouldn't have time to do that for all 200+ of them, so it would really help in getting someone to look at this. Thanks!

@gyurielf
Copy link
Contributor Author

@gyurielf could you create a repository that reproduces this? We've got a bunch of open issues and we wouldn't have time to do that for all 200+ of them, so it would really help in getting someone to look at this. Thanks!

Thanks for the reply!

Sure!

Give me 1-2 day please, and I will share the repo link here.

@dreitzner
Copy link
Contributor

After some debugging I think that the issue lies inside of Renderer._load_error

image

@gyurielf
Copy link
Contributor Author

gyurielf commented Oct 2, 2021

@gyurielf could you create a repository that reproduces this? We've got a bunch of open issues and we wouldn't have time to do that for all 200+ of them, so it would really help in getting someone to look at this. Thanks!

https://github.com/gyurielf/sveltekit-spa-demo

So now you can test it.

for e.g. hit:
http://localhost:3000/app
http://localhost:3000/app/12331
http://localhost:3000/app/auth
http://localhost:3000/app/auth/23213

The errors are always will be rendered by the root's folder __error.svelte.

@ktsangop
Copy link

ktsangop commented Oct 4, 2021

Not absolutely sure but this looks like it's coming from this line :

// TODO backtrack until we find an __error.svelte component

@baseballyama
Copy link
Member

If specified unknown path, should we render like xxx/__error.svelte page?

According to document,
my understanding is that If a specified page is to be rendered and an error occurs,
then xxx/__error.svelte will be rendered.
But in case of 404 (specified path is unknown), should we guess neighborhood xxx/__error.svelte?
I'm not sure.
But I'm worried that it will be security issue.
Because dynamic 404 page give inferred information to a hacker.
(But on the other hand, maybe I think this is not major issue.)

Actually after implementing this dynamic fallback for 404,
I started to think this specification is ok or not.
baseballyama@e244ee7

I want to know actual specification before create PR.

Just should we implement dynamic fallback?
Or should we add a configuration about it? (Default configuration is static error page for 404.)
Or we shouldn't implement it?
Or am I misunderstanding something?

@gyurielf
Copy link
Contributor Author

gyurielf commented Oct 5, 2021

Let's assume we have a logged in user on a dashboard and this user has an old bookmark in his browser or an old link in the browser history.
When he's hit the URL, then I want to show a 404 error (or 500 or any..) on that layout (e.g. in a dashboard) and keep the user on it. This way I can handle errors in a different way for logged in and logged out users.

my-app
└── src
    └── routes
        ├── app
        │   ├── auth
        │   │   ├── __layout.reset.svelte
        │   │   └── __index.svelte
        │   ├── __error.svelte
        │   ├── __layout.reset.svelte
        │   └── index.svelte  --`Here is the logged in user, get errors in this layout`
        ├── __error.svelte
        ├── __layout.svelte
        └── index.svelte --`Here is the logged out user, get errors in this layout`

So I think it's a normal to handle errors just how we would like to handle and where we would like to handle.

@baseballyama
Copy link
Member

Thank you for your comment!

So I think it's a normal to handle errors just how we would like to handle and where we would like to handle.

I checked Nuxt's document to find out how other FWs are doing.
And they supported dynamic fallback for 404 also.

You must see this layout as a component displayed when an error occurs (404, 500, etc.).

So your opinion and my current direction looks okay after refactoring and writing tests.
I will create PR.

@gyurielf
Copy link
Contributor Author

gyurielf commented Oct 9, 2021

@baseballyama Awesome! Thank you. Can I help you somehow ?

@gyurielf
Copy link
Contributor Author

gyurielf commented Dec 9, 2021

It's still actual.
The PR is in(thx for @baseballyama), so someone can review it ?

@Rich-Harris
Copy link
Member

To clarify, this is nothing to do with whether the app is an SPA or not or whether the fallback is correctly configured. Everything is working as intended!

Non-root error pages are rendered when an error occurs while rendering a specific page. A 404 indicates that there was no specific page to render. If you want to render different error pages for paths that begin with something specific like /auth, then there's already a mechanism for that — add a src/routes/auth/[...path].svelte file (replace path with whatever you like), and return a 404 from there:

<script context="module">
  /** @type {import('@sveltejs/kit').Load} */
  export function load({ params }) {
    return {
      status: 404,
      error: new Error(`Not found ${params.path}`)
    };
  }
</script>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants