sveltejs / kit

web development, streamlined
https://kit.svelte.dev
MIT License
18.15k stars 1.84k forks source link

Navigation HTTP error handling #12399

Open hopperelec opened 2 weeks ago

hopperelec commented 2 weeks ago

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

eltigerchino commented 2 weeks ago

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 commented 2 weeks ago

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. image 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.

eltigerchino commented 1 week ago

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

eltigerchino commented 1 week ago

@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 commented 1 week ago

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 commented 2 days ago

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?