diff --git a/client/src/App.test.tsx b/client/src/App.test.tsx index ad87b9451..566d1b4e2 100644 --- a/client/src/App.test.tsx +++ b/client/src/App.test.tsx @@ -580,24 +580,44 @@ describe('Version cache-busting', () => { expect(reload).not.toHaveBeenCalled() }) - it('FE-COMP-APP-027: the version marker is written even if the update path throws (#2228)', async () => { - // The marker used to be written after the purge and outside its try, so a - // throwing step left it unwritten and the whole destructive path repeated on - // every launch, for good. + it('FE-COMP-APP-027: an update path that throws is retried on the next launch, harmlessly (#2228)', async () => { + // A handover that never happened must not be recorded as done, or the + // device stays on the old build for good. The retry this buys has to be + // harmless: the purge that once repeated on every launch is gone, and a + // failure neither deletes anything nor reloads into the old worker's shell. localStorage.setItem('trek_app_version', '2.9.9') + const reload = vi.fn() + Object.defineProperty(window, 'location', { writable: true, value: { ...window.location, reload } }) + const cachesDelete = vi.fn().mockResolvedValue(true) + const unregister = vi.fn().mockResolvedValue(true) + vi.stubGlobal('caches', { keys: vi.fn().mockResolvedValue(['workbox-precache-v2', 'map-tiles']), delete: cachesDelete }) + const getRegistration = vi.fn().mockRejectedValue(new Error('no worker')) Object.defineProperty(navigator, 'serviceWorker', { configurable: true, - value: { getRegistration: vi.fn().mockRejectedValue(new Error('no worker')), addEventListener: vi.fn() }, + value: { getRegistration, getRegistrations: vi.fn().mockResolvedValue([{ unregister }]), addEventListener: vi.fn() }, }) server.use( http.get('/api/auth/app-config', () => - HttpResponse.json({ version: '2.9.10' }) + HttpResponse.json({ version: '2.9.10', places_provider: 'google' }) ) ) seedAuth() + const first = renderApp('/') + await waitFor(() => expect(getRegistration).toHaveBeenCalledTimes(1)) + await act(async () => { await Promise.resolve() }) + first.unmount() + expect(localStorage.getItem('trek_app_version')).toBe('2.9.9') + renderApp('/') - await waitFor(() => expect(localStorage.getItem('trek_app_version')).toBe('2.9.10')) + await waitFor(() => expect(getRegistration).toHaveBeenCalledTimes(2)) + await act(async () => { await Promise.resolve() }) + expect(localStorage.getItem('trek_app_version')).toBe('2.9.9') + expect(reload).not.toHaveBeenCalled() + expect(cachesDelete).not.toHaveBeenCalled() + expect(unregister).not.toHaveBeenCalled() + // Without a reload the session carries on, so the rest of the config applies. + expect(useAuthStore.getState().placesProvider).toBe('google') }) }) diff --git a/client/src/App.tsx b/client/src/App.tsx index 80bbf931f..0df993c00 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -17,6 +17,7 @@ import MobileShell from './mobile/MobileShell' import MRouteFallback from './mobile/components/MRouteFallback' import ErrorBoundary from './components/shared/ErrorBoundary' import { lazyWithRetry } from './utils/lazyWithRetry' +import { reconcileAppVersion } from './utils/versionHandover' import { useIsPhone } from './mobile/useIsPhone' import { TranslationProvider, useTranslation } from './i18n' import { authApi, isAuthPublicPath } from './api/client' @@ -309,6 +310,7 @@ export default function App() { if (config?.version) setAppVersion(config.version) if (config?.has_maps_key !== undefined) setHasMapsKey(config.has_maps_key) if (config?.has_amap_key !== undefined) setHasAmapKey(config.has_amap_key) + if (config?.places_provider) setPlacesProvider(config.places_provider) if (config?.timezone) setServerTimezone(config.timezone) if (config?.require_mfa !== undefined) setAppRequireMfa(!!config.require_mfa) if (config?.trip_reminders_enabled !== undefined) setTripRemindersEnabled(config.trip_reminders_enabled) @@ -318,50 +320,8 @@ export default function App() { if (config?.places_enrich_enabled !== undefined) setPlacesEnrichEnabled(config.places_enrich_enabled) if (config?.place_shadow_enabled !== undefined) setPlaceShadowEnabled(config.place_shadow_enabled) if (config?.permissions) usePermissionsStore.getState().setPermissions(config.permissions) - - // A version is a short release tag and nothing else. It arrives over the - // wire and is written to this device's storage, so only a value made of - // the characters a tag may contain is taken, as the match itself. Anything - // else is ignored, which also keeps a malformed value from being compared - // against the stored marker and starting an update on every launch. - const releaseTag = /^[\w.+-]{1,64}$/.exec(typeof config?.version === 'string' ? config.version : '') - if (releaseTag) { - const version = releaseTag[0] - const storedVersion = localStorage.getItem('trek_app_version') - // Record the version BEFORE acting on it. The old code wrote the marker - // after the purge and outside its try, so a throwing setItem (private - // mode, blocked site data, quota) left the caches deleted, the marker - // unwritten and the reload unreached, and the purge then repeated on - // every single launch, permanently (#2228). - try { localStorage.setItem('trek_app_version', version) } catch { /* site data blocked */ } - if (storedVersion && storedVersion !== version) { - // A newer build is deployed. Ask the service worker to fetch it and - // hand over; Workbox ('autoUpdate' + skipWaiting + clientsClaim) - // installs the new precache and only then drops the outdated one, so - // there is never a moment without an app shell. - // - // This used to delete EVERY Cache Storage bucket and unregister EVERY - // worker instead. That left the device with no shell and no worker - // until a fresh ~22 MB precache finished (minutes on mobile data), - // and anyone who closed the app or lost signal in that window was - // left with a PWA that could no longer start offline at all. It also - // threw away the map tiles and file blobs the user had deliberately - // downloaded for offline use (#2228). - try { - const reg = 'serviceWorker' in navigator ? await navigator.serviceWorker.getRegistration() : undefined - if (reg) { - navigator.serviceWorker.addEventListener('controllerchange', () => window.location.reload(), { once: true }) - await reg.update() - return - } - } catch { /* fall through to a plain reload */ } - window.location.reload() - return - } - } - // Last, so the version block above keeps its lines: the taint analyser - // re-raises its browser-storage finding for every line that moves under it. - if (config?.places_provider) setPlacesProvider(config.places_provider) + // Last, since a new release reloads the page from here. + await reconcileAppVersion(config?.version) }).catch(() => {}) }, []) diff --git a/client/src/App.versionHandover.test.tsx b/client/src/App.versionHandover.test.tsx new file mode 100644 index 000000000..473a24cf7 --- /dev/null +++ b/client/src/App.versionHandover.test.tsx @@ -0,0 +1,281 @@ +import React from 'react' +import { render, waitFor, act } from '@testing-library/react' +import { MemoryRouter } from 'react-router' +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { http, HttpResponse } from 'msw' +import { server } from '../tests/helpers/msw/server' +import { useAuthStore } from './store/authStore' +import { resetAllStores } from '../tests/helpers/store' +import App from './App' + +/* + * #2458: an installed PWA kept serving the previous UI after a server upgrade. + * + * The launch probe compares the server's version with a marker in localStorage + * and, on a mismatch, asks the service worker to fetch the new build and reloads + * once it has taken over. These tests drive that probe across consecutive + * launches against a fake service-worker container, so a handover that never + * completed can be told apart from one that did. + * + * The fake is installed before App is imported (vi.hoisted), so code that + * notes the controlling worker when its module loads sees the worker that + * served this page, exactly as in the browser. + */ +const sw = vi.hoisted(() => { + class FakeWorker extends EventTarget { + state: string + readonly scriptURL = '/sw.js' + constructor(state: string) { + super() + this.state = state + } + become(state: string) { + this.state = state + this.dispatchEvent(new Event('statechange')) + } + } + class FakeRegistration extends EventTarget { + active: FakeWorker | null = null + installing: FakeWorker | null = null + waiting: FakeWorker | null = null + readonly scope = '/' + update = vi.fn() + } + class FakeContainer extends EventTarget { + controller: FakeWorker | null = null + getRegistration = vi.fn() + // Listeners a page adds die with that page. Those added while modules load + // (before `trackPage` is switched on) stand for module-level code and stay. + trackPage = false + private pageListeners: Array<[string, EventListenerOrEventListenerObject, boolean | AddEventListenerOptions | undefined]> = [] + addEventListener(type: string, listener: EventListenerOrEventListenerObject | null, options?: boolean | AddEventListenerOptions) { + super.addEventListener(type, listener, options) + if (this.trackPage && listener) this.pageListeners.push([type, listener, options]) + } + dropPageListeners() { + for (const [type, listener, options] of this.pageListeners) super.removeEventListener(type, listener, options) + this.pageListeners = [] + } + } + const oldWorker = new FakeWorker('activated') + const registration = new FakeRegistration() + const container = new FakeContainer() + registration.active = oldWorker + container.controller = oldWorker + container.getRegistration.mockImplementation(async () => registration) + Object.defineProperty(navigator, 'serviceWorker', { configurable: true, value: container }) + return { FakeWorker, registration, container, oldWorker } +}) + +vi.mock('./pages/LoginPage', () => ({ default: () =>
Login
})) +vi.mock('./pages/DashboardPage', () => ({ default: () =>
Dashboard
})) +vi.mock('./hooks/useInAppNotificationListener.ts', () => ({ useInAppNotificationListener: vi.fn() })) + +// Every import above has been evaluated by now; from here on a listener belongs to a page. +sw.container.trackPage = true + +const reload = vi.fn() +let configHits = 0 + +function serveVersion(version: string, gate?: Promise) { + server.use( + http.get('/api/auth/app-config', async () => { + configHits++ + if (gate) await gate + return HttpResponse.json({ version }) + }), + ) +} + +/** The browser's handover: the new worker installs, activates and claims the page. */ +function handOver() { + const next = new sw.FakeWorker('activated') + sw.registration.installing = null + sw.registration.active = next + sw.container.controller = next + sw.container.dispatchEvent(new Event('controllerchange')) +} + +/** update() finds a new sw.js and starts installing it; `then` decides how the install ends. */ +function updateStartsInstall(then: 'activates' | 'fails') { + sw.registration.update.mockImplementation(async () => { + const next = new sw.FakeWorker('installing') + sw.registration.installing = next + setTimeout(() => { + if (then === 'activates') { + next.become('activated') + handOver() + } else { + // Workbox precaching is all-or-nothing: one failed fetch out of ~463 + // rejects the install event and the new worker is thrown away. + sw.registration.installing = null + next.become('redundant') + } + }, 0) + return sw.registration + }) +} + +/** One cold start of the installed app: fresh page, fresh session, same localStorage. */ +async function launch() { + sessionStorage.clear() + const hitsBefore = configHits + const view = render( + + + , + ) + await waitFor(() => expect(configHits).toBe(hitsBefore + 1)) + // Let the probe, update() and the fake install run to completion. + await act(async () => { await new Promise(r => setTimeout(r, 50)) }) + view.unmount() + sw.container.dropPageListeners() +} + +beforeEach(() => { + resetAllStores() + vi.clearAllMocks() + configHits = 0 + sw.registration.active = sw.oldWorker + sw.registration.installing = null + sw.registration.waiting = null + sw.registration.update.mockReset() + sw.registration.update.mockResolvedValue(sw.registration) + sw.container.controller = sw.oldWorker + sw.container.dropPageListeners() + sw.container.getRegistration.mockReset() + sw.container.getRegistration.mockImplementation(async () => sw.registration) + reload.mockReset() + Object.defineProperty(window, 'location', { writable: true, value: { ...window.location, reload } }) + server.use( + http.get('/api/health/features', () => HttpResponse.json({ bookingImport: false, aiParsing: false })), + http.get('/api/system-notices/active', () => HttpResponse.json([])), + ) + useAuthStore.setState({ + isLoading: false, + isAuthenticated: false, + user: null, + appRequireMfa: false, + loadUser: vi.fn().mockResolvedValue(undefined), + }) +}) + +describe('#2458 version handover: behaviour that must survive the fix', () => { + it('FE-COMP-APP-2458-P1: a confirmed handover reloads once and is not repeated on the next launch', async () => { + localStorage.setItem('trek_app_version', '4.3.0') + serveVersion('4.3.1') + updateStartsInstall('activates') + + await launch() + expect(sw.registration.update).toHaveBeenCalledTimes(1) + expect(reload).toHaveBeenCalledTimes(1) + expect(localStorage.getItem('trek_app_version')).toBe('4.3.1') + + await launch() + expect(sw.registration.update).toHaveBeenCalledTimes(1) + expect(reload).toHaveBeenCalledTimes(1) + }) + + it('FE-COMP-APP-2458-P2: nothing to hand over (sw.js unchanged, same worker in control) neither reloads nor retries', async () => { + // A release that changed nothing the precache holds leaves sw.js byte-identical, + // so update() installs nothing. The bundle this page runs is already current. + localStorage.setItem('trek_app_version', '4.3.0') + serveVersion('4.3.1') + + await launch() + expect(sw.registration.update).toHaveBeenCalledTimes(1) + expect(reload).not.toHaveBeenCalled() + + await launch() + expect(sw.registration.update).toHaveBeenCalledTimes(1) + expect(reload).not.toHaveBeenCalled() + }) + + it.each([ + ['a self-built image without the build arg', 'dev'], + ['a test instance pinned to an older tag', '4.0.0'], + ['a prerelease image, whose bundle keeps the last stable version', '4.4.0-pre.3'], + ])('FE-COMP-APP-2458-P3: %s (server says %s, bundle is built as another version) never loops', async (_label, serverVersion) => { + // The server reports APP_VERSION, the bundle carries client/package.json. On + // these setups the two never agree, so a check against the bundle's own + // version would see an update on every launch. + serveVersion(serverVersion) + + await launch() + await launch() + await launch() + expect(reload).not.toHaveBeenCalled() + expect(sw.registration.update).not.toHaveBeenCalled() + }) +}) + +describe('#2458 version handover: the reported defect', () => { + it('FE-COMP-APP-2458-01: an update() that rejects is retried on the next launch', async () => { + localStorage.setItem('trek_app_version', '4.3.0') + serveVersion('4.3.1') + sw.registration.update.mockRejectedValue(new TypeError('Failed to update a ServiceWorker: network error')) + + await launch() + expect(sw.registration.update).toHaveBeenCalledTimes(1) + + // Next cold start, network fine this time. + updateStartsInstall('activates') + await launch() + expect(sw.registration.update).toHaveBeenCalledTimes(2) + expect(reload).toHaveBeenCalled() + }) + + it('FE-COMP-APP-2458-02: a failed update() does not reload into the shell the same worker serves again', async () => { + // The old worker still controls the page, so a plain reload is answered from + // its precache: the same old UI, now with the marker saying it is current. + localStorage.setItem('trek_app_version', '4.3.0') + serveVersion('4.3.1') + sw.registration.update.mockRejectedValue(new TypeError('Failed to update a ServiceWorker: network error')) + + await launch() + expect(reload).not.toHaveBeenCalled() + }) + + it('FE-COMP-APP-2458-03: an install that goes redundant is retried on the next launch', async () => { + localStorage.setItem('trek_app_version', '4.3.0') + serveVersion('4.3.1') + updateStartsInstall('fails') + + await launch() + expect(sw.registration.update).toHaveBeenCalledTimes(1) + expect(reload).not.toHaveBeenCalled() + + updateStartsInstall('activates') + await launch() + expect(sw.registration.update).toHaveBeenCalledTimes(2) + expect(reload).toHaveBeenCalledTimes(1) + }) + + // Kept last: it swaps the controlling worker mid-launch, which code that + // remembers "the controller changed since this page loaded" keeps seeing. + it('FE-COMP-APP-2458-04: a worker that took over before the probe answered still reloads the page', async () => { + // The browser checks sw.js on every navigation by itself. When that check + // installs and activates the new worker before /api/auth/app-config has + // answered, the takeover happens while nobody listens, and update() then + // finds nothing newer. The page keeps running the bundle the old worker served. + localStorage.setItem('trek_app_version', '4.3.0') + let open!: () => void + const gate = new Promise(r => { open = r }) + serveVersion('4.3.1', gate) + + sessionStorage.clear() + const view = render( + + + , + ) + await waitFor(() => expect(configHits).toBe(1)) + act(() => handOver()) + open() + await act(async () => { await new Promise(r => setTimeout(r, 50)) }) + view.unmount() + sw.container.dropPageListeners() + + expect(reload).toHaveBeenCalledTimes(1) + }) +}) diff --git a/client/src/utils/versionHandover.test.ts b/client/src/utils/versionHandover.test.ts new file mode 100644 index 000000000..318614cc5 --- /dev/null +++ b/client/src/utils/versionHandover.test.ts @@ -0,0 +1,328 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +/* + * reconcileAppVersion notes the worker that served the page when its module + * loads, so every test installs its fake service-worker container first and + * then loads a fresh copy of the module. + */ + +class FakeWorker extends EventTarget { + state: string + constructor(state: string) { + super() + this.state = state + } + become(state: string) { + this.state = state + this.dispatchEvent(new Event('statechange')) + } +} + +class FakeRegistration { + installing: FakeWorker | null = null + waiting: FakeWorker | null = null + update = vi.fn(async () => this) +} + +class FakeContainer extends EventTarget { + controller: FakeWorker | null + getRegistration = vi.fn() + constructor(controller: FakeWorker | null) { + super() + this.controller = controller + } + /** The browser activates a new worker and hands it this page. */ + takeOver() { + this.controller = new FakeWorker('activated') + this.dispatchEvent(new Event('controllerchange')) + } +} + +function installWorker(controller: FakeWorker | null = new FakeWorker('activated')) { + const registration = new FakeRegistration() + const container = new FakeContainer(controller) + container.getRegistration.mockResolvedValue(registration) + Object.defineProperty(navigator, 'serviceWorker', { configurable: true, value: container }) + return { container, registration } +} + +/** update() finds a new sw.js and leaves the new worker installing. */ +function updateInstalls(registration: FakeRegistration) { + const incoming = new FakeWorker('installing') + registration.update.mockImplementation(async () => { + registration.installing = incoming + return registration + }) + return incoming +} + +async function loadModule() { + vi.resetModules() + return import('./versionHandover') +} + +/** Lets one kind of storage throw while the other keeps working. */ +function breakStorage(method: 'getItem' | 'setItem', broken: () => Storage) { + const original = Storage.prototype[method] as (...args: string[]) => unknown + vi.spyOn(Storage.prototype, method).mockImplementation(function (this: Storage, ...args: string[]) { + if (this === broken()) throw new DOMException('The operation is insecure.', 'SecurityError') + return original.apply(this, args) + } as never) +} + +const marker = () => localStorage.getItem('trek_app_version') +let reload: ReturnType + +beforeEach(() => { + reload = vi.fn() + Object.defineProperty(window, 'location', { writable: true, value: { ...window.location, reload } }) +}) + +afterEach(() => { + vi.restoreAllMocks() + delete (navigator as { serviceWorker?: unknown }).serviceWorker +}) + +describe('reconcileAppVersion: when nothing needs to happen', () => { + it.each([ + ['nothing at all', undefined], + ['a number', 431], + ['an empty string', ''], + ['markup', ''], + ['an overlong value', '4'.repeat(65)], + ])('FE-UTIL-VERSION-001: ignores %s as the reported version', async (_label, reported) => { + const { container } = installWorker() + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + + await reconcileAppVersion(reported) + + expect(marker()).toBe('4.3.0') + expect(container.getRegistration).not.toHaveBeenCalled() + expect(reload).not.toHaveBeenCalled() + }) + + it('FE-UTIL-VERSION-002: the first launch on a device only records the version', async () => { + const { container } = installWorker() + const { reconcileAppVersion } = await loadModule() + + await reconcileAppVersion('4.3.1') + + expect(marker()).toBe('4.3.1') + expect(container.getRegistration).not.toHaveBeenCalled() + expect(reload).not.toHaveBeenCalled() + }) + + it('FE-UTIL-VERSION-003: the version this device already runs changes nothing', async () => { + const { container } = installWorker() + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.1') + + await reconcileAppVersion('4.3.1') + + expect(container.getRegistration).not.toHaveBeenCalled() + expect(reload).not.toHaveBeenCalled() + }) + + it('FE-UTIL-VERSION-004: unreadable storage leaves the worker alone', async () => { + const { container } = installWorker() + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + breakStorage('getItem', () => localStorage) + + await expect(reconcileAppVersion('4.3.1')).resolves.toBeUndefined() + + expect(container.getRegistration).not.toHaveBeenCalled() + expect(reload).not.toHaveBeenCalled() + }) + + it('FE-UTIL-VERSION-005: a browser that throws on the service worker itself still loads the app', async () => { + Object.defineProperty(navigator, 'serviceWorker', { + configurable: true, + get() { throw new DOMException('The operation is insecure.', 'SecurityError') }, + }) + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + + await reconcileAppVersion('4.3.1') + + // Nothing could be asked, so the next launch asks again. + expect(marker()).toBe('4.3.0') + expect(reload).not.toHaveBeenCalled() + }) +}) + +describe('reconcileAppVersion: a new version without a handover to wait for', () => { + it('FE-UTIL-VERSION-010: without service worker support it records the version and reloads once', async () => { + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + + await reconcileAppVersion('4.3.1') + + expect(marker()).toBe('4.3.1') + expect(reload).toHaveBeenCalledTimes(1) + }) + + it('FE-UTIL-VERSION-011: without a registration it records the version and reloads once', async () => { + const { container } = installWorker(null) + container.getRegistration.mockResolvedValue(undefined) + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + + await reconcileAppVersion('4.3.1') + + expect(marker()).toBe('4.3.1') + expect(reload).toHaveBeenCalledTimes(1) + }) + + it('FE-UTIL-VERSION-012: a worker the browser swapped in since the page loaded gets one reload and no update', async () => { + const { container, registration } = installWorker() + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + // The browser's own check on navigation got there before the server answered. + container.takeOver() + + await reconcileAppVersion('4.3.1') + + expect(marker()).toBe('4.3.1') + expect(reload).toHaveBeenCalledTimes(1) + expect(registration.update).not.toHaveBeenCalled() + }) + + it('FE-UTIL-VERSION-013: a worker claiming a page it did not serve is no takeover', async () => { + const { container, registration } = installWorker(null) + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + container.controller = new FakeWorker('activated') + + await reconcileAppVersion('4.3.1') + + expect(registration.update).toHaveBeenCalledTimes(1) + expect(reload).not.toHaveBeenCalled() + }) + + it('FE-UTIL-VERSION-014: an unchanged sw.js leaves nothing to hand over', async () => { + const { container, registration } = installWorker() + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + + await reconcileAppVersion('4.3.1') + + expect(registration.update).toHaveBeenCalledTimes(1) + expect(marker()).toBe('4.3.1') + expect(reload).not.toHaveBeenCalled() + + // A takeover later in the session is not this launch's business. + container.takeOver() + expect(reload).not.toHaveBeenCalled() + }) +}) + +describe('reconcileAppVersion: handing over to the new build', () => { + it('FE-UTIL-VERSION-020: reloads once the new worker has taken over, and only then records the version', async () => { + const { container, registration } = installWorker() + const incoming = updateInstalls(registration) + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + + await reconcileAppVersion('4.3.1') + expect(marker()).toBe('4.3.0') + expect(reload).not.toHaveBeenCalled() + + incoming.become('activated') + container.takeOver() + expect(marker()).toBe('4.3.1') + expect(reload).toHaveBeenCalledTimes(1) + }) + + it('FE-UTIL-VERSION-021: a worker already installed and waiting counts as the incoming one', async () => { + const { container, registration } = installWorker() + registration.waiting = new FakeWorker('installed') + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + + await reconcileAppVersion('4.3.1') + expect(marker()).toBe('4.3.0') + + container.takeOver() + expect(reload).toHaveBeenCalledTimes(1) + }) + + it('FE-UTIL-VERSION-022: an update that fails is left for the next launch, without a reload', async () => { + const { container, registration } = installWorker() + registration.update.mockRejectedValue(new TypeError('Failed to update a ServiceWorker: network error')) + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + + await reconcileAppVersion('4.3.1') + + expect(marker()).toBe('4.3.0') + expect(reload).not.toHaveBeenCalled() + container.takeOver() + expect(reload).not.toHaveBeenCalled() + }) + + it('FE-UTIL-VERSION-023: an install that goes redundant is left for the next launch', async () => { + const { container, registration } = installWorker() + const incoming = updateInstalls(registration) + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + + await reconcileAppVersion('4.3.1') + incoming.become('redundant') + + expect(marker()).toBe('4.3.0') + container.takeOver() + expect(reload).not.toHaveBeenCalled() + }) + + it('FE-UTIL-VERSION-024: a registration lookup that throws is left for the next launch', async () => { + const { container } = installWorker() + container.getRegistration.mockRejectedValue(new Error('no worker')) + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + + await reconcileAppVersion('4.3.1') + + expect(marker()).toBe('4.3.0') + expect(reload).not.toHaveBeenCalled() + }) +}) + +describe('reconcileAppVersion: the reload guard', () => { + it('FE-UTIL-VERSION-030: a marker that cannot be written does not turn into a reload loop', async () => { + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + breakStorage('setItem', () => localStorage) + + // Every load of this session sees the new version again. + await reconcileAppVersion('4.3.1') + await reconcileAppVersion('4.3.1') + await reconcileAppVersion('4.3.1') + + expect(marker()).toBe('4.3.0') + expect(reload).toHaveBeenCalledTimes(1) + }) + + it('FE-UTIL-VERSION-031: the next release in the same session still gets its reload', async () => { + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + + await reconcileAppVersion('4.3.1') + await reconcileAppVersion('4.3.2') + + expect(marker()).toBe('4.3.2') + expect(reload).toHaveBeenCalledTimes(2) + }) + + it('FE-UTIL-VERSION-032: without session storage there is no guard, so there is no reload either', async () => { + const { reconcileAppVersion } = await loadModule() + localStorage.setItem('trek_app_version', '4.3.0') + breakStorage('getItem', () => sessionStorage) + + await reconcileAppVersion('4.3.1') + + expect(marker()).toBe('4.3.1') + expect(reload).not.toHaveBeenCalled() + }) +}) diff --git a/client/src/utils/versionHandover.ts b/client/src/utils/versionHandover.ts new file mode 100644 index 000000000..71e06ed12 --- /dev/null +++ b/client/src/utils/versionHandover.ts @@ -0,0 +1,142 @@ +/** + * Bringing an installed app onto the build the server runs now. + * + * Every launch asks the server for its version and compares it with a marker + * this device keeps. A different version means a release was deployed while + * this page may still run the previous bundle, answered from the service + * worker's precache. The worker is asked to fetch the new build; Workbox + * ('autoUpdate' with skipWaiting and clientsClaim) installs the new precache + * and only then drops the old one, and the page reloads once the new worker + * has taken over. + * + * The marker only moves once that handover is done or turned out to be + * unnecessary. A handover that failed (sw.js unreachable, a precache install + * that broke off on a flaky connection) is simply tried again on the next + * launch. That retry cannot loop: a failure never reloads, since the same old + * worker would answer the reload, and every reload here is limited to one per + * version and session. + */ + +const VERSION_KEY = 'trek_app_version' +const RELOAD_KEY = 'trek_app_version_reload' + +/** The worker controlling this page, or null. Reading it can throw where site data is blocked. */ +function currentController(): ServiceWorker | null { + try { + return 'serviceWorker' in navigator ? navigator.serviceWorker.controller : null + } catch { + return null + } +} + +// The worker that served this page, noted when the app loads. The browser checks +// sw.js on every navigation by itself, and when that check installs the new build +// before the server has told us its version, the takeover is over before anyone +// listens for it. A controller other than this one means the new shell is only a +// reload away. A page served without a worker came from the network, so a worker +// claiming it later is no takeover. +const servedBy = currentController() + +function markApplied(version: string): void { + try { localStorage.setItem(VERSION_KEY, version) } catch { /* site data blocked */ } +} + +/** + * One reload per server version and session. When the marker cannot be written, + * every load sees the new version again, and a page without a worker would + * otherwise reload forever. Without session storage there is no such guard, so + * the page stays as it is rather than risking the loop. + */ +function reloadOnce(version: string): void { + try { + if (sessionStorage.getItem(RELOAD_KEY) === version) return + sessionStorage.setItem(RELOAD_KEY, version) + } catch { + return + } + window.location.reload() +} + +async function handOver(reg: ServiceWorkerRegistration, version: string): Promise { + // Listening before asking for the update, so a quick takeover is not missed. + // Giving up disarms the listener: the reload belongs to this launch, and one + // arriving later in the session would hit the user in the middle of an edit. + let armed = true + navigator.serviceWorker.addEventListener('controllerchange', () => { + if (!armed) return + markApplied(version) + reloadOnce(version) + }, { once: true }) + + try { + await reg.update() + } catch { + armed = false + return + } + + const incoming = reg.installing ?? reg.waiting + if (!incoming) { + // sw.js is unchanged, so the release changed nothing the precache holds and + // the bundle this page runs is the current one. + armed = false + if (currentController() === servedBy) markApplied(version) + return + } + // Precaching is all or nothing: one failed request out of several hundred and + // the new worker is thrown away while the old one stays in charge. The marker + // has not moved, so the next launch comes back here. + incoming.addEventListener('statechange', () => { + if (incoming.state === 'redundant') armed = false + }) +} + +/** + * Compares the version the server reports with the one this device last ran + * and, when they differ, moves the app onto the new build. It never deletes a + * cache or unregisters a worker. That used to leave the device without an app + * shell until a fresh ~22 MB precache finished, so anyone who closed the app or + * lost signal in that window could no longer start it offline, and it threw + * away the map tiles and files the user had downloaded on purpose. + */ +export async function reconcileAppVersion(reported: unknown): Promise { + // A version is a short release tag and nothing else. It arrives over the + // wire and is written to this device's storage, so only a value made of + // the characters a tag may contain is taken, as the match itself. Anything + // else is ignored, which also keeps a malformed value from being compared + // against the stored marker and starting an update on every launch. + const releaseTag = /^[\w.+-]{1,64}$/.exec(typeof reported === 'string' ? reported : '') + if (!releaseTag) return + const version = releaseTag[0] + + let storedVersion: string | null + try { + storedVersion = localStorage.getItem(VERSION_KEY) + } catch { + return + } + if (!storedVersion) { + // The first launch on this device runs whatever the server just served. + markApplied(version) + return + } + if (storedVersion === version) return + + let reg: ServiceWorkerRegistration | undefined + try { + reg = 'serviceWorker' in navigator ? await navigator.serviceWorker.getRegistration() : undefined + } catch { + // Left for the next launch. A reload now would only reach the old worker again. + return + } + // Without a worker the page came from the network and runs the new bundle + // already, so the reload only confirms it. A controller other than the one + // that served the page means the browser finished the handover by itself, + // and the reload is what shows the new build. + if (!reg || (servedBy && currentController() !== servedBy)) { + markApplied(version) + reloadOnce(version) + return + } + await handOver(reg, version) +}