From 69e1d320ac50869a7310bdb13ba0d90e30c43cc0 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 24 Apr 2026 17:43:48 -0400 Subject: [PATCH] =?UTF-8?q?Cold-start=20guard=20for=20script=20engines=20?= =?UTF-8?q?=E2=80=94=20skip=20evaluation=20with=20empty=20upstream?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both VirtualTagEngine and ScriptedAlarmEngine share a pattern: the BuildReadCache helper iterates the script's declared input set, reading from _valueCache with a fallback to _upstream.ReadTag. When an upstream tag hasn't yet delivered its first subscription push, ReadTag returns a DataValueSnapshot with a null Value and BadNotConnected quality. User scripts then cast `(double)ctx.GetTag(path).Value` unconditionally and throw NullReferenceException — once per evaluation tick until the cache fills, spamming the log with identical stack traces. The existing catch block recovered (kept the prior state) but didn't silence the churn. Add AreInputsReady(cache) to both engines: return true only when every entry has a non-null Value and a non-Bad StatusCode (Good + Uncertain are both considered ready). Skip script evaluation when the check returns false — the engine holds the prior state (alarm) or the prior snapshot (virtual tag) until upstream delivers. Eliminates the cold- start NRE spam at root without changing the script-engine contract. Also: fix $changeLines.Count in test-galaxy.ps1 — PowerShell's Set-StrictMode -Version 3.0 errors on .Count when Where-Object returns 0 or 1 items. Wrap in `@(...)` to force an array; same pattern the sibling _common.ps1 already uses in Write-Summary for the same reason. Task #112 — the Galaxy live E2E now passes 3/7 stages (probe + source read + reverse-bridge-ACL). The remaining 4 stages (virtual-tag, subscribe-sees-change, alarm-fires, history-read) are deployment- specific: MoveInBatchID is idle in this Galaxy + its AccessLevel blocks writes + it's not historized. Cold-start behaviour is now correct, so once the seed points at a live attribute those stages should light up. Tests: 36/36 VirtualTags.Tests + 47/47 ScriptedAlarms.Tests green. Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/e2e/test-galaxy.ps1 | 6 ++-- .../ScriptedAlarmEngine.cs | 30 +++++++++++++++++++ .../VirtualTagEngine.cs | 25 ++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/scripts/e2e/test-galaxy.ps1 b/scripts/e2e/test-galaxy.ps1 index b158856..f13d58e 100644 --- a/scripts/e2e/test-galaxy.ps1 +++ b/scripts/e2e/test-galaxy.ps1 @@ -193,8 +193,10 @@ $subOut = (Get-Content $stdout.FullName -Raw) + (Get-Content $stderr.FullName -R Remove-Item $stdout.FullName, $stderr.FullName -ErrorAction SilentlyContinue # Any `=` followed by `(Good)` line after the initial subscribe-confirmation -# indicates at least one data-change tick arrived. -$changeLines = ($subOut -split "`n") | Where-Object { $_ -match "=\s+.*\(Good\)" } +# indicates at least one data-change tick arrived. The `@(...)` forces an array +# so `.Count` works on the 0-match + single-match cases that Set-StrictMode +# -Version 3.0 otherwise flags as `property 'Count' cannot be found`. +$changeLines = @(($subOut -split "`n") | Where-Object { $_ -match "=\s+.*\(Good\)" }) if ($changeLines.Count -gt 0) { Write-Pass "$($changeLines.Count) data-change events observed" $results += @{ Passed = $true } diff --git a/src/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs b/src/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs index 13e0d06..50bcea7 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs @@ -269,6 +269,15 @@ public sealed class ScriptedAlarmEngine : IDisposable AlarmState state, AlarmConditionState seed, DateTime nowUtc, CancellationToken ct) { var inputs = BuildReadCache(state.Inputs); + + // Cold-start guard — skip the predicate when any referenced upstream tag has no + // cached value yet (the upstream subscription hasn't delivered its first push). + // Without this, predicates that cast `(double)ctx.GetTag(path).Value` throw NRE on + // every tick until the cache fills, spamming the log with identical stack traces. + // Bad quality is treated the same: the input isn't available at the predicate's + // expected type, so the only defensible move is to hold the prior condition state. + if (!AreInputsReady(inputs)) return seed; + var context = new AlarmPredicateContext(inputs, state.Logger, _clock); bool predicateTrue; @@ -305,6 +314,27 @@ public sealed class ScriptedAlarmEngine : IDisposable return d; } + /// + /// Returns true when every entry in has a non-null value + /// and a Good-quality . A false here lets + /// callers short-circuit script evaluation — predicates that unconditionally cast + /// ctx.GetTag(path).Value to a numeric type would otherwise throw + /// until the upstream subscription delivers + /// its first push. + /// + private static bool AreInputsReady(IReadOnlyDictionary cache) + { + foreach (var kv in cache) + { + if (kv.Value is null || kv.Value.Value is null) return false; + // OPC UA Part 4 StatusCode: bit 31 = severity 10 (Bad). Treat Good + Uncertain + // as "ready"; Uncertain carries a value the script can still inspect + make a + // qualified decision from. Only outright Bad is skipped. + if ((kv.Value.StatusCode & 0x80000000u) != 0) return false; + } + return true; + } + private void EmitEvent(AlarmState state, AlarmConditionState condition, EmissionKind kind) { // Suppressed kind means shelving ate the emission — we don't fire for subscribers diff --git a/src/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs b/src/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs index 3ff2b9d..dac6599 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/VirtualTagEngine.cs @@ -228,6 +228,14 @@ public sealed class VirtualTagEngine : IDisposable try { var ctxCache = BuildReadCache(state.Reads); + + // Cold-start guard — hold the prior value when any upstream input is still + // unset or Bad-quality. Evaluating with nulls would throw inside the script + // (scripts cast ctx.GetTag(path).Value directly) and produce a persistent + // BadInternalError result until the upstream cache fills. Keeping the prior + // snapshot is more honest: the virtual tag simply hasn't been computed yet. + if (!AreInputsReady(ctxCache)) return; + var context = new VirtualTagContext( ctxCache, (p, v) => OnScriptSetVirtualTag(p, v), @@ -278,6 +286,23 @@ public sealed class VirtualTagEngine : IDisposable return map; } + /// + /// Returns true when every entry in has a non-null value + /// and a Good/Uncertain-quality . Scripts + /// cast ctx.GetTag(path).Value unconditionally; short-circuiting before the + /// script runs keeps a not-yet-cached upstream from faulting the virtual tag with + /// BadInternalError. + /// + private static bool AreInputsReady(IReadOnlyDictionary cache) + { + foreach (var kv in cache) + { + if (kv.Value is null || kv.Value.Value is null) return false; + if ((kv.Value.StatusCode & 0x80000000u) != 0) return false; + } + return true; + } + private void OnScriptSetVirtualTag(string path, object? value) { if (!_tags.ContainsKey(path))