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

fix(runtime): reject empty AddComment instead of silently swallowing it

Browse Source 
Validate AddComment up-front (IsNullOrWhiteSpace guard + Warning log) so
a blank-comment command is cleanly rejected before reaching the engine
rather than faulting inside ApplyAddComment and being silently swallowed
by the outer catch.  Mirrors the existing TimedShelve missing-UnshelveAtUtc
pattern.

Also fix two stale inline comments: the "async void crash" note on
TimedShelve now correctly says "fault escaping async Task → supervision
restart", and the ownership-filter now documents the benign race with a
concurrent LoadAsync clearing the loaded set.

Tests: AlarmCommand_add_comment_empty_text_is_rejected_not_driven (Theory
— empty string + whitespace) and AlarmCommand_add_comment_nonempty_drives_engine
(positive path, asserts CommentAdded transition on alerts topic).
This commit is contained in:
Joseph Doherty
2026-06-11 06:32:53 -04:00
parent 004558c241
commit 1d7e2a0f8b
2 changed files with 61 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ScriptedAlarms/ScriptedAlarmHostActor.cs
+9 -5
View File
@@ -310,6 +310,8 @@ public sealed class ScriptedAlarmHostActor : ReceiveActor
{
// Ownership filter FIRST: ignore commands for alarms this engine doesn't own. The topic is a
// cluster-wide broadcast, so the same command lands on every host — only the owner acts.
// Note: a concurrent LoadAsync can momentarily clear the loaded set, so a racing command may
// surface a spurious ArgumentException from the engine below — the outer catch absorbs it (benign).
if (!_engine.LoadedAlarmIds.Contains(cmd.AlarmId))
{
_log.Debug("ScriptedAlarmHost: ignoring AlarmCommand {Op} for unowned alarm {AlarmId}",
@@ -333,7 +335,7 @@ public sealed class ScriptedAlarmHostActor : ReceiveActor
case "TimedShelve":
// A timed shelve needs the absolute unshelve instant. T18 derives it from the OPC UA
// Duration (UtcNow + shelvingTime); a command missing it is malformed — log + reject
// rather than throw (a throw out of this async void would crash the actor).
// rather than throw (a fault escaping the async Task would trigger a supervision restart).
if (cmd.UnshelveAtUtc is not { } unshelveAt)
{
_log.Warning("ScriptedAlarmHost: rejecting TimedShelve for {AlarmId} — missing UnshelveAtUtc",
@@ -352,10 +354,12 @@ public sealed class ScriptedAlarmHostActor : ReceiveActor
await _engine.DisableAsync(cmd.AlarmId, cmd.User, CancellationToken.None);
break;
case "AddComment":
// AddComment's text is required by the engine (ApplyAddComment takes a non-null text);
// coalesce a null comment to empty so a comment-less AddComment is still a valid no-op
// rather than an NRE.
await _engine.AddCommentAsync(cmd.AlarmId, cmd.User, cmd.Comment ?? string.Empty, CancellationToken.None);
if (string.IsNullOrWhiteSpace(cmd.Comment))
{
_log.Warning("ScriptedAlarmHost: rejecting AddComment for {AlarmId} — empty comment text", cmd.AlarmId);
return;
}
await _engine.AddCommentAsync(cmd.AlarmId, cmd.User, cmd.Comment, CancellationToken.None);
break;
default:
_log.Warning("ScriptedAlarmHost: ignoring AlarmCommand with unknown operation {Op} for {AlarmId}",