Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Navigating to SvelteKit error page after a new build causes infinite page reload loop #65

Open
austerj opened this issue Sep 24, 2023 · 5 comments

Comments

@austerj
Copy link

austerj commented Sep 24, 2023

Using generateSW for my service worker, when I boot up a new build and do a page refresh or navigate to a route that throws a SvelteKit error (either from a load function in +page.ts or just a missing route), the client seems to get stuck in an infinite loop of navigating to the same page that generated the error and requesting /_app/version.json from the server.

Workbox repeatedly reports a full page navigation in the log (always to the same page) and triggering the associated invalidation of page data, and eventually things get halted by Chrome navigation throttling.

The issue only occurs on routes that throw errors from a +page.ts load function or are missing entirely (triggering the SvelteKit fallback error page). If I "start" on any non-error-generating page and only navigate to the error-generating one after the update, error handling works as normal again. Same goes for offline mode (since no new SW versions are found).

Service workers are still pretty new to me, so I haven't been able to identify what could be the trigger of this conflict between the fetch of new updates and SvelteKit errors. I would have assumed the behavior of purely client-side +error.svelte / +page.load interactions would behave similarly to just in-lining an {#if error} ... page in page.svelte, yet one plays nice with the service worker and the other does not.

I've been able to replicate it both in local builds and deployed builds on Vercel, so can exclude that it's related to http vs. https or any other local dev environment factors. Happy to provide more info / reproduce in a fresh repo if needed, otherwise will post here if I find a fix.

@userquin
Copy link
Member

userquin commented Sep 25, 2023

You should exclude server pages from the sw interception and use NetworkFirst/NetworkOnly runtime cahcing handler, checking for offline in the page (or before allowing the user navigating to those ssr pages). My suggestion is to exclude SSR pages from sw interception (use workbox.navegateFallbackDenylist), or at least exclude dynamic ones, you can use a custom runtime caching to include those SSR pages not being dynamic, for example a list (the content will vary but the route is static) (will not work on first navigation, that's, not being included in the cache in first server response, the cache needs to be created and cannot be done in the request).

Check the code in this PR, you can group pages in the same cache or just add a cache per route (not optimal): vite-pwa/nuxt#66

You can use this to exclude routes from sw interception, for example, excluding /list route:

workbox.navigateFallbackDenylist = [/^\/list$/]

To add a custom runtime caching without any cache and following with the /list route example (beware, you also need to include all the routes in navigateFallbackDenylist , and when offline requesting the page will fail):

workbox.runtimeCaching = [{
    urlPattern: ({ url, sameOrigin }) => sameOrigin && url.pathname.match(/^\/list$/),
    handler: 'NetworkOnly',
    options: {
      /* check the options in the workbox-build docs*/
      matchOptions: {
        ignoreVary: true,
        ignoreSearch: true
      },
      plugins: [{
        /* this callback will be called when the fetch call fails */
        handlerDidError: async () => Response.redirect('/error?offline', 302),
        /* this callback will prevent caching the response */
        cacheWillUpdate: async () => null
      }]
    }
}]

To add a custom runtime caching with cache and following with the /list route example (beware, you also need to include all the routes in navigateFallbackDenylist , and when offline requesting the page will fail if cache not created, should only happen in first request and then going offline and refreshing the page):

workbox.runtimeCaching = [{
    urlPattern: ({ url, sameOrigin }) => sameOrigin && url.pathname.match(/^\/list$/),
    handler: 'NetworkFirst',
    options: {
      cacheName: 'ssr-pages-cache',
      /* cache only 200 responses */
      cacheableResponse: {
        statuses: [200]
      },
      /* check the options in the workbox-build docs*/
      matchOptions: {
        ignoreVary: true,
        ignoreSearch: true
      },
      plugins: [{
        /* this callback will be called when the fetch call fails */
        handlerDidError: async () => Response.redirect('/error?offline', 302),
        /* this callback will prevent caching the response when not 200 */
        cacheWillUpdate: async ({ response }) => response.status === 200 ? response : null
      }]
    }
}]

OFC you can mix both strategies, adding multiple entries to the runtimeCaching array.

EDIT: in previous examples, update the error redirection to an existing error page or custom error page, if it is only in client side only it would be better, I don't remember if kit will create the error.html page (if so the html page should be in the sw precaching manifest)

@userquin
Copy link
Member

If you're using injectManifest strategy (custom sw), you've the equivalent via import { registerRoute } from 'workbox-routing', here an example, also with the corresponding denylist => navegateFallbackDenylist : https://github.com/elk-zone/elk/blob/main/service-worker/sw.ts

@austerj
Copy link
Author

austerj commented Sep 25, 2023

Thank you so much for taking the time to reply @userquin, I really appreciate it.

To give some more context: I am using dynamic routing for managing client-side data stored in local IndexedDB that is periodically synced in the background when online, i.e. there is no actual server-side dependencies involved in rendering these routes prior to the service worker entering the picture (and the project includes nested slugs - so keeping the clean SvelteKit codebase would be nice). It is a pretty traditional SPA with SSR disabled entirely for the main app group, though of course not for e.g. the API.

Giving up offline access for these routes in my case would unfortunately defeat the main use case of the service worker, not to mention that the server would have no work to do for these routes - and they actually are already working perfectly fine offline! It is only when online and detection of a new version occurs that trouble starts brewing. The error page propagation works as expected when offline or when no new updates have been deployed.

I also believe that even with this approach it wouldn't solve the issue completely, as one would always be able to trigger this same behavior by going to any route that falls outside the deny list + runtime caching pattern (leading to the standard SvelteKit 404 error page and this infinite request loop). Maybe this could be addressed by inverting the solution and using an allow list instead, but in any case, it won't work for me sadly.

For now I resorted to runtime caching for version.json for a short duration, which at least stops the flurry of server requests and lets the client-side navigation throttling kick in, though I suppose this will be highly browser dependent. It also should be quite an extreme edge case once live unless someone actively tries to reproduce it, as the service worker should get updated continuously and there should be few ways for a user to end up on an error page in the first place. Still, it would of course be preferable not to have to think about this.

I'll post an update if / when I've made any progress on understanding what causes this behavior.

@austerj
Copy link
Author

austerj commented Sep 25, 2023

Actually the example @userquin posted made me aware of some workbox options I hadn't considered before, which made me want to experiment with a more robust extension of the version.json caching trick.

I used the following:

workbox: {
    runtimeCaching: [
        {
            urlPattern: ({ url }) => {
                return url.pathname === '/_app/version.json'
            },
            handler: 'CacheFirst' as const,
            options: {
                cacheName: 'version-cache',
                cacheableResponse: {
                    statuses: [200],
                },
                expiration: {
                    maxAgeSeconds: 30,
                },
                plugins: [
                    {
                        cachedResponseWillBeUsed: async () =>
                            Response.redirect('/', 302),
                    },
                ],
            },
        },
    ],
}

...and expected this to make the service worker "detect" the request loop by accessing the cached response and exit the loop by redirecting to root, which should then at least update as normal. To my surprise, this didn't only fix the loop, but it did so without even losing the state of navigation.

The version.json is fetched, then the cached one is accessed, the service worker is updated and the error page is displayed correctly (it even shows the one generated by the updated service worker if any updates were made). I guess the redirect intercepts the service worker and forces it to fallback to render the current page via the root entry point without actually forcing a navigation in the browser?

I triple-checked in case somehow I had reconfigured something else, but indeed the addition of the redirect on cached response is the difference between triggering the infinite loop and everything working across dynamic routes or any other invalid route in the app.

I have no explanation, but it works!

@userquin
Copy link
Member

You can try building your kit app using development in the mode in your local, workbox modules should show a few traces in the console

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants