[fix] break out of 404 SPA loop

[dummdidumm]

Fixes #6854, but not other cases - when the root +layout(.server).js fails, we also go back to the server for a reload, which would also result in a loop. This could be handled maybe through storing some state in session storage and checking that upon entering an error state again.

Opening this PR regardless to get some discussion going - if we want to explictitly only handle this case for now, or go with a more general solution.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

[changeset-bot]

🦋 Changeset detected

Latest commit: c16fd2c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Rich-Harris]

I remember this being finicky when I last looked into it, but: could we handle the SPA initialisation case differently to how we handle hydration and navigation, and never fall back to the server in that case? That way we don't need to worry about infinite loops, or keeping track in localStorage or whatever (which has caveats, e.g. if you have the right privacy settings in Firefox you can't use it).

[dummdidumm]

That could be another possible solution. We would need to make sure it works for cases where only part of your app is an SPA, too, not sure how that would look like - possibly something like checking that when doing a root layout reload in the error case that the root layout tells you this is csr only.

[Rich-Harris]

In theory we know if a given page load is an SPA page load, because hydrate is false:

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

Lines 30 to 34 in 3c3b6a8

	if (hydrate) {
	await client._hydrate(hydrate);
	} else {
	client.goto(location.href, { replaceState: true });
	}

[dummdidumm]

Good point. Hydrate is false, and the router running means SPA mode. So therefore before falling back to the server we would check if we reload the url we are already on, and if so, prevent that. Essentially the existing SPA check but in a different place.

[dummdidumm]

pushed up a WIP, code is more convoluted than I'd like and the error is incorrect in case of a root error (404 instead of whatever it should be instead), will look at this again tomorrow

[dummdidumm]

Turns out the code was correct already, but I got confused by wrong error messages - turns out, it was/is buggy both on the client and the server:

• on the client it always returned "Internal Error" -> I fixed that here
• on the server, failing data requests (__data.js?..) are treated as "Not Found" because event.routeId is null in this case. Not sure what the right fix is here. Should we add the routeId of the page you're on, and if yes, can we find that out? Else we would need a status code handed to handleError after all. scratch that, one the server routeId is defined, but it's the empty string -> we need to fix the check. On the client it's null which I try to fix now.

[dummdidumm]

Should be good to go now. Besides handling the SPA loop this also fixed some bugs around error handling.