kit
kit copied to clipboard
invalidate() method timeout
Describe the problem
If invalidate() is called and the server generates an error, it appears the client is directed to an error page with the appropriate details. ✅
But if invalidate() is called and there is simply no response, then what? Right now it appears happy to wait forever (where "forever" is at least the 5 minutes I waited). This could lead to inconsistent state if not handled.
Should we be expected to add our own timeout mechanism or could a default timeout option be set or invalidate() allow a timeout parameter that could generate an error on the client?
Describe the proposed solution
Either a global timeout or a per-invalidate timeout, that will generate a timeout error on the client (or a combination of timeout and retry mechanism).
Alternatives considered
No response
Importance
would make my life easier
Additional Information
No response
This plus #5305, plus a separate conversation about allowing non-URL arguments to invalidate (useful for client-side state management etc) all point to the need for a bit of a rethink of the API, so I'm marking this as a breaking change
Allowing a timeout only on invalidate() seems like a weird API if we don't also allow it on regular client-side navigations or on programmatic goto() calls - which seems to indicate to me that the concept of load timeouts should live somewhere else, separate from any of these specific reasons it should be called, but I don't know where that should be.
I was wondering if such an API is really necessary. A user could do something like await Promise.race([new Promise((_, reject) => setTimeout() => reject('timeout'), 1000), invalidate('..')]) to get the behavior themselves. But then I realized that this would not abort the invalidation if there's no other navigation/invalidation in the meantime (if there was, SvelteKit would know that is outdated and ignore the result), so if it would resolve some time later that could be confusing.
I was also thinking about what @Conduitry said but I couldn't come up with a common place for this other than the config file which is too broad/not the right place. A thought I had was adding an input method to load functions that signal that things time out after some time, but that's not good API either. Ultimately I feel this is fine to be added on both invalidate and goto
Coming back to this again, my workaround with Promise.race does work if you do
await Promise.race([
new Promise((_, reject) => setTimeout() => { goto($page.url); reject(); }, 1000),
invalidate('..')
]);
This could be packed up into a little helper function. I'm inclined to move this post 1.0 to wait for more people expressing desire to have something like this built in where such a helper function (which also provides way more control, because you can do more than timeout) wouldn't suffice.
Other ways this could be solved:
- some kind of
abortLoading()function which stops any invalidation/navigation that currently takes place - passing in an abort signal to to invalidate/goto which you call when appropriate
Implementation-wise I think abortLoading is probably easier to implement
Whether we do it now or later, I think the framework should have support built-in for it. And I think it should happen automatically and not require any special APIs. It should probably be configurable both globally and for an individual call
I still don't see why we would want this on programmatic invalidate()/goto() but not have a way to specify it for intercepted link clicks (or for SSR'd content, for that matter). What makes the programmatic calls special that we want to be able to specify a timeout for them, but not the other times the framework is calling load() automatically?
What would happen if we allowed a export const timeout = 1000; in +page.js? Is there a reason to want different timeouts in the four different situations that load() might be getting called?
If we want to be code-over-configuration about this (at least at first) what about a wrapper function around the whole load() function that Promise.races it with a given timeout that can vary between SSR and CSR if the user wants? Higher-order functions that augment the load() function (say, to inject additional arguments) are already a pattern we're recommending. It'd be nice to eventually let people specify those on all of their load() functions at once in a somewhat more DRY way. If we turn the problem in this issue into an augmenting-load() problem, we could eventually solve both at once.
Everything @Conduitry is saying makes sense to me — this isn't an invalidate-specific issue, it belongs to the load itself. A higher-order function is a good solution.
A more declarative approach like export const timeout would be a nice addition, though it could take two different forms:
- pages inherit timeout configuration from parent layouts, as happens with other page options like
prerender - you have to declare
export const timeoutfor theloadfunction you're exporting from the same module
In a certain sense the second makes more sense — src/routes/potato/+page.js shouldn't really have any say over how long is too long to wait for the load function in src/routes/+layout.server.js. But the first would be a hell of a lot easier — set it once in your root layout, and it applies across the entire app, and it works the way you'd expect if you've used the other page options.
Either way it would be a non-breaking change, and the userland solution (higher order functions) can be used today, so I no longer think this needs to be solved pre-1.0.