dohertj2/lmxopcua
1
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(adminui): Monaco dispose on a gone circuit is expected, not an error (#902)

Browse Source 
Closing a tab with the script editor open ended the circuit; MonacoEditor.DisposeAsync then
called MonacoBlazor.dispose through SafeInvokeAsync, which caught only JSDisconnectedException,
so the TaskCanceledException from the torn-down circuit escaped and was logged at [ERR].

DisposeAsync now treats JSDisconnectedException, OperationCanceledException (incl.
TaskCanceledException) and ObjectDisposedException as the expected gone-circuit outcome and logs
them at Debug; a real JSException from a live page is still logged (Warning); the
DotNetObjectReference is released in a finally. MonacoEditor is the only component in the repo
that uses JS interop, so there is nothing else to align.

bUnit tests: dispose throwing each of the three gone-circuit exceptions neither escapes nor logs
above Debug (with a positive control that the failing call ran), and a JSException still warns.
They fail against the previous DisposeAsync.

Claude-Session: https://claude.ai/code/session_01GT67ACmpMR7b9LGRPLijMi
This commit is contained in:
Joseph Doherty
2026-11-13 01:50:37 -05:00
parent d528838864
commit b85e004bdb
3 changed files with 127 additions and 198 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/MonacoEditor.razor
+33 -5
View File
@@ -237,12 +237,40 @@
public async ValueTask DisposeAsync()
{
if (_initialized)
try
{
// Disposal commonly races a circuit disconnect — JSDisconnectedException
// here is expected and silent; a real JSException is still logged.
await SafeInvokeAsync("MonacoBlazor.dispose", "dispose editor", _id);
if (_initialized)
{
try
{
await JS.InvokeVoidAsync("MonacoBlazor.dispose", _id);
}
catch (Exception ex) when (IsGoneCircuit(ex))
{
// lmxopcua#902: disposal normally runs because the circuit is ending (tab closed, navigation,
// reconnect timeout), so the JS side — and the editor with it — is already gone. The interop
// call then fails with JSDisconnectedException, or with TaskCanceledException /
// ObjectDisposedException when the circuit tore down mid-call. Nothing is left to clean up.
Logger.LogDebug("Monaco editor {EditorId}: JS dispose skipped, the circuit is gone ({Reason}).",
_id, ex.GetType().Name);
}
catch (JSException ex)
{
// The page is alive but Monaco's own dispose threw — a real fault, surfaced.
Logger.LogWarning(ex, "Monaco editor {EditorId}: failed to dispose editor.", _id);
}
}
}
finally
{
_dotNetRef?.Dispose();
}
_dotNetRef?.Dispose();
}
/// <summary>
/// True for the failures a JS-interop call raises when the circuit (or the page) has already gone away — the
/// expected outcome of interop issued from <see cref="DisposeAsync"/>.
/// </summary>
private static bool IsGoneCircuit(Exception ex) =>
ex is JSDisconnectedException or OperationCanceledException or ObjectDisposedException;
}
tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Deployments/DeploymentDriftProbeSqlServerTests.cs
-193
View File
@@ -1,193 +0,0 @@
using Microsoft.EntityFrameworkCore;
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.AdminUI.Deployments;
using ZB.MOM.WW.OtOpcUa.Configuration;
using ZB.MOM.WW.OtOpcUa.Configuration.Entities;
namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Deployments;
/// <summary>
/// The SQL-Server-backed half of <see cref="DeploymentDriftProbeTests"/>: pins that
/// <c>DeploymentDriftProbe</c>'s watermark aggregate — <c>ConfigEdits.GroupBy(_ =&gt; 1).Select(g =&gt;
/// new { g.Max(...), g.Count() })</c> — actually <b>translates</b>, and translates to the right
/// values, on the real provider.
/// </summary>
/// <remarks>
/// <para>
/// EF InMemory evaluates that query with LINQ-to-Objects, so the InMemory suite proves the C#
/// and nothing about the SQL. Here an untranslatable shape throws and fails the test outright,
/// and a mistranslation is caught by moving exactly one half of the aggregate at a time:
/// </para>
/// <list type="bullet">
/// <item>a later-timestamped edit moves <c>Max</c> only (its <c>Count</c> also moves, so this
/// one alone is not decisive) —</item>
/// <item>an <b>earlier</b>-timestamped edit leaves <c>Max</c> where it was and moves
/// <c>Count</c> alone, which is the only thing that can invalidate the cache; a translation
/// that dropped the <c>Count</c> projection would serve the stale verdict and fail here.</item>
/// </list>
/// <para>
/// The watermark itself is private, so the assertions read it through its one observable
/// consequence: whether the probe recomposes. That is deliberate — asserting a copy of the query
/// would pin a copy, not the production one.
/// </para>
/// <para>
/// The table is cleared at the start of every test: the collection shares one database and the
/// watermark is a whole-table aggregate.
/// </para>
/// </remarks>
[Collection(nameof(AdminUiSqlServerCollection))]
public sealed class DeploymentDriftProbeSqlServerTests(AdminUiSqlServerFixture sql)
{
private const string SealedHash = "sealed-hash-aaaa";
/// <summary>A fixed instant so the "earlier edit" case is unambiguously earlier, clock skew aside.</summary>
private static readonly DateTime Anchor = new(2026, 8, 17, 12, 0, 0, DateTimeKind.Utc);
[SqlServerFact]
public async Task An_empty_ConfigEdits_table_yields_a_verdict_and_caches_it()
{
// The no-rows shape is the one GroupBy(_ => 1) makes subtle: SQL Server produces NO group at all,
// so FirstOrDefaultAsync must land on null rather than on a row of (NULL, 0).
await ClearConfigEditsAsync();
var compose = new CountingCompose(SealedHash);
var probe = new DeploymentDriftProbe(sql.Factory, compose.ComposeAsync);
var first = await probe.GetDriftAsync(SealedHash, CancellationToken.None);
var second = await probe.GetDriftAsync(SealedHash, CancellationToken.None);
first.ShouldBe(false);
second.ShouldBe(false);
compose.Count.ShouldBe(1);
}
[SqlServerFact]
public async Task An_unchanged_ConfigEdits_table_reuses_the_cached_verdict()
{
await ClearConfigEditsAsync();
await AddEditsAsync(Anchor, Anchor.AddSeconds(1));
var compose = new CountingCompose(SealedHash);
var probe = new DeploymentDriftProbe(sql.Factory, compose.ComposeAsync);
await probe.GetDriftAsync(SealedHash, CancellationToken.None);
var second = await probe.GetDriftAsync(SealedHash, CancellationToken.None);
// Two reads of the same rows must produce an EQUAL watermark. Not a tautology against SQL:
// EditedAtUtc round-trips through datetime2 and an unstable projection would recompose forever.
second.ShouldBe(false);
compose.Count.ShouldBe(1);
}
[SqlServerFact]
public async Task A_later_edit_moves_the_watermark_and_recomposes()
{
await ClearConfigEditsAsync();
await AddEditsAsync(Anchor);
var compose = new CountingCompose(SealedHash);
var probe = new DeploymentDriftProbe(sql.Factory, compose.ComposeAsync);
(await probe.GetDriftAsync(SealedHash, CancellationToken.None)).ShouldBe(false);
await AddEditsAsync(Anchor.AddHours(1));
compose.Result = "a-different-hash";
(await probe.GetDriftAsync(SealedHash, CancellationToken.None)).ShouldBe(true);
compose.Count.ShouldBe(2);
}
[SqlServerFact]
public async Task An_earlier_edit_moves_only_the_count_and_still_recomposes()
{
// The decisive case for the Count half: MAX(EditedAtUtc) is unchanged, so a translation that lost
// the COUNT(*) projection would report the same watermark and serve the stale (false) verdict.
await ClearConfigEditsAsync();
await AddEditsAsync(Anchor);
var compose = new CountingCompose(SealedHash);
var probe = new DeploymentDriftProbe(sql.Factory, compose.ComposeAsync);
(await probe.GetDriftAsync(SealedHash, CancellationToken.None)).ShouldBe(false);
await AddEditsAsync(Anchor.AddHours(-1));
compose.Result = "a-different-hash";
(await probe.GetDriftAsync(SealedHash, CancellationToken.None)).ShouldBe(true);
compose.Count.ShouldBe(2);
}
[SqlServerFact]
public async Task A_same_timestamp_edit_moves_only_the_count_and_still_recomposes()
{
// Two mutations inside one tick — the case the CLAUDE.md/probe remarks call out explicitly.
await ClearConfigEditsAsync();
await AddEditsAsync(Anchor);
var compose = new CountingCompose(SealedHash);
var probe = new DeploymentDriftProbe(sql.Factory, compose.ComposeAsync);
(await probe.GetDriftAsync(SealedHash, CancellationToken.None)).ShouldBe(false);
await AddEditsAsync(Anchor);
compose.Result = "a-different-hash";
(await probe.GetDriftAsync(SealedHash, CancellationToken.None)).ShouldBe(true);
compose.Count.ShouldBe(2);
}
[SqlServerFact]
public async Task Emptying_the_table_after_a_verdict_recomposes()
{
// Exercises the populated-then-empty transition, i.e. the aggregate crossing back to "no group".
await ClearConfigEditsAsync();
await AddEditsAsync(Anchor, Anchor.AddSeconds(1));
var compose = new CountingCompose(SealedHash);
var probe = new DeploymentDriftProbe(sql.Factory, compose.ComposeAsync);
(await probe.GetDriftAsync(SealedHash, CancellationToken.None)).ShouldBe(false);
await ClearConfigEditsAsync();
compose.Result = "a-different-hash";
(await probe.GetDriftAsync(SealedHash, CancellationToken.None)).ShouldBe(true);
compose.Count.ShouldBe(2);
}
// -------------------------------------------------------------------------------------- Helpers
private async Task ClearConfigEditsAsync()
{
await using var db = await sql.Factory.CreateDbContextAsync();
await db.ConfigEdits.ExecuteDeleteAsync();
}
private async Task AddEditsAsync(params DateTime[] editedAtUtc)
{
await using var db = await sql.Factory.CreateDbContextAsync();
foreach (var at in editedAtUtc)
{
db.ConfigEdits.Add(new ConfigEdit
{
EntityType = "Equipment",
EntityId = Guid.NewGuid(),
FieldsJson = "{}",
EditedBy = "test",
SourceNode = "node-1",
EditedAtUtc = at,
});
}
await db.SaveChangesAsync();
}
/// <summary>Stand-in for the <c>ConfigComposer</c> seam that counts how often it was invoked.</summary>
private sealed class CountingCompose(string result)
{
public string Result { get; set; } = result;
public int Count { get; private set; }
public Task<string> ComposeAsync(OtOpcUaConfigDbContext db)
{
Count++;
return Task.FromResult(Result);
}
}
}
tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/MonacoEditorDisposeTests.cs
+94
View File
@@ -0,0 +1,94 @@
using Bunit;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.JSInterop;
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared;
namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.ScriptAnalysis;
/// <summary>
/// lmxopcua#902: closing a tab with the script editor open ended the circuit, and the editor's
/// <c>DisposeAsync</c> JS call then failed with <see cref="TaskCanceledException"/>, which escaped and was logged
/// at Error as an unhandled circuit exception. A JS-interop failure caused by the circuit already being gone is
/// the expected outcome of disposal: it must not escape and must not log above Debug. A genuine
/// <see cref="JSException"/> from a live page is still surfaced (Warning).
/// </summary>
[Trait("Category", "Unit")]
public sealed class MonacoEditorDisposeTests : BunitContext
{
private readonly CapturingLoggerProvider _logs = new();
public MonacoEditorDisposeTests()
{
Services.AddLogging(b => b.AddProvider(_logs).SetMinimumLevel(LogLevel.Trace));
JSInterop.SetupVoid("MonacoBlazor.createEditor", _ => true).SetVoidResult();
}
public static TheoryData<string> GoneCircuitFailures => new()
{
nameof(TaskCanceledException),
nameof(JSDisconnectedException),
nameof(ObjectDisposedException),
};
private static Exception Make(string name) => name switch
{
nameof(TaskCanceledException) => new TaskCanceledException(),
nameof(JSDisconnectedException) => new JSDisconnectedException("circuit gone"),
nameof(ObjectDisposedException) => new ObjectDisposedException("JSRuntime"),
_ => throw new ArgumentOutOfRangeException(nameof(name)),
};
[Theory]
[MemberData(nameof(GoneCircuitFailures))]
public async Task Dispose_on_a_gone_circuit_is_quiet(string failure)
{
var dispose = JSInterop.SetupVoid("MonacoBlazor.dispose", _ => true);
dispose.SetException(Make(failure));
Render<MonacoEditor>(p => p.Add(x => x.ShowToolbar, false));
// Disposing the render tree is what the circuit does when the tab closes.
await Should.NotThrowAsync(() => DisposeComponentsAsync());
JSInterop.VerifyInvoke("MonacoBlazor.dispose"); // positive control: the failing call really ran
_logs.Entries.Where(e => e.Level > LogLevel.Debug).ShouldBeEmpty();
_logs.Entries.ShouldContain(e => e.Level == LogLevel.Debug && e.Message.Contains(failure));
}
[Fact]
public async Task A_real_js_fault_on_dispose_is_still_logged()
{
JSInterop.SetupVoid("MonacoBlazor.dispose", _ => true).SetException(new JSException("monaco blew up"));
Render<MonacoEditor>(p => p.Add(x => x.ShowToolbar, false));
await Should.NotThrowAsync(() => DisposeComponentsAsync());
_logs.Entries.ShouldContain(e => e.Level == LogLevel.Warning && e.Exception is JSException);
}
private sealed record LogEntry(LogLevel Level, string Message, Exception? Exception);
private sealed class CapturingLoggerProvider : ILoggerProvider
{
public List<LogEntry> Entries { get; } = new();
public ILogger CreateLogger(string categoryName) => new Logger(this);
public void Dispose() { }
private sealed class Logger(CapturingLoggerProvider owner) : ILogger
{
public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;
public bool IsEnabled(LogLevel logLevel) => true;
public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception,
Func<TState, Exception?, string> formatter)
{
lock (owner.Entries) owner.Entries.Add(new LogEntry(logLevel, formatter(state, exception), exception));
}
}
}
}