fix(dashboard): bounded alarm drains, feed resubscribe, live pill, drop dead hub factory; doc corrections

2026-08-16 04:12:25 -04:00
parent e1ff05c605
commit 3faa272db9
11 changed files with 240 additions and 127 deletions
@@ -21,6 +21,13 @@ public abstract class DashboardPageBase : ComponentBase, IAsyncDisposable
    /// </summary>
    private static readonly TimeSpan WatchDrainTimeout = TimeSpan.FromSeconds(5);

+    /// <summary>
+    /// Delay between a feed subscription ending and the resubscribe that replaces it.
+    /// Long enough that a feed failing on every attempt cannot spin, short enough that
+    /// the page is stale for about a snapshot interval rather than until navigation.
+    /// </summary>
+    private static readonly TimeSpan ResubscribeDelay = TimeSpan.FromSeconds(1);
+
    private readonly CancellationTokenSource _watchCancellation = new();
    private Task? _watchTask;

@@ -88,31 +95,71 @@ public abstract class DashboardPageBase : ComponentBase, IAsyncDisposable
        GC.SuppressFinalize(this);
    }

+    /// <summary>
+    /// Renders every snapshot the feed yields, resubscribing whenever the subscription ends
+    /// for any reason other than this page going away.
+    /// </summary>
+    /// <remarks>
+    /// The feed detaches a subscriber when its pump's source faults or completes, so a single
+    /// enumeration is not a lifetime: without the outer loop the first fault froze the page on
+    /// its last snapshot until the operator navigated. Resubscribing is also what restarts the
+    /// feed — only a subscriber that finds no live generation starts a pump — so the page is
+    /// the recovery path, not merely its beneficiary.
+    /// </remarks>
+    /// <param name="cancellationToken">Cancelled by <see cref="DisposeAsync"/> when the page goes away.</param>
+    /// <returns>A task that completes when the page is disposed.</returns>
    private async Task WatchSnapshotsAsync(CancellationToken cancellationToken)
    {
-        try
+        // One log line per fault *transition*, not per retry: a feed that is down stays down
+        // for many iterations, and a warning per second per open page is noise, not signal.
+        bool faultLogged = false;
+
+        while (!cancellationToken.IsCancellationRequested)
        {
-            await foreach (DashboardSnapshot snapshot in SnapshotFeed
-                .WatchAsync(cancellationToken)
-                .ConfigureAwait(false))
+            try
            {
-                Snapshot = snapshot;
-                await InvokeAsync(StateHasChanged).ConfigureAwait(false);
+                await foreach (DashboardSnapshot snapshot in SnapshotFeed
+                    .WatchAsync(cancellationToken)
+                    .ConfigureAwait(false))
+                {
+                    Snapshot = snapshot;
+                    faultLogged = false;
+                    await InvokeAsync(StateHasChanged).ConfigureAwait(false);
+                }
+            }
+            catch (OperationCanceledException)
+            {
+                // The page is going away.
+                return;
+            }
+            catch (Exception error) when (!faultLogged)
+            {
+                // The last rendered snapshot stays on screen while the retry runs, and the
+                // snapshot service keeps serving GetSnapshot() for the next page load.
+                faultLogged = true;
+                Logger?.LogWarning(
+                    error,
+                    "Live snapshot updates failed for dashboard page {Page}; retrying every {Delay}. "
+                    + "It keeps the last rendered snapshot until they resume.",
+                    GetType().Name,
+                    ResubscribeDelay);
+            }
+            catch (Exception)
+            {
+                // Same fault, already reported above; the retry below is unconditional.
+            }
+
+            // The enumeration's own disposal (run by the await foreach on every exit path)
+            // is what releases the dead subscription, so the delay below is only paced —
+            // there is nothing left of the old subscription to unwind here.
+            try
+            {
+                await Task.Delay(ResubscribeDelay, cancellationToken).ConfigureAwait(false);
+            }
+            catch (OperationCanceledException)
+            {
+                return;
            }
-        }
-        catch (OperationCanceledException)
-        {
-            // The page is going away.
-        }
-        catch (Exception error)
-        {
-            // The feed is best-effort: the last rendered snapshot stays on screen and the
-            // snapshot service keeps serving GetSnapshot() for the next page load. Logged
-            // once here, on the way out of the loop — never per snapshot.
-            Logger?.LogWarning(
-                error,
-                "Live snapshot updates ended for dashboard page {Page}; it keeps the last rendered snapshot.",
-                GetType().Name);
        }
    }
 }
@@ -168,6 +168,13 @@
    private int _maxSeverity = 1000;
    private string _search = string.Empty;

+    // Upper bound on waiting for either background loop while disposing, mirroring
+    // DashboardPageBase's snapshot-watch drain: both loops marshal renders through the
+    // renderer's dispatcher and disposal can run on that same dispatcher, so the wait is
+    // bounded rather than unconditional — an unconditional one hangs teardown for good on
+    // a wedged dispatcher.
+    private static readonly TimeSpan LoopDrainTimeout = TimeSpan.FromSeconds(5);
+
    private readonly CancellationTokenSource _cts = new();
    private Task? _pollTask;

@@ -336,29 +343,32 @@
    {
        await _cts.CancelAsync();

-        if (_pollTask is not null)
-        {
-            try
-            {
-                await _pollTask;
-            }
-            catch (OperationCanceledException)
-            {
-            }
-        }
-
-        if (_providerStatusTask is not null)
-        {
-            try
-            {
-                await _providerStatusTask;
-            }
-            catch (OperationCanceledException)
-            {
-            }
-        }
+        await DrainAsync(_pollTask);
+        await DrainAsync(_providerStatusTask);

        _cts.Dispose();
        GC.SuppressFinalize(this);
    }
+
+    // The accepted cost of the bound is an abandoned loop that keeps its alarm-service
+    // subscription (and its poll timer) until it does unwind; the alternative — waiting
+    // forever on a dispatcher that is not draining — wedges the circuit teardown itself.
+    private static async Task DrainAsync(Task? loop)
+    {
+        if (loop is null)
+        {
+            return;
+        }
+
+        try
+        {
+            await loop.WaitAsync(LoopDrainTimeout);
+        }
+        catch (TimeoutException)
+        {
+        }
+        catch (OperationCanceledException)
+        {
+        }
+    }
 }
@@ -1,5 +1,9 @@
@page "/sessions/{SessionId}"
@inherits DashboardPageBase
+@* Load-bearing: DisposeAsync below hides the base method with `new`, so Blazor only calls
+   it because this directive re-declares IAsyncDisposable on the derived component. Drop
+   this line and the base's DisposeAsync runs instead — the event subscription and pump
+   leak, silently. *@
@implements IAsyncDisposable
@using ZB.MOM.WW.MxGateway.Contracts.Proto
@using ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs
@@ -358,6 +362,43 @@ else
            // Either the renderer went away mid-dispatch, or the drain below timed out
            // and disposed the cancellation source this loop is still reading.
        }
+        finally
+        {
+            await MarkDisconnectedAsync(subscription).ConfigureAwait(false);
+        }
+    }
+
+    // This pump is the only thing feeding the "live" pill, so the pill goes dark the moment
+    // the pump stops — the subscription's channel completing under a page that is still
+    // watching (the broadcaster dropped it, the session ended) is exactly what the pill
+    // exists to show, and without this it read "live" until navigation.
+    // A pump whose subscription has already been replaced must not touch it: the newer
+    // subscription's pump owns the pill now. Same dispatcher-owned identity check the
+    // render batch uses — and detach nulls _eventSubscription before cancelling, so a
+    // detach-driven exit correctly falls through here without repainting.
+    private async Task MarkDisconnectedAsync(IDashboardEventSubscription subscription)
+    {
+        try
+        {
+            await InvokeAsync(() =>
+            {
+                if (!ReferenceEquals(_eventSubscription, subscription))
+                {
+                    return;
+                }
+
+                _eventsConnected = false;
+                StateHasChanged();
+            }).ConfigureAwait(false);
+        }
+        catch (ObjectDisposedException)
+        {
+            // The renderer went away; there is no pill left to repaint.
+        }
+        catch (OperationCanceledException)
+        {
+            // The circuit is tearing down; same.
+        }
    }

    private async Task DetachEventsAsync()
@@ -401,6 +442,11 @@ else
            : string.Empty;
    }

+    // `new` hides DashboardPageBase.DisposeAsync rather than overriding it (the base method
+    // is not virtual), so this runs only via the IAsyncDisposable interface slot the
+    // `@implements IAsyncDisposable` directive at the top of this file re-declares on the
+    // derived type. Remove either half and disposal silently resolves to the base method:
+    // the snapshot watch is cancelled, the event subscription and pump are not.
    public new async ValueTask DisposeAsync()
    {
        await DetachEventsAsync();
@@ -45,8 +45,10 @@ public static class DashboardServiceCollectionExtensions
        services.AddSingleton<DashboardApiKeyAuthorization>();
        services.AddSingleton<IDashboardApiKeyManagementService, DashboardApiKeyManagementService>();
        services.AddSingleton<IDashboardSessionAdminService, DashboardSessionAdminService>();
+        // Singleton, and the only consumer scope left is HubTokenAuthenticationHandler plus
+        // the /hubs/token endpoint: server-rendered pages read the in-process feeds, so
+        // nothing in this process builds a hub connection or needs a token for one.
        services.AddSingleton<HubTokenService>();
-        services.AddScoped<Hubs.DashboardHubConnectionFactory>();
        services.AddScoped<IDashboardBrowseService, DashboardBrowseService>();
        // Singleton: EventsHub instances are transient (one per hub invocation), so the
        // subscriber bookkeeping they share with the broadcaster must outlive them.
@@ -14,10 +14,12 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard;
 /// <remarks>
 /// This service is registered as a singleton in
 /// <see cref="DashboardServiceCollectionExtensions.AddGatewayDashboard"/> and
-/// is shared by two consumer scopes: <c>DashboardHubConnectionFactory</c>
-/// (scoped, per-circuit; calls <see cref="Issue"/> from the cookie-authenticated
-/// dashboard) and <c>HubTokenAuthenticationHandler</c> (transient, per-request;
-/// calls <see cref="Validate"/> from the SignalR negotiate / connection path).
+/// is shared by two consumer scopes: the <c>/hubs/token</c> endpoint (calls
+/// <see cref="Issue"/> for a cookie-authenticated caller) and
+/// <c>HubTokenAuthenticationHandler</c> (transient, per-request; calls
+/// <see cref="Validate"/> from the SignalR negotiate / connection path). Both
+/// serve external/remote hub consumers — server-rendered dashboard pages read the
+/// in-process feeds and never mint a hub token.
 /// The underlying <see cref="ITimeLimitedDataProtector"/> is thread-safe, so
 /// minting and validating concurrently from any number of callers is safe;
 /// future maintainers should preserve the singleton lifetime to keep the
@@ -31,9 +33,10 @@ public sealed class HubTokenService
    // revocable. A short lifetime bounds the exposure window of a token captured from a proxy
    // or log after logout (the cookie is cleared on logout, but outstanding tokens are not), and
    // bounds how long a stale role set survives a role change. Five minutes is transparent to
-    // clients because DashboardHubConnectionFactory mints a fresh token on every (re)connect;
-    // see docs/GatewayDashboardDesign.md. Heavier jti-denylist revocation is deliberately
-    // deferred until per-session hub ACLs land, when tokens gain session binding.
+    // clients that re-fetch from /hubs/token on every (re)connect, which is what a remote hub
+    // consumer is expected to do; see docs/GatewayDashboardDesign.md. Heavier jti-denylist
+    // revocation is deliberately deferred until per-session hub ACLs land, when tokens gain
+    // session binding.
    internal static readonly TimeSpan TokenLifetime = TimeSpan.FromMinutes(5);

    private readonly ITimeLimitedDataProtector _protector;
@@ -1,39 +0,0 @@
-using Microsoft.AspNetCore.Components;
-using Microsoft.AspNetCore.Components.Authorization;
-using Microsoft.AspNetCore.SignalR.Client;
-
-namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs;
-
-/// <summary>
-/// Client-side helper that builds a <see cref="HubConnection"/> targeted at a
-/// dashboard hub. Mints a fresh data-protected bearer token via
-/// <see cref="HubTokenService"/> on every (re)connect so the connection
-/// authenticates against <see cref="DashboardAuthenticationDefaults.HubAuthenticationScheme"/>
-/// without needing to forward the browser's HttpOnly cookie.
-/// </summary>
-public sealed class DashboardHubConnectionFactory(
-    NavigationManager navigation,
-    HubTokenService tokens,
-    AuthenticationStateProvider authState)
-{
-    /// <summary>Creates a new hub connection to the specified hub path.</summary>
-    /// <param name="hubPath">The relative hub path (e.g., "/hubs/snapshot").</param>
-    /// <returns>A configured hub connection with automatic reconnection and token authentication.</returns>
-    public HubConnection Create(string hubPath)
-    {
-        ArgumentException.ThrowIfNullOrWhiteSpace(hubPath);
-
-        Uri hubUrl = navigation.ToAbsoluteUri(hubPath);
-        return new HubConnectionBuilder()
-            .WithUrl(hubUrl, options =>
-            {
-                options.AccessTokenProvider = async () =>
-                {
-                    AuthenticationState state = await authState.GetAuthenticationStateAsync().ConfigureAwait(false);
-                    return tokens.Issue(state.User);
-                };
-            })
-            .WithAutomaticReconnect()
-            .Build();
-    }
-}