From 01253818de677772206a19e39b8ab0936cb55ad1 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 09:52:56 -0400 Subject: [PATCH] fix(runtime): VT script failure/timeout degrades node quality to Bad once per transition (02/S13) --- ...2-03-vt-failure-quality-plan.md.tasks.json | 2 +- .../VirtualTags/VirtualTagActor.cs | 57 ++++++++++++++++--- .../VirtualTags/VirtualTagActorTests.cs | 6 +- 3 files changed, 56 insertions(+), 9 deletions(-) diff --git a/archreview/plans/R2-03-vt-failure-quality-plan.md.tasks.json b/archreview/plans/R2-03-vt-failure-quality-plan.md.tasks.json index 5c69ef1e..d0abbad6 100644 --- a/archreview/plans/R2-03-vt-failure-quality-plan.md.tasks.json +++ b/archreview/plans/R2-03-vt-failure-quality-plan.md.tasks.json @@ -18,7 +18,7 @@ { "id": "R2-03-T3", "subject": "VirtualTagActor failure/throw publishes Bad EvaluationResult once per transition, gated on all dependencyRefs arrived; recovery bypasses value-dedup; update Evaluator_failure_publishes_ScriptLogEntry_warning expectation (turns T1 green)", - "status": "pending", + "status": "completed", "blockedBy": [ "R2-03-T2" ] diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs index 4e1f6b95..c5641b81 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs @@ -13,8 +13,17 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.VirtualTags; /// Wraps a single virtual-tag expression. Receives dependency-tag updates, recomputes the /// expression via an injected , and emits an /// to the parent (the publish actor) whenever the value -/// actually changes. Script failures publish a Warning on the +/// actually changes. Script failures publish a on the /// script-logs DPS topic so operators see the diagnostic in the live tail. +/// +/// +/// 02/S13 — a persistent script failure/timeout also degrades the node's OPC UA quality to +/// (carrying the last-known value), published once per +/// transition (Good→Bad) and gated on every declared dependency having arrived at least once +/// — so a broken script no longer serves its last Good value forever, and a fresh/respawned +/// multi-dep script does not flash Bad during deploy warm-up. Recovery to a Good value clears the +/// Bad state and is force-published even when the value equals the pre-failure value. +/// /// public sealed class VirtualTagActor : ReceiveActor { @@ -41,6 +50,9 @@ public sealed class VirtualTagActor : ReceiveActor private bool _hasLastValue; private object? _lastValue; + // 02/S13: true once a Bad EvaluationResult has been published and not yet cleared by a Good + // recovery. Gates Bad publishes to once-per-transition and force-publishes the recovery value. + private bool _lastPublishedBad; /// Factory method to create Props for a VirtualTagActor. /// Unique identifier for the virtual tag. @@ -122,27 +134,28 @@ public sealed class VirtualTagActor : ReceiveActor catch (Exception ex) { _log.Warning(ex, "VirtualTag {Id}: evaluator threw", _virtualTagId); - OtOpcUaTelemetry.VirtualTagEval.Add(1, new KeyValuePair("outcome", "fail")); - PublishLog("Error", $"evaluator threw: {ex.Message}"); + OnEvaluationFailed($"evaluator threw: {ex.Message}", "Error", msg.TimestampUtc); return; } if (!result.Success) { - OtOpcUaTelemetry.VirtualTagEval.Add(1, new KeyValuePair("outcome", "fail")); - PublishLog("Warning", result.Reason ?? "evaluator failure"); + OnEvaluationFailed(result.Reason ?? "evaluator failure", "Warning", msg.TimestampUtc); return; } // Skip no-change results. Real evaluator returns Ok(value); Null returns NoChange — both - // safe because Null never produces a fresh value. + // safe because Null never produces a fresh value. NoChange is "no fresh value", NOT a recovery, + // so it must stay above the dedup and must NOT clear _lastPublishedBad. if (ReferenceEquals(result, VirtualTagEvalResult.NoChange)) { OtOpcUaTelemetry.VirtualTagEval.Add(1, new KeyValuePair("outcome", "skip")); return; } - if (_hasLastValue && Equals(_lastValue, result.Value)) + // Value dedup — but bypass it while recovering from Bad, so a recovery whose value equals the + // pre-failure value still republishes Good (otherwise the node would stay Bad forever). + if (!_lastPublishedBad && _hasLastValue && Equals(_lastValue, result.Value)) { OtOpcUaTelemetry.VirtualTagEval.Add(1, new KeyValuePair("outcome", "skip")); return; @@ -150,11 +163,41 @@ public sealed class VirtualTagActor : ReceiveActor _hasLastValue = true; _lastValue = result.Value; + _lastPublishedBad = false; OtOpcUaTelemetry.VirtualTagEval.Add(1, new KeyValuePair("outcome", "ok")); var evalResult = new EvaluationResult(_virtualTagId, result.Value, msg.TimestampUtc, CorrelationId.NewId()); Context.Parent.Tell(evalResult); } + /// 02/S13 failure tail shared by the evaluator-threw catch and the !Success branch. + /// Always meters + publishes the per-failure script-log entry (the pre-existing operator + /// diagnostic); additionally publishes a Bad — carrying the + /// last-known value if any — once per Good→Bad transition, and only after every declared dependency + /// has arrived (the inputs-ready gate that suppresses the cold-start/respawn warm-up flash). + /// The failure reason for the script-log entry. + /// The script-log level ("Warning" for a Failure result, "Error" for a throw). + /// The triggering dependency change's timestamp, stamped on the Bad result. + private void OnEvaluationFailed(string reason, string level, DateTime timestampUtc) + { + OtOpcUaTelemetry.VirtualTagEval.Add(1, new KeyValuePair("outcome", "fail")); + PublishLog(level, reason); + + if (_lastPublishedBad || !AllDependenciesArrived()) + { + return; + } + + _lastPublishedBad = true; + Context.Parent.Tell(new EvaluationResult( + _virtualTagId, _hasLastValue ? _lastValue : null, timestampUtc, CorrelationId.NewId(), OpcUaQuality.Bad)); + } + + /// Inputs-ready gate (mirrors the dormant VirtualTagEngine.AreInputsReady): true once + /// every declared dependency ref has been seen at least once. Called once per failure — the ref list + /// is small, so no caching. A vacuously-true empty ref list means a no-dependency script degrades on + /// its first failure. + private bool AllDependenciesArrived() => _dependencyRefs.All(_dependencies.ContainsKey); + private void PublishLog(string level, string message) { var entry = new ScriptLogEntry( diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagActorTests.cs index af001e59..80ca9929 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagActorTests.cs @@ -4,6 +4,7 @@ using Shouldly; using Xunit; using ZB.MOM.WW.OtOpcUa.Commons.Engines; using ZB.MOM.WW.OtOpcUa.Commons.Messages.Logging; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; using ZB.MOM.WW.OtOpcUa.Runtime.VirtualTags; @@ -80,7 +81,10 @@ public sealed class VirtualTagActorTests : RuntimeActorTestBase entry.VirtualTagId.ShouldBe("vt-1"); }, duration: TimeSpan.FromMilliseconds(500)); - parent.ExpectNoMsg(TimeSpan.FromMilliseconds(100)); + // 02/S13: the failure now ALSO degrades the node — with no declared dependencyRefs the + // inputs-ready gate is vacuously satisfied, so a Bad EvaluationResult reaches the parent (this + // replaces the old ExpectNoMsg, which encoded the stale-Good bug). + parent.ExpectMsg().Quality.ShouldBe(OpcUaQuality.Bad); } /// Test evaluator that sums integer dependency values.