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

Feature Request: Render 404s internally for kit.appDir assets #9802

Closed
Conduitry opened this issue Apr 28, 2023 · 3 comments · Fixed by #11597
Closed

Feature Request: Render 404s internally for kit.appDir assets #9802

Conduitry opened this issue Apr 28, 2023 · 3 comments · Fixed by #11597
Labels
feature / enhancement New feature or request p3-edge-case SvelteKit cannot be used in an uncommon way

Comments

@Conduitry
Copy link
Member

Describe the problem

Requests for paths under /_app/ (or, more generally, under the kit.appDir folder) that do not match an existing asset are passed along to the user's application. Generally, this just results in a waste of time to generate nice styled error responses that no user will see, but in certain situations can actually result in undesirable behavior.

Describe the proposed solution

Requests for all resources under kit.appDir should be handled internally by SvelteKit, never reaching the user's handle hook, even for missing assets.

We'd need to come up with a very generic 404 response for these. There's no real reason for it to be customizable, since this will not be visible to users.

Alternatively, we could render the src/error.html template in this case. It doesn't especially matter.

Alternatives considered

If people want this optimization, they can also do it in userland, by bailing early in their handle hook for /_app/ URLs without calling resolve(event).

Importance

nice to have

Additional Information

Quoting parts of what I wrote in Discord earlier:

To go a little into some of the weeds for my particular use case: Authentication is handled externally, but a certain part of the app, as well as the /_app/ paths, are accessible unauthenticated. My nice shiny 404 page expects to be able to query some APIs with the user's session in order to render it. If someone requests /_app/does_not_exist while unauthenticated, this tries to render the nice 404 page, which tries to hit the API, which it cannot, and which results in a 500 error, and I want in general to avoid all 500s in my logs unless something has actually gone wrong. The easiest way to deal with this for me is going to be to shortcircuit the /_app/ URLs in my handle hook and immediately return a 404 rather than continuing with handling it.

Unless someone's messing around, these [missing /_app/ assets] would only get requested when a user is still on an old version of the app and we're redeployed in the meantime - at which point they get full-page-reloaded and it should stop happening.

@Conduitry Conduitry added feature / enhancement New feature or request p3-edge-case SvelteKit cannot be used in an uncommon way labels Apr 28, 2023
@eltigerchino
Copy link
Member

Posted in discord before I found this thread.

Doing this would be nice for the cloudflare adapter which requires a fallback page for missing assets. The current one is running everyone’s hooks to build the fallback which is annoying for some people #9386

@Conduitry
Copy link
Member Author

I was taking a look at this, and I'm not positive what would be the best way to handle it. Each adapter output is responsible for serving its own appDir assets. In at least some of them (e.g., the Node adapter), this then falls back to the main app when the file is not found. I don't know whether this is how all of the other official adapters work.

We could try to handle this at the adapter level (making it return 404s for missing files under appDir, but I really don't have a clear idea on what would be involved with that for any of the adapters beyond the Node one. I'm not sure whether there'd be a good way to use the src/error.html template from the adapters, in case we wanted to be able to do that. On the plus side, if we did want to consider this a technically breaking change, we could do a major bump in the affected adapter(s) without bumping core Kit's version.

Alternatively, we could bake this into Kit itself, which could presumably happen in a single place, and could affect both dev and preview mode as well. For adapters for serverless platforms, this would be a little less efficient than if we were able to do it at the platform level in some way. From a performance perspective, it would be ideal if we could configure that platform to return a static error file for missing /_app/ assets without needing to hit our application code.

Ultimately, it may make sense to handle it in both places - in KIt itself as a fallback, and in any adapters where it is possible to do so.

I think the next question is whether any of the adapters already currently work this way. By testing the Svelte and SvelteKit sites, I can see that for the Vercel adapter, missing /_app/ assets are not statically handled, and fall back to getting handled by the main app. I do not know what the Netlify, Cloudflare, or Cloudflare Workers adapters do.

@eltigerchino
Copy link
Member

eltigerchino commented May 1, 2023

I do not know what the Netlify, Cloudflare, or Cloudflare Workers adapters do.

The Cloudflare adapter will always bypass the kit server when making requests to /_app/. Therefore, it requires a 404.html, otherwise it will serve a prerendered root index.html (if any), or a blank page with a 404 status. We opted to always generate a fallback page, but this has broken some users' builds when it runs their handle hook during build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature / enhancement New feature or request p3-edge-case SvelteKit cannot be used in an uncommon way
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@Conduitry @eltigerchino and others