Simplify server pinging

frontend/sync-client/src/services/server-config.test.ts

@@ -0,0 +1,80 @@
+import assert from "node:assert/strict";
+import { test } from "node:test";
+import { ServerConfig } from "./server-config";
+import { SyncService } from "./sync-service";
+import { Settings } from "../persistence/settings";
+import { Logger } from "../tracing/logger";
+import { SUPPORTED_API_VERSION } from "../consts";
+
+const config = {
+    supportedApiVersion: SUPPORTED_API_VERSION,
+    isAuthenticated: true,
+    mergeableFileExtensions: ["md"],
+    serverVersion: "test"
+};
+const make = (fetch: typeof globalThis.fetch) =>
+    new ServerConfig(
+        new SyncService(
+            "device",
+            new Settings(
+                new Logger(),
+                { remoteUri: "http://test" },
+                async () => {}
+            ),
+            fetch,
+            { get: () => undefined, save: async () => {} }
+        )
+    );
+
+test("configuration shares one validated request and retries failures", async () => {
+    let calls = 0;
+    const server = make(async () => {
+        if (++calls === 1) throw new Error("offline");
+        return Response.json(config);
+    });
+    await assert.rejects(server.getConfig(), /offline/);
+    const [a, b] = await Promise.all([server.getConfig(), server.getConfig()]);
+    assert.equal(a, b);
+    assert.equal(calls, 2);
+    await server.getConfig();
+    assert.equal(calls, 2);
+});
+
+test("connection checks are independent and validate authentication and version", async () => {
+    let response = config;
+    const server = make(async () => Response.json(response));
+    const cached = await server.getConfig();
+    response = { ...config, isAuthenticated: false };
+    assert.equal((await server.checkConnection()).isSuccessful, false);
+    response = { ...config, supportedApiVersion: SUPPORTED_API_VERSION + 1 };
+    assert.equal((await server.checkConnection()).isSuccessful, false);
+    assert.equal(await server.getConfig(), cached);
+    server.reset();
+    await assert.rejects(server.getConfig(), /Unsupported API/);
+    response = config;
+    assert.deepEqual(await server.getConfig(), config);
+});
+
+for (const fail of [false, true]) {
+    test(`reset fences a late ${fail ? "failed" : "successful"} configuration request`, async () => {
+        const first = Promise.withResolvers<Response>();
+        let calls = 0;
+        const server = make(async () =>
+            ++calls === 1 ? first.promise : Response.json(config)
+        );
+        const old = server.getConfig();
+        server.reset();
+        const current = await server.getConfig();
+        if (fail) {
+            first.reject(new Error("old request"));
+            await assert.rejects(old, /old request/);
+        } else {
+            first.resolve(
+                Response.json({ ...config, mergeableFileExtensions: ["txt"] })
+            );
+            await old;
+        }
+        assert.equal(await server.getConfig(), current);
+        assert.equal(calls, 2);
+    });
+}

frontend/sync-client/src/services/server-config.ts

@@ -4,7 +4,6 @@ import {
     ServerVersionMismatchError
 } from "../errors/errors";
 import type { SyncService } from "./sync-service";
-import type { PingResponse } from "./types/PingResponse";
 
 export interface ServerConfigData {
     mergeableFileExtensions: string[];
@@ -13,18 +12,16 @@ export interface ServerConfigData {
 }
 
 export class ServerConfig {
-    private response: Promise<PingResponse> | undefined;
-    private config: ServerConfigData | undefined;
+    private config: Promise<ServerConfigData> | undefined;
 
-    public constructor(private readonly syncService: SyncService) {}
+    public constructor(private readonly syncService: SyncService) { }
 
     private static validateConfig(config: ServerConfigData): void {
         if (config.supportedApiVersion !== SUPPORTED_API_VERSION) {
             const shouldUpgradeClient =
                 config.supportedApiVersion > SUPPORTED_API_VERSION;
             throw new ServerVersionMismatchError(
-                `Unsupported API version: ${config.supportedApiVersion}. Consider upgrading the ${
-                    shouldUpgradeClient ? "client" : "sync-server"
+                `Unsupported API version: ${config.supportedApiVersion}. Consider upgrading the ${shouldUpgradeClient ? "client" : "sync-server"
                 } to ensure compatibility`
             );
         }
@@ -41,29 +38,18 @@ export class ServerConfig {
         await this.getConfig();
     }
 
-    public async checkConnection(forceUpdate = false): Promise<{
+    public async checkConnection(): Promise<{
         isSuccessful: boolean;
         message: string;
     }> {
         try {
-            let { response } = this;
-            if (!response || forceUpdate) {
-                response = this.response = this.syncService.ping();
-            }
+            const result = await this.syncService.ping();
 
-            const result: PingResponse = await response; // it must be defined, otherwise we would have thrown above
-            this.config = result;
-
-            if (result.isAuthenticated) {
-                return {
-                    isSuccessful: true,
-                    message: `Successfully connected to server (version: ${result.serverVersion}) and authenticated`
-                };
-            }
+            ServerConfig.validateConfig(result);
 
             return {
-                isSuccessful: false,
-                message: `Successfully connected to server (version: ${result.serverVersion}) but failed to authenticate`
+                isSuccessful: true,
+                message: `Successfully connected to server (version: ${result.serverVersion}) and authenticated`
             };
         } catch (e) {
             return {
@@ -75,22 +61,23 @@ export class ServerConfig {
 
     public async getConfig(): Promise<ServerConfigData> {
         if (!this.config) {
-            this.response ??= this.syncService.ping();
-            try {
-                this.config = await this.response;
-            } catch (error) {
-                this.response = undefined;
-                throw error;
-            }
+            const pending = this.syncService
+                .ping()
+                .then((config) => {
+                    ServerConfig.validateConfig(config);
+                    return config;
+                })
+                .catch((error: unknown) => {
+                    if (this.config === pending) this.config = undefined;
+                    throw error;
+                });
+
+            this.config = pending;
         }
-
-        ServerConfig.validateConfig(this.config);
-
         return this.config;
     }
 
     public reset(): void {
-        this.response = undefined;
         this.config = undefined;
     }
 }