diff --git a/.claude/launch.json b/.claude/launch.json index b513cccdc2..1797b6d4bb 100644 --- a/.claude/launch.json +++ b/.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 } ] } diff --git a/backend/FwLite/FwLiteMaui/Services/ConnectivitySyncTrigger.cs b/backend/FwLite/FwLiteMaui/Services/ConnectivitySyncTrigger.cs index 8dcb9dd19c..09be469563 100644 --- a/backend/FwLite/FwLiteMaui/Services/ConnectivitySyncTrigger.cs +++ b/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 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) diff --git a/backend/FwLite/FwLiteShared.Tests/AppUpdate/UpdateCheckerTests.cs b/backend/FwLite/FwLiteShared.Tests/AppUpdate/UpdateCheckerTests.cs index ceec805d4c..4d3fdff7cc 100644 --- a/backend/FwLite/FwLiteShared.Tests/AppUpdate/UpdateCheckerTests.cs +++ b/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>()); + _throttle = new UpdateCheckThrottle(_preferences, options, Mock.Of>()); + var throttle = _throttle; return new UpdateChecker( _httpClientFactoryMock.Object, Mock.Of>(), @@ -78,4 +86,124 @@ public void ShouldCheckReleaseFeed_WhenConfigSetToNever_ReturnsFalse() checker.ShouldCheckReleaseFeed().Should().BeFalse(); } + + /// Routes the Lexbox client through and counts the requests. + private StubHandler UseHttpHandler(Func send) => + UseAsyncHttpHandler(() => Task.FromResult(send())); + + private StubHandler UseAsyncHttpHandler(Func> send) + { + var handler = new StubHandler(send); + _httpClientFactoryMock.Setup(f => f.CreateClient(UpdateChecker.HttpClientName)) + .Returns(() => new HttpClient(handler, false)); + return handler; + } + + private class StubHandler(Func> send) : HttpMessageHandler + { + private int _requests; + public int Requests => _requests; + + protected override Task 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(); +#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(); + } } diff --git a/backend/FwLite/FwLiteShared/AppUpdate/UpdateChecker.cs b/backend/FwLite/FwLiteShared/AppUpdate/UpdateChecker.cs index a9febc79c9..e2eee0f79f 100644 --- a/backend/FwLite/FwLiteShared/AppUpdate/UpdateChecker.cs +++ b/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 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 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 ApplyUpdate(FwLiteRelease release) @@ -96,16 +113,29 @@ private bool ShouldPromptBeforeUpdate() return platformUpdateService.IsOnMeteredConnection(); } - private async Task ShouldUpdateAsync() + /// The server's answer, or null when the request failed before getting a response. + private async Task 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 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); } } diff --git a/backend/FwLite/FwLiteShared/FwLiteSharedKernel.cs b/backend/FwLite/FwLiteShared/FwLiteSharedKernel.cs index 6898389487..6b13e4f718 100644 --- a/backend/FwLite/FwLiteShared/FwLiteSharedKernel.cs +++ b/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(); var httpClientBuilder = services.AddHttpClient(OAuthClient.AuthHttpClientName); httpClientBuilder.AddHttpMessageHandler(); + 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")); + } + + /// + /// 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. + /// + 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, diff --git a/backend/FwLite/FwLiteWeb/Services/NetworkChangeSyncTrigger.cs b/backend/FwLite/FwLiteWeb/Services/NetworkChangeSyncTrigger.cs index e05d001d4e..ebd88039f7 100644 --- a/backend/FwLite/FwLiteWeb/Services/NetworkChangeSyncTrigger.cs +++ b/backend/FwLite/FwLiteWeb/Services/NetworkChangeSyncTrigger.cs @@ -1,17 +1,19 @@ using System.Net.NetworkInformation; +using FwLiteShared.AppUpdate; using FwLiteShared.Projects; namespace FwLiteWeb.Services; // Cross-platform counterpart to MAUI's ConnectivitySyncTrigger, for hosts without IConnectivity: when the OS // reports network availability returning, re-ensure push listeners so a session started offline picks up -// without waiting on PushListenerRecoveryService's periodic backstop. NetworkAvailabilityChanged is the +// without waiting on PushListenerRecoveryService's periodic backstop, and retry the startup update check. NetworkAvailabilityChanged is the // System.Net.NetworkInformation analog of MAUI's ConnectivityChanged and is edge-triggered (it only fires on // a change), so e.IsAvailable alone is the "came back online" signal — no previous-state tracking needed. // It shares GetIsNetworkAvailable's optimism (a virtual adapter keeps availability true), so it can miss a // real-uplink recovery; the periodic backstop still covers those. public sealed class NetworkChangeSyncTrigger( LexboxProjectChangeListener lexboxProjectChangeListener, + UpdateChecker updateChecker, ILogger logger) : IHostedService { public Task StartAsync(CancellationToken cancellationToken) @@ -32,6 +34,21 @@ private void OnNetworkAvailabilityChanged(object? sender, NetworkAvailabilityEve if (!e.IsAvailable) return; logger.LogInformation("Network availability regained; 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 network availability change"); + } } private async Task EnsureListeners(CancellationToken cancellationToken = default) diff --git a/backend/FwLite/Taskfile.yml b/backend/FwLite/Taskfile.yml index 3b2c919046..88d0dc7d01 100644 --- a/backend/FwLite/Taskfile.yml +++ b/backend/FwLite/Taskfile.yml @@ -18,7 +18,7 @@ tasks: dir: ./FwLiteWeb cmd: dotnet run -- --FwLite:UseDevAssets=false web-chaos: - desc: Run FwLiteWeb with some Chaos injected to http requests to lexbox, requests will have chaos 30% of the time, this includes some latency of 5 seconds + desc: Run FwLiteWeb with some Chaos injected to http requests to lexbox (auth + update check), requests will have chaos 30% of the time (set FW_LITE_CHAOS to a rate like 1.0 to override), this includes 5 seconds of latency, 500 responses and DNS-style "No such host" failures dir: ./FwLiteWeb env: FW_LITE_CHAOS: true