From e2b31a9fd219c172f1aa6eb23536497a9e54ca18 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 16 Jun 2026 06:26:00 -0400 Subject: [PATCH] =?UTF-8?q?fix(siteruntime):=20M2.12=20review=20nits=20?= =?UTF-8?q?=E2=80=94=20observe=20logger=20fault=20+=20meaningful=20source?= =?UTF-8?q?=20fallback=20(#25)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace bare task-discard with ContinueWith(OnlyOnFaulted|ExecuteSynchronously) so a faulted ISiteEventLogger is logged and swallowed rather than going to the unobserved-task firehose. Replace the "ScriptRuntimeContext" class-name fallback with the meaningful "InstanceScript:{instanceName}" identifier (matching the site-event-log source convention). Update the method doc-comment to state the best-effort contract explicitly. Pin the new fallback value in the shape-precision test. --- .../Scripts/ScriptRuntimeContext.cs | 42 +++++++++++++++---- .../Scripts/RecursionLimitSiteEventTests.cs | 4 ++ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs index 10604912..86b88e0b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs @@ -250,14 +250,38 @@ public class ScriptRuntimeContext /// /// M2.12 (#25): fire-and-forget emission of a script Error site event /// for a recursion-limit violation. Mirrors the call shape used by - /// ScriptExecutionActor's catch blocks (WP-32 / M1.8). The returned - /// task is intentionally discarded by callers (_ =) so the event log - /// never blocks or faults the throw that follows. A null logger is a no-op. + /// ScriptExecutionActor's catch blocks (WP-32 / M1.8). A fault from + /// the site-event logger is observed-and-dropped (best-effort) via + /// ContinueWith(OnlyOnFaulted) — it never blocks or faults the + /// _logger.LogError + throw path that follows. A null logger is a no-op. /// - private Task EmitRecursionLimitEventAsync(string msg) => - _siteEventLogger?.LogEventAsync( - "script", "Error", _instanceName, _sourceScript ?? "ScriptRuntimeContext", msg) - ?? Task.CompletedTask; + private void EmitRecursionLimitEventAsync(string msg) + { + if (_siteEventLogger == null) + return; + + var source = string.IsNullOrEmpty(_instanceName) + ? "recursion-guard" + : $"InstanceScript:{_instanceName}"; + + var logTask = _siteEventLogger.LogEventAsync("script", "Error", _instanceName, source, msg); + if (!logTask.IsCompleted) + { + logTask.ContinueWith( + t => _logger.LogWarning(t.Exception, + "Site event log write failed for recursion-limit violation on instance '{Instance}'", + _instanceName), + CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + } + else if (logTask.IsFaulted) + { + _logger.LogWarning(logTask.Exception, + "Site event log write failed for recursion-limit violation on instance '{Instance}'", + _instanceName); + } + } /// /// Gets the current value of an attribute from the Instance Actor. @@ -333,7 +357,7 @@ public class ScriptRuntimeContext $"CallScript('{scriptName}') rejected at depth {nextDepth}."; _logger.LogError(msg); // M2.12 (#25): emit to site event log in addition to ILogger; fire-and-forget. - _ = EmitRecursionLimitEventAsync(msg); + EmitRecursionLimitEventAsync(msg); throw new InvalidOperationException(msg); } @@ -498,7 +522,7 @@ public class ScriptRuntimeContext _logger.LogError(msg); // M2.12 (#25): emit to site event log via the parent context's // helper — single emission path, fire-and-forget. - _ = _context.EmitRecursionLimitEventAsync(msg); + _context.EmitRecursionLimitEventAsync(msg); throw new InvalidOperationException(msg); } diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/RecursionLimitSiteEventTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/RecursionLimitSiteEventTests.cs index 4fe09f7f..3843d08d 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/RecursionLimitSiteEventTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/RecursionLimitSiteEventTests.cs @@ -143,6 +143,7 @@ public class RecursionLimitSiteEventTests string? capturedCategory = null; string? capturedSeverity = null; string? capturedInstance = null; + string? capturedSource = null; string? capturedMessage = null; var loggerMock = new Mock(); @@ -157,6 +158,7 @@ public class RecursionLimitSiteEventTests capturedCategory = cat; capturedSeverity = sev; capturedInstance = inst; + capturedSource = src; capturedMessage = msg; }) .Returns(Task.CompletedTask); @@ -169,6 +171,8 @@ public class RecursionLimitSiteEventTests Assert.Equal("script", capturedCategory); Assert.Equal("Error", capturedSeverity); Assert.Equal(InstanceName, capturedInstance); + // Source fallback: no sourceScript wired, so the helper uses "InstanceScript:{instanceName}". + Assert.Equal($"InstanceScript:{InstanceName}", capturedSource); Assert.NotNull(capturedMessage); Assert.Contains("CallScript", capturedMessage); Assert.Contains("2", capturedMessage); // maxCallDepth in the message