`invalidateAll()` during a navigation causes browser to hang

[austincrim]

Describe the bug

Hey Svelte friends, thanks for all you do 👊.

Not sure if this is a bug or if I'm holding something wrong, but I'm building a tiny site to check sports scores and came across this behavior.

After loading data in +page.server.ts, I show a list of scores by league as well as navigation to switch between leagues. On mount, I set up a kind of crude polling by calling invalidateAll() in a setInterval every 5 seconds (in order to get live updating scores).

After clicking a link to switch leagues, sometimes the site would hang indefinitely until a hard refresh. I think the issue is an invalidation during a navigation, but it could be the other way around.

Reproduction

https://stackblitz.com/edit/sveltejs-kit-template-default-fbr4us?file=src/routes/[league]/+page.svelte

try navigating around several times, you should see the browser hang at some point

CleanShot.2023-03-07.at.10.41.13.mp4

Logs

No response

System Info

System:
    OS: macOS 13.2
    CPU: (8) arm64 Apple M1 Pro
    Memory: 364.58 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.14.2 - ~/.nvm/versions/node/v16.14.2/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v16.14.2/bin/yarn
    npm: 8.5.0 - ~/.nvm/versions/node/v16.14.2/bin/npm
  Browsers:
    Chrome: 110.0.5481.177
    Chrome Canary: 113.0.5636.0
    Firefox: 103.0.2
    Safari: 16.2
  npmPackages:
    @sveltejs/adapter-vercel: ^2.3.0 => 2.3.0 
    @sveltejs/kit: ^1.5.0 => 1.10.0 
    svelte: ^3.54.0 => 3.55.1 
    vite: ^4.0.0 => 4.1.4

Severity

serious, but I can work around it

Additional Information

No response

[dummdidumm]

The problem is that invalidateAll will use the current url to retrigger all the load functions associated with it. Now if goto is started, the url hasn't updated yet, so invalidateAll will use the current (but soon previous) URL to do its invalidation.

The question now is: Which approach is the right one? Does invalidate use the current url, or should it use the next url (but not when prefetching happens)? There might be people out there (unknowingly?) relying on the current behavior.

[austincrim]

For my use case, it would be preferable if the invalidateAll call was cancelled altogether. My first assumption was that inflight invalidations would be cancelled if a navigation was triggered, but that's probably naive thinking.

Has there been discussion of exposing a cancellation mechanism to invalidate and invalidateAll? Maybe using an AbortController?

Now if goto is started, the url hasn't updated yet, so invalidateAll will use the current (but soon previous) URL to do its invalidation.

Maybe a purely academic question, but do you know why this causes the browser to hang and the navigation to never "complete"?

Thanks for your help!

[gtm-nayan]

Maybe a purely academic question, but do you know why this causes the browser to hang and the navigation to never "complete"?

That'd be because of this check,

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

Line 1015 in 39f1631

if (token !== nav_token) return false;

, the part that resets the client state, the navigating store etc. are below it and they never get called if the invalidation changes the token.

For my use case, it would be preferable if the invalidateAll call was cancelled altogether. My first assumption was that inflight invalidations would be cancelled if a navigation was triggered, but that's probably naive thinking.

I suppose we could not run the invalidation if there is a navigation ongoing, but we'd have to decide what needs to happen in the following scenario:

User clicks a link initiating a navigation to a sibling page and while the navigation is ongoing, user clicks a button to invalidate some data in a parent layout

[austincrim]

@gtm-nayan thanks for the follow up!

User clicks a link initiating a navigation to a sibling page and while the navigation is ongoing, user clicks a button to invalidate some data in a parent layout

That's an interesting one. I suppose you would want both the invalidation and navigation in that instance.

In any case, it would be great to have a short-term fix or workaround for my case so that the app doesn't hang forever. Maybe you know off the top of your head, but would the behavior be any different if I used invalidate + depends instead of invalidateAll?

[gtm-nayan]

would the behavior be any different if I used invalidate + depends instead of invalidateAll?

No, it would still hang. I guess the quick fix would be to prioritize navigation, skip invalidation if a navigation is ongoing, cause navigations go ahead regardless of any javascript actions in a page without a client side router.

And then we can try to figure out concurrent navigation + invalidation later, which would probably look like:

1. make navigation request
2. if an invalidation occurs in the meantime, check which nodes were affected by the navigation after it resolves
3. make another request to invalidate the remaining marked nodes

[austincrim]

I'm definitely willing to help with the quick fix, if a contributor has time to provide me some direction (I've never contributed before). Otherwise, I'm fine waiting for someone else to make the PR.

Thanks for your help!

[dummdidumm]

I have seen code before where someone logs out, and instead of redirecting in the action, invalidateAll retriggers the load functions which in turn throw a redirect because some auth check now triggers. So invalidate(All) could also result in route changes, which makes it so hard to decide what the correct behavior is.
To put it another way: should a goto that's triggered shortly after another goto win? I'd say yes (also mimics browser behavior). So should an invalidate(All) which results in a redirect also win? But should an invalidate(All) that's triggered while navigation but does not result in a new page also win? If redirect=wins, no redirect=ignored, how to decide whether to ignore it or not? We can only know until we have the result. I'm undecided yet what the best behavior is.

[elikoga]

We are encountering this using normal invalidates triggered by a websocket

[elikoga]

Either we defer the invalidate calls after navigation or we handle navigation tokens differently