Prefetch repeatedly refetching after a 40x from a route

[Curstantine]

Describe the bug

SvelteKit repeatedly tries to prefetch a page that throws 40x http codes.
Which should not be the case for 401 and 403, as they are typically used for restricting access to content.

This also adds the issue of hooks.server.ts being spammed with each request.

Reproduction

https://github.com/Curstantine/sveltekit-prefetch-4xx-repeat

Logs

No response

System Info

System:
    OS: Linux 6.0 undefined
    CPU: (4) x64 Intel(R) Core(TM) i5-3210M CPU @ 2.50GHz
    Memory: 4.82 GB / 7.65 GB
    Container: Yes
    Shell: 3.5.1 - /bin/fish
  Binaries:
    Node: 19.3.0 - /usr/bin/node
    Yarn: 1.22.19 - /usr/bin/yarn
    npm: 8.19.2 - /usr/bin/npm
  npmPackages:
    @sveltejs/adapter-auto: ^1.0.0 => 1.0.0 
    @sveltejs/kit: ^1.0.0 => 1.0.1 
    svelte: ^3.54.0 => 3.55.0 
    vite: ^4.0.0 => 4.0.3

Severity

annoyance

Additional Information

No response

[Rich-Harris]

The current behaviour is deliberate — we don't want to store preloaded page data if an error occurred during loading, but nor do we want to prevent preloading if it failed at an earlier time. What change would you suggest?

If you don't want a link to be preloaded, you can disable it with data-sveltekit-preload=data="off" (on the <a> or a parent element): https://kit.svelte.dev/docs/link-options#data-sveltekit-preload-data

[Curstantine]

I see, but I think there should be a way to handle these specific http codes, since the spec for 401 and 403 suggests that:

The client SHOULD NOT automatically repeat the request with the same credentials. The client MAY repeat the request with new or different credentials.

And it would be a concern for people with request limits and such.

I'd be fine with turning off preloading for authenticated routes though, thanks.

[dummdidumm]

One thing we could do is to not try prefetching a resource again if it did respond with an error. I assume the minority of cases would recover from a non-20x response directly on next prefetch, and these cases are rather rare anyway - and since this is mostly about perceived performance of the app, it's probably ok to bail at this point.

[cdcarson]

Maybe related or fixable at the same time. Fresh install of SvelteKit with data-sveltekit-preload-data="hover" in the body tag. If I have a link to a +server.ts GET endpoint I repeatedly get an error in the browser when hovering:

Uncaught (in promise) Error: Attempted to preload a URL that does not belong to this app: http://127.0.0.1:5173/sign-out
    at preload_data (client.js?t=1674082040563&v=92267838:208:10)
    at preload (client.js?t=1674082040563&v=92267838:1192:6)
    at client.js?t=1674082040563&v=92267838:1153:5
preload_data @ client.js?t=1674082040563&v=92267838:208
preload @ client.js?t=1674082040563&v=92267838:1192
(anonymous) @ client.js?t=1674082040563&v=92267838:1153
setTimeout (async)
(anonymous) @ client.js?t=1674082040563&v=92267838:1152

Is this the intended behavior (i.e. the client doesn't differentiate endpoints from pages?) And, if so, shouldn't it just be caught and warned rather than thrown? I can post a separate issue if warranted. Or just work around it.

Also: Shouldn't the default preload be code rather than data? Downloading frontend assets is fine; hitting up the server for data seems dicey. I don't know -- I just came upon data-sveltekit-preload-data="hover" for the first time while trying to diagnose another issue. My hot take is that I'd want to preload data selectively, not everywhere

[Rich-Harris]

@cdcarson good catch, those error messages are fixed in #8961.

Also: Shouldn't the default preload be code rather than data?

The idea is to make navigations instant as often as possible, but with a heuristic that means a navigation is very likely to follow a preload (either a link is tapped, or the mouse comes to rest over a link). With code, you're guaranteed some latency since we can't do stuff in the background. It's certainly a debatable topic; we landed on defaulting to data but made it easy to customise for that reason. I wouldn't say that decision is set in stone, though this is the first pushback I'm aware of; whether that's because people agree with the decision or simply aren't aware of it, I couldn't say.

[cdcarson]

we landed on defaulting to data

I'm not set against it. There's a benefit for certain types of pages. But it seems like it could put more load on the database in an unpredictable way, based on page layout and user behavior.

My untested rule of thumb would be: If a page's data is pre-loadable it should also be cacheable close to the server (mostly hit a key-value store rather than the database.) If the browser-server roundtrip matters then so does the server-data roundtrip. Some pages fit this model, some don't.