Navigation HTTP error handling

[hopperelec]

Describe the problem

I have a "form" like this:

<script>
let value = "";
</script>

<input type="text" placeholder="Game ID..." bind:value/>
<button type="button" on:click={async () => await goto(`/game/${value}/`} disabled={!value}>Join game</button>

Currently, if some error occurs while trying to join the game (e.g: if the game doesn't exist), the error will be shown by the /game/[gameId]/ route, meaning the user needs to navigate back. Ideally, the error message would be displayed as part of the form.

Describe the proposed solution

I'm not sure exactly how the flow from beforeNavigate to onNavigate to afterNavigate works, but I assume SvelteKit would be aware of errors during onNavigate in which case $page.error should be set before the onNavigate callback is called and then onNavigate should have a cancel method similarly to beforeNavigate. Alternatively, there could be a dedicated onNavigationError

Alternatives considered

Have a route which checks if a game exists, then call that before calling goto. However, this means I need to check if the game exists twice, which isn't ideal.

Importance

would make my life easier

Additional Information

No response

[teemingc]

Would preload fit your use case? For example:

<script>
import { preload, goto } from '$app/navigation';

let value =  '';

async function customGoto(href) {
  try {
    await preload(href);
  } catch (error) {
    alert(error.message)
  }
  await goto(href);
}
</script>

<input type="text" placeholder="Game ID..." bind:value/>
<button type="button" on:click={async () => await customGoto(`/game/${value}/`} disabled={!value}>Join game</button>

[hopperelec]

I hadn't actually seen preload so thanks for showing me this! However, I couldn't get it working.

await preload doesn't throw an error. This seems like it should be the working customGoto

async function joinGame(href: string) {
	const preloadResult = await preloadData(href);
	if (preloadResult.type === "loaded" && preloadResult.status !== 200) {
		alert(preloadResult.status);
	} else {
		await goto(href);
	}
}

However, it doesn't work. preload.status always seems to be 200. If I put a console.log(preloadResult) and look in the console, I see this which clearly shows that the page is throwing a 403 error, but preloadResult shows 200. If I look in the network tab, I can see that no 403 status is actually being returned for any requests and that the client is only being informed of the error via the __data.json response body.

{
    "type": "data",
    "nodes": [
        {
            "type": "skip"
        },
        {
            "type": "skip"
        },
        {
            "type": "error",
            "error": {
                "message": "You do not have access to this game!"
            },
            "status": 403
        }
    ]
}

So, based on all this, I'm guessing that this method would only work for errors which would outright prevent a goto from working, not for errors handled by the app.

[teemingc]

Sorry for getting back to you late! I found some time to play around and can confirm there's no obvious way to detect errors because it's been explicitly excluded from the preloadData function return. https://github.com/sveltejs/kit/blob/c9b2e6586369522ba170edf24a124a5446db1640/packages/kit/src/runtime/client/client.js#L1797-L1807

Here's a "workaround" in the meantime: you can check the data property of the returned result and if there are no keys, then it's probably an error (or the load function just doesn't return data on success).

import { preloadData, goto } from '$app/navigation';

async function attemptGoto (pathname) {
	const result = await preloadData(pathname);

	// load function threw an error
	if (result.type === 'loaded' && Object.keys(result.data).length === 0) {
		alert('there was an error so we do not navigate');
		return;
	}

    // load function threw a redirect
	if (result.type === 'redirect') {
		await goto(result.location);
		return;
	}

    // load function was successful
	await goto(pathname);
}

demo

[teemingc]

@Rich-Harris should preloadData know about thrown errors from load functions or should it primarily be used to load the code / data for the next page?

[hopperelec]

That's an interesting workaround. It doesn't work if the target route uses a layout which returns data but, to get around that, I can pass the number of keys (hard-coded) in the layout of the target route to customGoto and check against that. Ideally, though, I'd be able to show the user the specific error message.

[Rich-Harris]

It does feel like result.type should be 'loaded' | 'redirect' | 'error' rather than 'loaded' | 'redirect'. This feels like an oversight. Would fixing that be a breaking change, do we think?

[carmas123]

This is a very simple reproduction:

https://stackblitz.com/edit/sveltejs-kit-template-default-7retxz?file=src%2Froutes%2F%2Bpage.svelte

[hmnd]

Unfortunately, @eltigerchino's workaround doesn't quite work, since preloadData still returns the layout data even if it couldn't load the page data. I've encountered this issue using {#await} in a shallow loaded dialog. It looks like the best workaround for now is to explicitly check if a particular key exists on the result.data object.

[teemingc]

I've drafted a PR for this fix. I think we can agree it's not a breaking change since it never worked correctly in the first place (status always returned 200 even on errors). The new return type should look something like below:

export function preloadData(href: string): Promise<{
	type: 'loaded';
	status: number;
	data: Record<string, any>;
} | {
	type: 'redirect';
	location: string;
} | {
	type: 'error';
	status: number;
	error: App.Error;
}>;

Any thoughts on this?

[hopperelec]

Awesome! This was an existing issue so it might be worth creating a separate issue/PR for this, but you showing the type has made me notice that the status is missing if there's a redirect, which seems a bit pointless. For one, if the status is always provided then it makes any logic dependent on the status a lot simpler since it doesn't need to check the type first. But also, there are many redirect HTTP status codes (300 - 399) which could be used by developers to quickly indicate information which the client might want to know before checking the location.

[teemingc]

the status is missing if there's a redirect, which seems a bit pointless. For one, if the status is always provided then it makes any logic dependent on the status a lot simpler since it doesn't need to check the type first. But also, there are many redirect HTTP status codes (300 - 399) which could be used by developers to quickly indicate information which the client might want to know before checking the location.

Currently, the server doesn't return the status property in the __data.json payload if it's a redirect, so we would need to change that if we decide to standardise this.

[hopperelec]

Hm, that's a bit annoying, but would I be correct in saying that wouldn't be a breaking change since it would just mean adding (not removing/changing) a property? Or is the keyset used for validation or something?

[teemingc]

Hm, that's a bit annoying, but would I be correct in saying that wouldn't be a breaking change since it would just mean adding (not removing/changing) a property? Or is the keyset used for validation or something?

It shouldn't be. I haven't heard any objections around returning the status for redirects yet so I tried adding it. The new interface looks something like this:

	(
		| {
				type: 'loaded';
				data: Record<string, any>;
		  }
		| {
				type: 'redirect';
				location: string;
		  }
		| {
				type: 'error';
				error: App.Error;
		  }
	) & {
		status: number;
	}

[AndreasHald]

Is the PR for this issue delayed until 3.0? seems like a major issue that we can't handle errors when doing shallow or programatic navigation

Doesn't it invalidate the example here? because even if an error is returned in the load you won't go to the else case

if (result.type === 'loaded' && result.status === 200) {
   pushState(href, { selected: result.data });
} else {
   // something bad happened! try navigating
   goto(href);
}

I'd argue that because of this, it's not so much a breaking change as a bug fix