Avoid race condition in SW version request

Previously the SW could receive the wrong auth token (by reading before
it was written by the main thread) and then loop attempting to
use it unsuccessfully, indefinitely.

Now, the SW reloads the auth token for each test, meaning that
although initial requests may fail, it will recover as soon as the UI
shares the required token.

This also remvoes the 'force update' SW logic, added in the distance
past to handle a mismatch between desktop versions. The desktop version
that requires this is before the minimum version in our runtime config,
so those users will already be prompted to upgrade, and have been for a
very long time.

Author: pimterry

src/services/server-api.ts

@@ -14,41 +14,41 @@ import { GraphQLApiClient } from './server-graphql-api';
 import { RestApiClient } from './server-rest-api';
 import { RequestDefinition, RequestOptions } from '../model/send/send-request-model';
 
-const authTokenPromise = !RUNNING_IN_WORKER
-    // Main UI gets given the auth token directly in its URL:
-    ? Promise.resolve(new URLSearchParams(window.location.search).get('authToken') ?? undefined)
-    // For workers, the new (March 2020) UI shares the auth token with SW via IDB:
-    : localForage.getItem<string>('latest-auth-token')
-        .then((authToken) => {
-            if (authToken) return authToken;
-
-            // Old UI (Jan-March 2020) shares auth token via SW query param:
-            const workerParams = new URLSearchParams(
-                (self as unknown as WorkerGlobalScope).location.search
-            );
-            return workerParams.get('authToken') ?? undefined;
-
-            // Pre-Jan 2020 UI doesn't share auth token - ok with old desktop, fails with 0.1.18+.
-        });
+async function getAuthToken() {
+    if (!RUNNING_IN_WORKER) {
+        return Promise.resolve(
+            new URLSearchParams(window.location.search).get('authToken') ?? undefined
+        );
+    }
+
+    // For workers, the UI shares the auth token with SW via IDB:
+    const authToken = await localForage.getItem<string>('latest-auth-token')
+    if (authToken) return authToken;
+}
 
 const serverReady = getDeferred();
 export const announceServerReady = () => serverReady.resolve();
 export const waitUntilServerReady = () => serverReady.promise;
 
-const apiClient: Promise<GraphQLApiClient | RestApiClient> = authTokenPromise.then(async (authToken) => {
-    // Delay checking, just to avoid spamming requests that we know won't work. Doesn't work
-    // in the SW, which doesn't get a server-ready ping, and is delayed quite a while anyway.
+const apiClient = (async (): Promise<GraphQLApiClient | RestApiClient> => {
+    // Delay checking, just to avoid spamming requests that we know won't work. Doesn't work in
+    // the update SW, which doesn't get a server-ready ping, so just wait a bit:
     if (!RUNNING_IN_WORKER) await waitUntilServerReady();
-
-    const restClient = new RestApiClient(authToken);
-    const graphQLClient = new GraphQLApiClient(authToken);
+    else await delay(5000);
 
     // To work out which API is supported, we loop trying to get the version from
     // each one (may take a couple of tries as the server starts up), and then
     // check the resulting version to see what's supported.
 
     let version: string | undefined;
     while (!version) {
+        // Reload auth token each time. For main UI it'll never change (but load is instant),
+        // while for SW there's a race so we need to reload just in case:
+        const authToken = await getAuthToken();
+
+        const restClient = new RestApiClient(authToken);
+        const graphQLClient = new GraphQLApiClient(authToken);
+
         version = await restClient.getServerVersion().catch(() => {
             console.log("Couldn't get version from REST API");
 
@@ -58,15 +58,20 @@ const apiClient: Promise<GraphQLApiClient | RestApiClient> = authTokenPromise.th
             });
         });
 
-        if (!version) await delay(100);
+        if (version) {
+            if (versionSatisfies(version, SERVER_REST_API_SUPPORTED)) {
+                return restClient;
+            } else {
+                return graphQLClient;
+            }
+        } else {
+            // Wait a little then try again:
+            await delay(100);
+        }
     }
 
-    if (versionSatisfies(version, SERVER_REST_API_SUPPORTED)) {
-        return restClient;
-    } else {
-        return graphQLClient;
-    }
-});
+    throw new Error(`Unreachable error: got version ${version} but couldn't pick an API client`);
+})();
 
 export async function getServerVersion(): Promise<string> {
     return (await apiClient).getServerVersion();

src/services/ui-update-worker.ts

@@ -1,7 +1,9 @@
 import { initSentry, logError } from '../errors';
 initSentry(process.env.SENTRY_DSN);
 
+// Used elsewhere in server API requests later to get the auth token:
 import * as localForage from 'localforage';
+localForage.config({ name: "httptoolkit", version: 1 });
 
 import { registerRoute, NavigationRoute } from 'workbox-routing';
 import { PrecacheController } from 'workbox-precaching'
@@ -11,38 +13,8 @@ import { StaleWhileRevalidate, NetworkOnly } from 'workbox-strategies';
 import packageMetadata from '../../package.json';
 import { getServerVersion } from './server-api';
 import { lastServerVersion, versionSatisfies } from './service-versions';
-import { delay } from '../util/promise';
 
 const appVersion = process.env.UI_VERSION || "Unknown";
-localForage.config({ name: "httptoolkit", version: 1 });
-
-// Check if the server is accessible, and that we've been given the relevant auth
-// details. If we haven't, that means the desktop has started the server with an
-// auth token, but we haven't received it. In general, that means the UI hasn't
-// passed it on, because it's outdated and doesn't understand it. To fix this,
-// we forcibly update the UI immediately. Should only ever happen once.
-
-type ServerStatus = 'accessible' | 'auth-required' | 'inaccessible'
-const serverStatus: Promise<ServerStatus> =
-    fetch("http://127.0.0.1:45457/", { method: 'POST' })
-    .then((response) => {
-        if (response.status === 403) return 'auth-required';
-        else return 'accessible';
-    })
-    .catch(() => 'inaccessible' as ServerStatus)
-    .then((status) => {
-        console.log('Service worker server status:', status);
-        return status;
-    });
-
-const forceUpdateRequired = serverStatus.then(async (status) => {
-    // Update should be forced if we're using a server that requires auth,
-    // the UI hasn't properly provided an auth token, and there is
-    // currently an active service worker (i.e. a cached UI)
-    return status === 'auth-required' &&
-        !(await localForage.getItem<string>('latest-auth-token')) &&
-        !!self.registration.active;
-});
 
 type PrecacheEntry = {
     url: string;
@@ -67,13 +39,6 @@ const precacheController = getPrecacheController();
 const precacheName = precacheController.strategy.cacheName;
 
 async function precacheNewVersionIfSupported(event: ExtendableEvent) {
-    if (await forceUpdateRequired) {
-        // Don't bother precaching: we want to take over & then force kill/refresh everything ASAP
-        self.skipWaiting();
-        logError("Force update required on newly installed SW");
-        return;
-    }
-
     await checkServerVersion();
 
     // Any required as the install return types haven't been updated for v4, so still use 'updatedEntries'
@@ -84,15 +49,6 @@ async function precacheNewVersionIfSupported(event: ExtendableEvent) {
 async function checkServerVersion() {
     const serverVersion = await getServerVersion().catch(async (e) => {
         console.log("Failed to get server version. Fallback back to last version anyway...");
-
-        console.log(
-            "Version unavailable but not forcing update, why?",
-            "status", await serverStatus,
-            "got token", !!(await localForage.getItem<string>('latest-auth-token')),
-            "SW registrations", self.registration,
-            "active SW", !!self.registration?.active
-        );
-
         logError(e);
 
         // This isn't perfect, but it's a pretty good approximation of when it's safe to update
@@ -102,7 +58,7 @@ async function checkServerVersion() {
 
         // This should never happen: the serverStatus checks should guarantee that we can
         // talk to the server in almost all cases, or that we have cached data. Fail & report it.
-        throw new Error("No server version available, even though server check passed");
+        throw new Error("No server version available in UI update worker");
     });
 
     console.log(`Connected httptoolkit-server version is ${serverVersion}.`);
@@ -150,40 +106,17 @@ self.addEventListener('install', (event: ExtendableEvent) => {
 });
 
 self.addEventListener('activate', async (event) => {
+    console.log('Update worker activating...');
     event.waitUntil((async () => {
         console.log(`SW activating for version ${appVersion}`);
 
-        if (await forceUpdateRequired) {
-            logError("Force update required on newly activated SW");
-
-            resettingSw = true; // Pass through all requests
-
-            // Take over and refresh all client pages:
-            await self.clients.claim();
-            const clients = await self.clients.matchAll({ type: 'window' });
-            clients.map((client) => {
-                console.log(`Refreshing ${client.url}`);
-                return (client as WindowClient).navigate(client.url)
-                    .catch(async (e) => {
-                        // On the first error, try once more, after a brief delay. Sometimes
-                        // claim() might not process fast enough, and this is necessary.
-                        await delay(100);
-                        return (client as WindowClient).navigate(client.url);
-                    });
-            });
-
-            // Unregister, so that future registration loads the SW & caches from scratch
-            self.registration.unregister();
-            console.log("SW forcibly refreshed");
-        } else {
-            // This can be removed only once we know that _nobody_ is using SWs from before 2019-05-09
-            deleteOldWorkboxCaches();
-
-            await precacheController.activate(event);
-
-            // Delete the old (now unused) SW event logs
-            indexedDB.deleteDatabase('keyval-store');
-        }
+        // This can be removed only once we know that _nobody_ is using SWs from before 2019-05-09
+        deleteOldWorkboxCaches();
+
+        await precacheController.activate(event);
+
+        // Delete the old (now unused) SW event logs
+        indexedDB.deleteDatabase('keyval-store');
     })());
 });