Hardened background work against disconnected circuits (#937)

This commit is contained in:
Thorsten Sommer authored and GitHub committed 2026-08-25 18:53:09 +02:00
1 parent 99089f853e
commit f03c2d1c88
44 files changed
+500 -126

No files matched your search

@@ -131,7 +131,12 @@ public sealed class AIJobService(SettingsManager settingsManager, MessageBus mes
await CheckpointChatAsync(state, force: true);
await this.NotifyChangedAsync(state);
_ = Task.Factory.StartNew(async () => await this.RunChatGenerationAsync(state), TaskCreationOptions.LongRunning);
//
// Unwrap matters here: StartNew with an async delegate hands back a task which completes as soon
// as the generation started, wrapping the task which does the actual work. Watching the outer one
// would tell us nothing about how the generation itself ended.
//
Task.Factory.StartNew(async () => await this.RunChatGenerationAsync(state), TaskCreationOptions.LongRunning).Unwrap().Observe($"{nameof(AIJobService)}: running a chat generation");
return state.Snapshot;
}
@@ -1,16 +1,121 @@
using AIStudio.Assistants;
using AIStudio.Tools.Services;
namespace AIStudio.Tools;
public static class JsRuntimeExtensions
{
private static readonly ILogger LOGGER = Program.LOGGER_FACTORY.CreateLogger(nameof(JsRuntimeExtensions));
public static async Task GenerateAndShowDiff(this IJSRuntime jsRuntime, string text1, string text2)
{
await jsRuntime.InvokeVoidAsync("generateDiff", text1, text2, AssistantLowerBase.RESULT_DIV_ID, AssistantLowerBase.BEFORE_RESULT_DIV_ID);
}
public static async Task ClearDiv(this IJSRuntime jsRuntime, string divId)
{
await jsRuntime.InvokeVoidAsync("clearDiv", divId);
}
/// <summary>
/// Calls a JavaScript function which returns nothing, and tolerates a circuit which is already gone.
/// </summary>
/// <remarks>
/// Blazor cannot issue JS interop calls once the browser connection of a circuit is gone. That happens
/// during every reload and while a component gets disposed, so the failure is expected rather than
/// exceptional. Discarding such a call is not an option, though: the discarded task keeps the fault
/// until the finalizer reports it as an unobserved task exception, without any hint at its origin.
/// This method is the one place which knows how to await such a call and what to do with its failure.
/// </remarks>
/// <param name="jsRuntime">The JS runtime to call.</param>
/// <param name="identifier">The name of the JavaScript function.</param>
/// <param name="args">The arguments for the JavaScript function.</param>
/// <returns>True when the browser ran the function. Callers which remember what they told the browser
/// must check this: a call which never arrived leaves the browser in its previous state.</returns>
public static async ValueTask<bool> TryInvokeVoidAsync(this IJSRuntime jsRuntime, string identifier, params object?[]? args)
{
try
{
await jsRuntime.InvokeVoidAsync(identifier, args);
return true;
}
catch (Exception exception)
{
LogInvocationFailure(exception, identifier);
return false;
}
}
/// <summary>
/// Calls a JavaScript function which returns nothing, unless the circuit is known to be disconnected.
/// </summary>
/// <remarks>
/// Prefer this over the variant without a circuit state wherever the caller knows its circuit. While a
/// browser connection is gone, every single call would otherwise throw, which is needless work for
/// something we already know cannot succeed — a component of a disconnected circuit which keeps
/// rendering would produce one such exception per render.
/// </remarks>
/// <param name="jsRuntime">The JS runtime to call.</param>
/// <param name="circuitState">The circuit of the caller.</param>
/// <param name="identifier">The name of the JavaScript function.</param>
/// <param name="args">The arguments for the JavaScript function.</param>
/// <returns>True when the browser ran the function, false when it was skipped or failed.</returns>
public static async ValueTask<bool> TryInvokeVoidAsync(this IJSRuntime jsRuntime, CircuitStateService circuitState, string identifier, params object?[]? args)
{
if (!circuitState.IsConnected)
{
LOGGER.LogDebug("The JS call '{Identifier}' was skipped because the browser connection of the circuit '{CircuitId}' is down.", identifier, circuitState.CircuitId);
return false;
}
return await jsRuntime.TryInvokeVoidAsync(identifier, args);
}
/// <summary>
/// Calls a function of a JavaScript module which returns nothing, and tolerates a circuit which is
/// already gone. See the remarks on the JS runtime variant of this method.
/// </summary>
/// <param name="module">The JavaScript module to call.</param>
/// <param name="identifier">The name of the function inside the module.</param>
/// <param name="args">The arguments for the function.</param>
/// <returns>True when the browser ran the function, false when it failed.</returns>
public static async ValueTask<bool> TryInvokeVoidAsync(this IJSObjectReference module, string identifier, params object?[]? args)
{
try
{
await module.InvokeVoidAsync(identifier, args);
return true;
}
catch (Exception exception)
{
LogInvocationFailure(exception, identifier);
return false;
}
}
private static void LogInvocationFailure(Exception exception, string identifier)
{
switch (exception)
{
//
// The circuit is disconnected or disposed, or the call was canceled while it was on its way.
// None of this is a defect: it is what a reload, a lost connection, or a disposed component
// looks like from here.
//
case JSDisconnectedException:
case ObjectDisposedException:
case OperationCanceledException:
LOGGER.LogDebug("The JS call '{Identifier}' was not completed because the browser connection was gone: {Reason}", identifier, exception.Message);
break;
// The call reached the browser, but failed there. That is worth knowing about:
case JSException:
LOGGER.LogWarning(exception, "The JS call '{Identifier}' failed in the browser.", identifier);
break;
default:
LOGGER.LogError(exception, "The JS call '{Identifier}' failed unexpectedly.", identifier);
break;
}
}
}
+75 -3
View File
@@ -1,5 +1,7 @@
using System.Collections.Concurrent;
using AIStudio.Tools.Services;
using Microsoft.AspNetCore.Components;
// ReSharper disable RedundantRecordClassKeyword
@@ -11,6 +13,7 @@ public sealed class MessageBus
private readonly ConcurrentDictionary<IMessageBusReceiver, ComponentBase[]> componentFilters = new();
private readonly ConcurrentDictionary<IMessageBusReceiver, Event[]> componentEvents = new();
private readonly ConcurrentDictionary<IMessageBusReceiver, CircuitStateService> receiverCircuits = new();
private readonly ConcurrentDictionary<Event, ConcurrentQueue<Message>> deferredMessages = new();
private readonly ConcurrentQueue<Message> messageQueue = new();
private readonly SemaphoreSlim sendingSemaphore = new(1, 1);
@@ -39,16 +42,53 @@ public sealed class MessageBus
this.componentEvents[receiver] = events.ToArray();
}
public void RegisterComponent(IMessageBusReceiver receiver)
/// <summary>
/// Registers a receiver at the bus.
/// </summary>
/// <param name="receiver">That's you, the receiver.</param>
/// <param name="circuitState">The circuit this receiver belongs to. Components hand over their circuit
/// so the bus can let them go when that circuit ends. Services which live longer than any circuit,
/// such as hosted services, hand over nothing.</param>
public void RegisterComponent(IMessageBusReceiver receiver, CircuitStateService? circuitState = null)
{
this.componentFilters.TryAdd(receiver, []);
this.componentEvents.TryAdd(receiver, []);
if (circuitState is not null)
this.receiverCircuits[receiver] = circuitState;
}
public void Unregister(IMessageBusReceiver receiver)
{
this.componentFilters.TryRemove(receiver, out _);
this.componentEvents.TryRemove(receiver, out _);
this.receiverCircuits.TryRemove(receiver, out _);
}
/// <summary>
/// Removes all receivers which belong to one circuit.
/// </summary>
/// <remarks>
/// The circuit handler calls this when a circuit ends. Components deregister themselves when they get
/// disposed, but a circuit which was retained and then dropped does not give all of them that chance.
/// Since the bus holds a strong reference to every receiver, those leftovers would stay and would be
/// served forever.
/// </remarks>
/// <param name="circuitState">The circuit whose receivers must go.</param>
/// <returns>The number of removed receivers.</returns>
public int UnregisterCircuit(CircuitStateService circuitState)
{
var numRemovedReceivers = 0;
foreach (var (receiver, receiverCircuit) in this.receiverCircuits)
{
if (!ReferenceEquals(receiverCircuit, circuitState))
continue;
this.Unregister(receiver);
numRemovedReceivers++;
}
return numRemovedReceivers;
}
private record class Message(ComponentBase? SendingComponent, Event TriggeredEvent, object? Data);
@@ -71,7 +111,7 @@ public sealed class MessageBus
if (eventFilter.Length == 0 || eventFilter.Contains(message.TriggeredEvent))
// We don't await the task here because we don't want to block the message bus:
_ = receiver.ProcessMessage(message.SendingComponent, message.TriggeredEvent, message.Data);
_ = DeliverMessage(receiver, message);
}
}
}
@@ -85,6 +125,38 @@ public sealed class MessageBus
}
}
/// <summary>
/// Hands one message to one receiver and observes how that went.
/// </summary>
/// <remarks>
/// The bus must not wait for a receiver, since one slow receiver would hold up everybody else. Not
/// waiting is not the same as not caring, though: a receiver whose circuit is gone fails with a
/// disconnect or disposal exception, and nobody would ever see where it came from. Such a task
/// carries its fault until the finalizer reports it as an unobserved task exception — naming a task
/// type instead of the receiver and the event. This is where we give those failures a name.
/// </remarks>
/// <param name="receiver">The receiver of the message.</param>
/// <param name="message">The message to deliver.</param>
private static async Task DeliverMessage(IMessageBusReceiver receiver, Message message)
{
try
{
await receiver.ProcessMessage(message.SendingComponent, message.TriggeredEvent, message.Data);
}
catch (Exception exception) when (exception is JSDisconnectedException or ObjectDisposedException or OperationCanceledException)
{
//
// Expected whenever the browser connection of a receiver is gone: the app keeps circuits
// of reloaded or sleeping windows around, and their components still receive events.
//
LOG?.LogDebug("The receiver '{ReceiverName}' did not process the event '{Event}' because its circuit was gone: {Reason}", receiver.GetType().Name, message.TriggeredEvent, exception.Message);
}
catch (Exception exception)
{
LOG?.LogError(exception, "The receiver '{ReceiverName}' failed while processing the event '{Event}'.", receiver.GetType().Name, message.TriggeredEvent);
}
}
public Task SendError(DataErrorMessage dataErrorMessage) => this.SendMessage(null, Event.SHOW_ERROR, dataErrorMessage);
public Task SendWarning(DataWarningMessage dataWarningMessage) => this.SendMessage(null, Event.SHOW_WARNING, dataWarningMessage);
@@ -50,7 +50,7 @@ public static partial class PluginFactory
LOG.LogInformation($"Start hot reloading plugins for path '{HOT_RELOAD_WATCHER.Path}'.");
try
{
HOT_RELOAD_DEBOUNCE_TIMER.Elapsed += (_, _) => _ = ReloadPluginsAsync();
HOT_RELOAD_DEBOUNCE_TIMER.Elapsed += (_, _) => ReloadPluginsAsync().Observe($"{nameof(PluginFactory)}: hot reloading plugins");
HOT_RELOAD_WATCHER.IncludeSubdirectories = true;
@@ -0,0 +1,61 @@
using Microsoft.AspNetCore.Components.Server.Circuits;
namespace AIStudio.Tools.Services;
/// <summary>
/// Follows the life of one circuit, so the rest of the app knows when its browser is unreachable.
/// </summary>
/// <remarks>
/// The app keeps disconnected circuits for a long time on purpose, cf. the retention settings in
/// Program.cs. That is what lets a user return to a working app after the machine woke up — but it also
/// means that the components of reloaded or sleeping windows stay alive and keep receiving events. They
/// may keep working: everything they do on the server is fine. Only JavaScript interop is impossible
/// while the connection is gone. So this handler does two things, and deliberately nothing more:
/// it publishes the connection state, and it cleans up once a circuit is truly over.
/// </remarks>
public sealed class AIStudioCircuitHandler(CircuitStateService circuitState, MessageBus messageBus, ILogger<AIStudioCircuitHandler> logger)
: CircuitHandler
{
#region Overrides of CircuitHandler
public override Task OnCircuitOpenedAsync(Circuit circuit, CancellationToken cancellationToken)
{
circuitState.AssignCircuit(circuit.Id);
logger.LogInformation("The circuit '{CircuitId}' was opened.", circuit.Id);
return Task.CompletedTask;
}
public override Task OnConnectionUpAsync(Circuit circuit, CancellationToken cancellationToken)
{
circuitState.MarkAsConnected();
logger.LogInformation("The browser connection of the circuit '{CircuitId}' is up.", circuit.Id);
return Task.CompletedTask;
}
public override Task OnConnectionDownAsync(Circuit circuit, CancellationToken cancellationToken)
{
circuitState.MarkAsDisconnected();
logger.LogInformation("The browser connection of the circuit '{CircuitId}' is down. Its JavaScript interop is paused until it returns.", circuit.Id);
return Task.CompletedTask;
}
public override Task OnCircuitClosedAsync(Circuit circuit, CancellationToken cancellationToken)
{
circuitState.MarkAsDisconnected();
//
// The components of this circuit will not come back, so nobody would ever deregister them:
// Blazor disposes components of a retained circuit without giving them a chance to run their
// disposal in every case. Without this, the message bus would keep and serve them forever.
//
var numRemovedReceivers = messageBus.UnregisterCircuit(circuitState);
logger.LogInformation("The circuit '{CircuitId}' was closed. Removed {NumReceivers} message bus receiver(s) of that circuit.", circuit.Id, numRemovedReceivers);
return Task.CompletedTask;
}
#endregion
}
@@ -0,0 +1,46 @@
namespace AIStudio.Tools.Services;
/// <summary>
/// Knows whether the browser connection of one circuit is currently up.
/// </summary>
/// <remarks>
/// There is one instance of this service per circuit, i.e. per browser window. It exists because the app
/// keeps disconnected circuits around for a long time, cf. the retention settings in Program.cs: after a
/// reload or while the machine sleeps, the components of the old circuit are still alive and still receive
/// events. They may do their work as before — only JavaScript interop is impossible while the connection
/// is gone. This is what tells them apart. Only the circuit handler changes this state.
/// </remarks>
public sealed class CircuitStateService
{
private volatile bool isConnected = true;
/// <summary>
/// True, as long as the browser of this circuit is reachable, and thus JS interop is possible.
/// </summary>
/// <remarks>
/// This starts as true: a circuit is created for a connected browser, and the handler reports the
/// first connection only afterwards. Starting as false would block the interop of the first render.
/// </remarks>
public bool IsConnected => this.isConnected;
/// <summary>
/// The ID of this circuit, for logging purposes. It is "n/a" until the circuit was opened.
/// </summary>
public string CircuitId { get; private set; } = "n/a";
/// <summary>
/// Called by the circuit handler when the circuit was opened.
/// </summary>
/// <param name="circuitId">The ID of the opened circuit.</param>
public void AssignCircuit(string circuitId) => this.CircuitId = circuitId;
/// <summary>
/// Called by the circuit handler when the browser connection was established or restored.
/// </summary>
public void MarkAsConnected() => this.isConnected = true;
/// <summary>
/// Called by the circuit handler when the browser connection was lost or the circuit ended.
/// </summary>
public void MarkAsDisconnected() => this.isConnected = false;
}
@@ -173,7 +173,7 @@ public sealed class MediaTranscriptionService(
}
this.UpdateImportState(target, Path.GetFileName(mediaPaths[0]), MediaTranscriptionPhase.QUEUED, null, MediaImportStatus.QUEUED);
_ = Task.Run(() => this.RunAttachmentBatchAsync(mediaPaths, target, ownerChat));
Task.Run(() => this.RunAttachmentBatchAsync(mediaPaths, target, ownerChat)).Observe($"{nameof(MediaTranscriptionService)}: running an attachment batch");
return true;
}
@@ -191,7 +191,7 @@ public sealed class MediaTranscriptionService(
}
this.UpdateImportState(target, Path.GetFileName(mediaPath), MediaTranscriptionPhase.QUEUED, null, MediaImportStatus.QUEUED);
_ = Task.Run(() => this.RunTextImportAsync(mediaPath, target));
Task.Run(() => this.RunTextImportAsync(mediaPath, target)).Observe($"{nameof(MediaTranscriptionService)}: running a text import");
return true;
}
@@ -72,8 +72,8 @@ public sealed class RustAvailabilityMonitorService : BackgroundService, IMessage
// be a transient issue.
//
_ = this.VerifyRustAvailability();
_ = this.VerifyRustAvailability();
this.VerifyRustAvailability().Observe($"{nameof(RustAvailabilityMonitorService)}: verifying the Rust availability");
this.VerifyRustAvailability().Observe($"{nameof(RustAvailabilityMonitorService)}: verifying the Rust availability");
}
if (numEvents <= UNAVAILABLE_EVENT_THRESHOLD)
@@ -26,7 +26,12 @@ public sealed partial class RustService
{
try
{
// Fire-and-forget the log event to avoid blocking:
//
// Fire-and-forget the log event to avoid blocking. This is the one place which deliberately
// discards its task instead of observing it: observing means logging the failure, and logging
// means sending another log event through this very method. A broken connection to Rust would
// feed itself. The unobserved task exception handler in Program.cs remains the safety net here.
//
var request = new LogEventRequest(timestamp, level, category, message, exception, stackTrace);
_ = this.http.PostAsJsonAsync("/log/event", request, this.jsonRustSerializerOptions);
}
@@ -0,0 +1,53 @@
namespace AIStudio.Tools;
public static class TaskExtensions
{
private static readonly ILogger LOGGER = Program.LOGGER_FACTORY.CreateLogger(nameof(TaskExtensions));
/// <summary>
/// Lets a task run on its own, but keeps an eye on how it ends.
/// </summary>
/// <remarks>
/// Use this wherever a task is started without awaiting it. Discarding one instead means that nobody
/// ever looks at its outcome: the task carries its exception until the garbage collector finalizes it,
/// and only then does it show up as an unobserved task exception — naming a task type, without any
/// hint at what was running. Around a circuit which is gone, that is the common case: components of a
/// reloaded or sleeping window still receive events and still schedule work.
/// </remarks>
/// <param name="task">The task to watch.</param>
/// <param name="context">What this task was doing, for the log entry.</param>
public static void Observe(this Task task, string context)
{
task.ContinueWith(finishedTask =>
LogFailure(finishedTask.Exception, context),
CancellationToken.None,
TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously,
TaskScheduler.Default);
}
private static void LogFailure(AggregateException? exception, string context)
{
if (exception is null)
return;
foreach (var innerException in exception.Flatten().InnerExceptions)
{
switch (innerException)
{
//
// The browser connection is gone, or the component was disposed while its work was still
// on its way. Neither is a defect: it is what a reload or a closed window looks like.
//
case JSDisconnectedException:
case ObjectDisposedException:
case OperationCanceledException:
LOGGER.LogDebug("Background work '{Context}' stopped because its circuit was gone: {Reason}", context, innerException.Message);
break;
default:
LOGGER.LogError(innerException, "Background work '{Context}' failed.", context);
break;
}
}
}
}