Retry FW Lite update check after a failed fetch

.claude/launch.json

@@ -6,6 +6,18 @@
       "runtimeExecutable": "task",
       "runtimeArgs": ["fw-lite-web"],
       "port": 5137
+    },
+    {
+      "name": "viewer-dev",
+      "runtimeExecutable": "pnpm",
+      "runtimeArgs": ["-C", "frontend/viewer", "run", "dev"],
+      "port": 5173
+    },
+    {
+      "name": "fw-lite-web-chaos",
+      "runtimeExecutable": "powershell",
+      "runtimeArgs": ["-NoProfile", "-Command", "$env:FW_LITE_CHAOS='1.0'; dotnet run --project backend/FwLite/FwLiteWeb -- --FwLite:UpdateCheckCondition=Always"],
+      "port": 5137
     }
   ]
 }

backend/FwLite/FwLiteMaui/Services/ConnectivitySyncTrigger.cs

@@ -1,13 +1,16 @@
+using FwLiteShared.AppUpdate;
 using FwLiteShared.Projects;
 using Microsoft.Extensions.Hosting;
 using Microsoft.Extensions.Logging;
 
 namespace FwLiteMaui.Services;
 
-// Primary use case: app started offline should start syncing if the device comes online
+// Primary use case: app started offline should start syncing (and finish its startup network work, like
+// the update check) if the device comes online
 public sealed class ConnectivitySyncTrigger(
     IConnectivity connectivity,
     LexboxProjectChangeListener lexboxProjectChangeListener,
+    UpdateChecker updateChecker,
     ILogger<ConnectivitySyncTrigger> logger) : IHostedService
 {
     private NetworkAccess _lastAccess;
@@ -38,6 +41,21 @@ private void OnConnectivityChanged(object? sender, ConnectivityChangedEventArgs
 
         logger.LogInformation("Connectivity regained (internet access); ensuring push listeners");
         _ = EnsureListeners();
+        _ = RetryUpdateCheck();
+    }
+
+    //the startup check is a no-op when it fails before reaching the server (no throttle record), so
+    //this retries it once the network is actually usable. TryUpdate itself honors the interval gate.
+    private async Task RetryUpdateCheck()
+    {
+        try
+        {
+            await updateChecker.TryUpdate();
+        }
+        catch (Exception e)
+        {
+            logger.LogWarning(e, "Failed to check for updates after connectivity change");
+        }
     }
 
     private async Task EnsureListeners(CancellationToken cancellationToken = default)

backend/FwLite/FwLiteShared.Tests/AppUpdate/UpdateCheckerTests.cs

@@ -1,6 +1,11 @@
+using System.Net;
+using System.Net.Http.Json;
+using System.Net.Sockets;
+using System.Text;
 using FwLiteShared;
 using FwLiteShared.AppUpdate;
 using FwLiteShared.Events;
+using LexCore.Entities;
 using Microsoft.Extensions.Caching.Memory;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
@@ -18,10 +23,13 @@ public class UpdateCheckerTests
     // UpdateCheckThrottleTests).
     private readonly InMemoryPreferencesService _preferences = new();
 
+    private UpdateCheckThrottle _throttle = null!;
+
     private UpdateChecker CreateUpdateChecker(FwLiteConfig? config = null)
     {
         var options = Options.Create(config ?? new FwLiteConfig());
-        var throttle = new UpdateCheckThrottle(_preferences, options, Mock.Of<ILogger<UpdateCheckThrottle>>());
+        _throttle = new UpdateCheckThrottle(_preferences, options, Mock.Of<ILogger<UpdateCheckThrottle>>());
+        var throttle = _throttle;
         return new UpdateChecker(
             _httpClientFactoryMock.Object,
             Mock.Of<ILogger<UpdateChecker>>(),
@@ -78,4 +86,124 @@ public void ShouldCheckReleaseFeed_WhenConfigSetToNever_ReturnsFalse()
 
         checker.ShouldCheckReleaseFeed().Should().BeFalse();
     }
+
+    /// <summary>Routes the Lexbox client through <paramref name="send"/> and counts the requests.</summary>
+    private StubHandler UseHttpHandler(Func<HttpResponseMessage> send) =>
+        UseAsyncHttpHandler(() => Task.FromResult(send()));
+
+    private StubHandler UseAsyncHttpHandler(Func<Task<HttpResponseMessage>> send)
+    {
+        var handler = new StubHandler(send);
+        _httpClientFactoryMock.Setup(f => f.CreateClient(UpdateChecker.HttpClientName))
+            .Returns(() => new HttpClient(handler, false));
+        return handler;
+    }
+
+    private class StubHandler(Func<Task<HttpResponseMessage>> send) : HttpMessageHandler
+    {
+        private int _requests;
+        public int Requests => _requests;
+
+        protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
+        {
+            Interlocked.Increment(ref _requests);
+            return send();
+        }
+    }
+
+    private static HttpResponseMessage NoUpdateResponse() => new(HttpStatusCode.OK)
+    {
+        Content = JsonContent.Create(new ShouldUpdateResponse(null))
+    };
+
+    private static HttpRequestException DnsFailure() =>
+        new("No such host is known. (lexbox.org:443)", new SocketException((int)SocketError.HostNotFound));
+
+    [Fact]
+    public async Task CheckForUpdate_WhenRequestNeverReachesServer_DoesNotRecordCheckAndRetries()
+    {
+        //the scenario from the field: app launched while a VPN was still connecting, so DNS was dead.
+        //That must not count as a check, otherwise the next retry is UpdateCheckInterval (8h) away.
+        var handler = UseHttpHandler(() => throw DnsFailure());
+        var checker = CreateUpdateChecker(new FwLiteConfig { Os = FwLitePlatform.Windows });
+
+        (await checker.CheckForUpdate()).Should().BeNull();
+
+        _throttle.LastUpdateCheck.Should().Be(DateTime.MinValue);
+        _throttle.ShouldCheckForUpdate().Should().BeTrue();
+
+        //a second call (e.g. connectivity regained a minute later) must hit the server again rather than
+        //the manual-check cache
+        await checker.CheckForUpdate();
+        handler.Requests.Should().Be(2);
+    }
+
+    [Fact]
+    public async Task CheckForUpdate_WhenServerResponds_RecordsCheckAndCachesResult()
+    {
+        var handler = UseHttpHandler(() => new HttpResponseMessage(HttpStatusCode.OK)
+        {
+            Content = JsonContent.Create(new ShouldUpdateResponse(null))
+        });
+        var checker = CreateUpdateChecker(new FwLiteConfig { Os = FwLitePlatform.Windows });
+
+        (await checker.CheckForUpdate()).Should().BeNull();
+
+        _throttle.LastUpdateCheck.Should().BeCloseTo(DateTime.UtcNow, TimeSpan.FromMinutes(1));
+        _throttle.ShouldCheckForUpdate().Should().BeFalse();
+
+        await checker.CheckForUpdate();
+        handler.Requests.Should().Be(1, "a recent answer is served from the manual-check cache");
+    }
+
+    [Fact]
+    public async Task CheckForUpdate_WhenResponseIsMalformed_StillRecordsCheck()
+    {
+        //the server answered; a body we can't parse is not fixed by asking again on every launch
+        UseHttpHandler(() => new HttpResponseMessage(HttpStatusCode.OK)
+        {
+            Content = new StringContent("this is not json", Encoding.UTF8, "application/json")
+        });
+        var checker = CreateUpdateChecker(new FwLiteConfig { Os = FwLitePlatform.Windows });
+
+        (await checker.CheckForUpdate()).Should().BeNull();
+
+        _throttle.ShouldCheckForUpdate().Should().BeFalse();
+    }
+
+    [Fact]
+    public async Task TryUpdate_WhenCalledConcurrently_OnlyOneRequestIsMade()
+    {
+        //startup check in flight (slow DNS) while connectivity-regained triggers a retry: the second caller
+        //must wait for the first and then see the recorded check instead of fetching (and applying) again
+        var release = new TaskCompletionSource<HttpResponseMessage>();
+#pragma warning disable VSTHRD003 // the test owns this TaskCompletionSource and completes it below
+        var handler = UseAsyncHttpHandler(() => release.Task);
+#pragma warning restore VSTHRD003
+        var checker = CreateUpdateChecker(new FwLiteConfig { Os = FwLitePlatform.Windows });
+
+        var first = checker.TryUpdate();
+        var second = checker.TryUpdate();
+        //let both callers get as far as they can before the server answers
+        await Task.Delay(50);
+        handler.Requests.Should().Be(1);
+
+        release.SetResult(NoUpdateResponse());
+        await Task.WhenAll(first, second);
+
+        handler.Requests.Should().Be(1);
+        _throttle.ShouldCheckForUpdate().Should().BeFalse();
+    }
+
+    [Fact]
+    public async Task CheckForUpdate_WhenServerReturnsError_StillRecordsCheck()
+    {
+        //the server was reachable and answered, so hammering it again on every launch gains nothing
+        UseHttpHandler(() => new HttpResponseMessage(HttpStatusCode.InternalServerError));
+        var checker = CreateUpdateChecker(new FwLiteConfig { Os = FwLitePlatform.Windows });
+
+        (await checker.CheckForUpdate()).Should().BeNull();
+
+        _throttle.ShouldCheckForUpdate().Should().BeFalse();
+    }
 }

backend/FwLite/FwLiteShared/AppUpdate/UpdateChecker.cs

@@ -19,6 +19,7 @@ public class UpdateChecker(
     UpdateCheckThrottle throttle,
     IMemoryCache cache) : BackgroundService
 {
+    public const string HttpClientName = "Lexbox";
     private const string CacheKey = "ManualUpdateCheck";
     private static readonly TimeSpan CacheDuration = TimeSpan.FromMinutes(2);
 
@@ -27,25 +28,41 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
         await TryUpdate();
     }
 
+    private readonly SemaphoreSlim _automaticCheckLock = new(1, 1);
+
     public async Task<UpdateResult?> TryUpdate()
     {
-        if (!ShouldCheckReleaseFeed()) return null;
-        var update = await CheckForUpdate();
-        if (update is null) return null;
-        return await ApplyUpdate(update.Release);
+        //the startup check and a connectivity-regained retry can overlap. Both would pass the throttle before
+        //either records a response and apply the same update twice, so serialize them and evaluate the gate
+        //only once the previous attempt has finished.
+        await _automaticCheckLock.WaitAsync();
+        try
+        {
+            if (!ShouldCheckReleaseFeed()) return null;
+            var update = await CheckForUpdate();
+            if (update is null) return null;
+            return await ApplyUpdate(update.Release);
+        }
+        finally
+        {
+            _automaticCheckLock.Release();
+        }
     }
 
     public async Task<AvailableUpdate?> CheckForUpdate()
     {
-        return await cache.GetOrCreateAsync(CacheKey, async entry =>
-        {
-            entry.AbsoluteExpirationRelativeToNow = CacheDuration;
-            var response = await ShouldUpdateAsync();
-            throttle.RecordCheck();
-            return response.Update
-                ? new AvailableUpdate(response.Release, platformUpdateService.SupportsAutoUpdate)
-                : null;
-        });
+        if (cache.TryGetValue(CacheKey, out AvailableUpdate? cached)) return cached;
+        var response = await ShouldUpdateAsync();
+        //a request that never reached the server (offline, DNS still down while a VPN connects) is not a check:
+        //leave the throttle and the manual-check cache alone so the next launch or connectivity recovery
+        //retries instead of waiting out UpdateCheckInterval
+        if (response is null) return null;
+        throttle.RecordCheck();
+        var update = response.Update
+            ? new AvailableUpdate(response.Release, platformUpdateService.SupportsAutoUpdate)
+            : null;
+        cache.Set(CacheKey, update, CacheDuration);
+        return update;
     }
 
     public async Task<UpdateResult> ApplyUpdate(FwLiteRelease release)
@@ -96,16 +113,29 @@ private bool ShouldPromptBeforeUpdate()
         return platformUpdateService.IsOnMeteredConnection();
     }
 
-    private async Task<ShouldUpdateResponse> ShouldUpdateAsync()
+    /// <returns>The server's answer, or null when the request failed before getting a response.</returns>
+    private async Task<ShouldUpdateResponse?> ShouldUpdateAsync()
     {
+        HttpResponseMessage response;
         try
         {
-            var response = await httpClientFactory
-                .CreateClient("Lexbox")
+            response = await httpClientFactory
+                .CreateClient(HttpClientName)
                 .SendAsync(new HttpRequestMessage(HttpMethod.Get, config.Value.UpdateUrl)
                 {
                     Headers = { { "User-Agent", $"Fieldworks-Lite-Client/{config.Value.AppVersion}" } }
                 });
+        }
+        catch (Exception ex)
+        {
+            logger.LogError(ex, "Failed to fetch latest release");
+            return null;
+        }
+
+        //from here on the server has answered, so whatever goes wrong still counts as a check: a bad
+        //response is not fixed by asking again sooner than UpdateCheckInterval
+        try
+        {
             if (!response.IsSuccessStatusCode)
             {
                 var responseContent = await response.Content.ReadAsStringAsync();
@@ -120,7 +150,7 @@ private async Task<ShouldUpdateResponse> ShouldUpdateAsync()
         }
         catch (Exception ex)
         {
-            logger.LogError(ex, "Failed to fetch latest release");
+            logger.LogError(ex, "Failed to read should update response");
             return new ShouldUpdateResponse(null);
         }
     }

backend/FwLite/FwLiteShared/FwLiteSharedKernel.cs

@@ -16,7 +16,10 @@
 using Microsoft.Extensions.Options;
 using Microsoft.JSInterop;
 using MiniLcm.Project;
+using System.Globalization;
+using System.Net.Sockets;
 using Polly;
+using Polly.Simmy.Fault;
 using Polly.Simmy;
 using SIL.Harmony;
 
@@ -28,6 +31,8 @@ public static IServiceCollection AddFwLiteShared(this IServiceCollection service
     {
         services.AddMemoryCache();
         services.AddHttpClient();
+        var lexboxClientBuilder = services.AddHttpClient(UpdateChecker.HttpClientName);
+        if (ChaosEnabled(environment)) ConfigureHttpClientChaos(lexboxClientBuilder);
         services.AddHttpClient(MixpanelClient.HttpClientName, client =>
         {
             client.Timeout = TimeSpan.FromSeconds(10);
@@ -98,12 +103,9 @@ private static void AddAuthHelpers(this IServiceCollection services, IHostEnviro
         services.AddTransient<HttpClientRefreshDelegate>();
         var httpClientBuilder = services.AddHttpClient(OAuthClient.AuthHttpClientName);
         httpClientBuilder.AddHttpMessageHandler<HttpClientRefreshDelegate>();
+        if (ChaosEnabled(environment)) ConfigureHttpClientChaos(httpClientBuilder);
         if (environment.IsDevelopment())
         {
-            if (!string.IsNullOrEmpty(Environment.GetEnvironmentVariable("FW_LITE_CHAOS")))
-            {
-                ConfigureHttpClientChaos(httpClientBuilder);
-            }
             // Allow self-signed certificates in development
             httpClientBuilder.ConfigurePrimaryHttpMessageHandler(() =>
             {
@@ -116,14 +118,40 @@ private static void AddAuthHelpers(this IServiceCollection services, IHostEnviro
         }
     }
 
+    private static bool ChaosEnabled(IHostEnvironment environment)
+    {
+        return environment.IsDevelopment() &&
+               !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("FW_LITE_CHAOS"));
+    }
+
+    /// <summary>
+    /// FW_LITE_CHAOS=true injects chaos into 30% of requests; a number between 0 and 1 (e.g. 1.0) sets the
+    /// rate directly, which makes a specific failure reproducible instead of a dice roll.
+    /// </summary>
+    private static double ChaosInjectionRate()
+    {
+        var value = Environment.GetEnvironmentVariable("FW_LITE_CHAOS");
+        return double.TryParse(value, NumberStyles.Float, CultureInfo.InvariantCulture, out var rate)
+            ? Math.Clamp(rate, 0, 1)
+            : 0.3;
+    }
+
     private static void ConfigureHttpClientChaos(IHttpClientBuilder builder)
     {
         builder.AddResilienceHandler("chaos",
             pipelineBuilder =>
             {
-                const double injectionRate = 0.3;
+                var injectionRate = ChaosInjectionRate();
                 pipelineBuilder.AddChaosLatency(injectionRate, TimeSpan.FromSeconds(5))
-                    .AddChaosFault(injectionRate, () => new InvalidOperationException("Chaos injected fault"))
+                    .AddChaosFault(new ChaosFaultStrategyOptions
+                    {
+                        InjectionRate = injectionRate,
+                        FaultGenerator = new FaultGenerator()
+                            .AddException(() => new InvalidOperationException("Chaos injected fault"))
+                            //what SocketsHttpHandler throws when DNS is unreachable, e.g. while a VPN is still connecting
+                            .AddException(() => new HttpRequestException("No such host is known. (chaos)",
+                                new SocketException((int)SocketError.HostNotFound)))
+                    })
                     .AddChaosOutcome(new()
                     {
                         InjectionRate = injectionRate,