From f08038db23920eeba70c28746b67b4023463be92 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 16 Jun 2026 06:20:58 -0400 Subject: [PATCH] =?UTF-8?q?feat(siteruntime):=20M2.12=20(#25)=20=E2=80=94?= =?UTF-8?q?=20emit=20script=20Error=20site=20event=20on=20recursion-limit?= =?UTF-8?q?=20violation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inject ISiteEventLogger into ScriptRuntimeContext (additive optional ctor param, defaulted null, all existing callers source-compatible). Add a single private EmitRecursionLimitEventAsync helper that fires-and-forgets a "script"/Error site event; called at both recursion guard sites (CallScript at ~:332 and ScriptCallHelper.CallShared at ~:499). ScriptExecutionActor threads the already-resolved siteEventLogger singleton into the context; AlarmExecutionActor leaves it null (no siteEventLogger wired there). Existing _logger.LogError + throw behaviour unchanged. Tests: RecursionLimitSiteEventTests — 5 tests covering both CallScript and CallShared (ISiteEventLogger.LogEventAsync called once with category "script", severity "Error"; null logger path does not throw). --- .../Actors/ScriptExecutionActor.cs | 6 +- .../Scripts/ScriptRuntimeContext.cs | 37 +++- .../Scripts/RecursionLimitSiteEventTests.cs | 176 ++++++++++++++++++ 3 files changed, 217 insertions(+), 2 deletions(-) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/RecursionLimitSiteEventTests.cs diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptExecutionActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptExecutionActor.cs index f4c2348e..6cc43423 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptExecutionActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptExecutionActor.cs @@ -217,7 +217,11 @@ public class ScriptExecutionActor : ReceiveActor // and the four cached-call telemetry constructors can stamp // it onto NotificationSubmit.SourceNode and // SiteCallOperational.SourceNode respectively. - sourceNode: sourceNode); + sourceNode: sourceNode, + // M2.12 (#25): thread the singleton site event logger so + // recursion-limit violations at CallScript/CallShared emit a + // script Error site event in addition to ILogger.LogError. + siteEventLogger: siteEventLogger); var globals = new ScriptGlobals { diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs index 4bf48d21..10604912 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs @@ -13,6 +13,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Types; using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit; using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; using AuditEvent = ZB.MOM.WW.Audit.AuditEvent; +using ZB.MOM.WW.ScadaBridge.SiteEventLogging; using ZB.MOM.WW.ScadaBridge.StoreAndForward; namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; @@ -94,6 +95,13 @@ public class ScriptRuntimeContext /// private readonly string? _sourceScript; + /// + /// M2.12 (#25): site event logger for recording recursion-limit violations + /// to the local SQLite event log. Optional — when null the emission is + /// skipped; the existing _logger.LogError + throw path is unchanged. + /// + private readonly ISiteEventLogger? _siteEventLogger; + /// /// Audit Log #23: best-effort emitter for boundary-crossing actions executed /// by the script. Optional — when null the helpers degrade to a no-op audit @@ -179,6 +187,13 @@ public class ScriptRuntimeContext /// ; this only records the spawner. /// /// Optional cluster node identifier (node-a/node-b) for audit trail stamping. + /// + /// M2.12 (#25): optional site event logger. When supplied, recursion-limit + /// violations at CallScript and CallShared emit a + /// script Error event in addition to the existing + /// ILogger.LogError + throw. When null the existing behaviour is + /// unchanged; all existing callers and tests remain source-compatible. + /// public ScriptRuntimeContext( IActorRef instanceActor, IActorRef self, @@ -199,7 +214,8 @@ public class ScriptRuntimeContext ICachedCallTelemetryForwarder? cachedForwarder = null, Guid? executionId = null, Guid? parentExecutionId = null, - string? sourceNode = null) + string? sourceNode = null, + ISiteEventLogger? siteEventLogger = null) { _instanceActor = instanceActor; _self = self; @@ -227,8 +243,22 @@ public class ScriptRuntimeContext // Audit Log #23 (ParentExecutionId): stored verbatim — no `?? NewGuid()` // fallback. A non-routed run legitimately has no parent and stays null. _parentExecutionId = parentExecutionId; + // M2.12 (#25): optional — null when not wired (tests / AlarmExecutionActor). + _siteEventLogger = siteEventLogger; } + /// + /// 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. + /// + private Task EmitRecursionLimitEventAsync(string msg) => + _siteEventLogger?.LogEventAsync( + "script", "Error", _instanceName, _sourceScript ?? "ScriptRuntimeContext", msg) + ?? Task.CompletedTask; + /// /// Gets the current value of an attribute from the Instance Actor. /// Uses Ask pattern (system boundary between script execution and instance state). @@ -302,6 +332,8 @@ public class ScriptRuntimeContext var msg = $"Script call depth exceeded maximum of {_maxCallDepth}. " + $"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); throw new InvalidOperationException(msg); } @@ -464,6 +496,9 @@ public class ScriptRuntimeContext var msg = $"Script call depth exceeded maximum of {_maxCallDepth}. " + $"CallShared('{scriptName}') rejected at depth {nextDepth}."; _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); 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 new file mode 100644 index 00000000..4fe09f7f --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/RecursionLimitSiteEventTests.cs @@ -0,0 +1,176 @@ +using Akka.Actor; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using ZB.MOM.WW.ScadaBridge.SiteEventLogging; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Scripts; + +/// +/// M2.12 (#25): recursion-limit violations at CallScript and +/// CallShared must emit a script Error site event via +/// in addition to the existing +/// ILogger.LogError + throw. +/// +public class RecursionLimitSiteEventTests +{ + private const string InstanceName = "test-instance"; + private const string SourceScript = "ScriptActor:OnTick"; + + /// + /// Build a wired at the recursion + /// limit (currentCallDepth == maxCallDepth) so the very next + /// CallScript or CallShared call trips the guard. + /// + private static ScriptRuntimeContext CreateContextAtLimit( + ISiteEventLogger siteEventLogger, + int maxCallDepth = 3) + { + var compilationService = new ScriptCompilationService( + NullLogger.Instance); + var sharedScriptLibrary = new SharedScriptLibrary( + compilationService, NullLogger.Instance); + + return new ScriptRuntimeContext( + ActorRefs.Nobody, + ActorRefs.Nobody, + sharedScriptLibrary, + currentCallDepth: maxCallDepth, // already AT the limit + maxCallDepth: maxCallDepth, + askTimeout: TimeSpan.FromSeconds(5), + instanceName: InstanceName, + logger: NullLogger.Instance, + siteEventLogger: siteEventLogger); + } + + // ------------------------------------------------------------------------- + // CallScript + // ------------------------------------------------------------------------- + + [Fact] + public async Task CallScript_RecursionLimitExceeded_EmitsScriptErrorSiteEvent() + { + var loggerMock = new Mock(); + loggerMock + .Setup(l => l.LogEventAsync( + It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + + var context = CreateContextAtLimit(loggerMock.Object); + + // The call must throw — recursion limit hit + await Assert.ThrowsAsync( + () => context.CallScript("AnyScript")); + + // Give the fire-and-forget task a moment to complete + await Task.Yield(); + + loggerMock.Verify(l => l.LogEventAsync( + "script", + "Error", + InstanceName, + It.IsAny(), // source (may be null when sourceScript not set) + It.Is(m => m.Contains("CallScript") && m.Contains("depth")), + null), + Times.Once); + } + + [Fact] + public async Task CallScript_RecursionLimitExceeded_NullLogger_DoesNotThrow() + { + // Null siteEventLogger — existing behaviour must be fully unchanged: + // LogError + throw, no NullReferenceException from the emission path. + var context = CreateContextAtLimit(siteEventLogger: null!); + + var ex = await Assert.ThrowsAsync( + () => context.CallScript("AnyScript")); + + Assert.Contains("depth", ex.Message); + } + + // ------------------------------------------------------------------------- + // CallShared (via Scripts.CallShared) + // ------------------------------------------------------------------------- + + [Fact] + public async Task CallShared_RecursionLimitExceeded_EmitsScriptErrorSiteEvent() + { + var loggerMock = new Mock(); + loggerMock + .Setup(l => l.LogEventAsync( + It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + + var context = CreateContextAtLimit(loggerMock.Object); + + await Assert.ThrowsAsync( + () => context.Scripts.CallShared("AnySharedScript")); + + await Task.Yield(); + + loggerMock.Verify(l => l.LogEventAsync( + "script", + "Error", + InstanceName, + It.IsAny(), + It.Is(m => m.Contains("CallShared") && m.Contains("depth")), + null), + Times.Once); + } + + [Fact] + public async Task CallShared_RecursionLimitExceeded_NullLogger_DoesNotThrow() + { + var context = CreateContextAtLimit(siteEventLogger: null!); + + var ex = await Assert.ThrowsAsync( + () => context.Scripts.CallShared("AnySharedScript")); + + Assert.Contains("depth", ex.Message); + } + + // ------------------------------------------------------------------------- + // Verify the event shape precisely (category, severity, message content) + // ------------------------------------------------------------------------- + + [Fact] + public async Task CallScript_RecursionLimitExceeded_EventHasCorrectCategoryAndSeverity() + { + string? capturedCategory = null; + string? capturedSeverity = null; + string? capturedInstance = null; + string? capturedMessage = null; + + var loggerMock = new Mock(); + loggerMock + .Setup(l => l.LogEventAsync( + It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny())) + .Callback( + (cat, sev, inst, src, msg, det) => + { + capturedCategory = cat; + capturedSeverity = sev; + capturedInstance = inst; + capturedMessage = msg; + }) + .Returns(Task.CompletedTask); + + var context = CreateContextAtLimit(loggerMock.Object, maxCallDepth: 2); + await Assert.ThrowsAsync( + () => context.CallScript("DeepScript")); + await Task.Yield(); + + Assert.Equal("script", capturedCategory); + Assert.Equal("Error", capturedSeverity); + Assert.Equal(InstanceName, capturedInstance); + Assert.NotNull(capturedMessage); + Assert.Contains("CallScript", capturedMessage); + Assert.Contains("2", capturedMessage); // maxCallDepth in the message + } +}