diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 74104d3d6..f6af07d1a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -1651,7 +1651,19 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 } // Re-checked: a stall may have outlived the condition (the liveness check above ran before it). - if (IsLiveCondition(condition)) DeliverToNotifierItems(monitored, e); + if (IsLiveCondition(condition)) + { + // #855 review 3: one hop's failure must not cost the hops after it (before #855 this hop was a channel + // write, which cannot throw; the SDK isolated failures per item). + try + { + DeliverToNotifierItems(monitored, e); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "OtOpcUaNodeManager: failed to deliver a condition event at notifier {NodeId}", node.NodeId); + } + } } else { @@ -5815,7 +5827,18 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 // through its OLD equipment folder (#808). if (!IsLiveCondition(node)) return; - var scopes = AclEnforced ? ConditionVisibilityScopes(node) : null; + List? scopes; + try + { + scopes = AclEnforced ? ConditionVisibilityScopes(node) : null; + } + catch (Exception ex) + { + // #855 review 3: withheld from the Server-object items (fail closed), but the walk goes on to the folders. + _logger.LogWarning(ex, "OtOpcUaNodeManager: failed to decide a condition event for Server-object items ({NodeId})", node.NodeId); + return; + } + var reportingSession = (context as ISessionSystemContext)?.SessionId; var deferred = e is ConditionEventSnapshot { Deferred: true } d ? d : null; @@ -6595,6 +6618,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 foreach (var (e, _, scopes) in replay) { + // #855 review 3: never replay a condition a deploy removed or replaced since the walk collected it. + if (ConditionOf(e) is AlarmConditionState walked && !IsLiveCondition(walked)) continue; if (scopes is not null && !IsConditionVisibleTo(item, scopes)) continue; if (ServiceResult.IsBad(ValidateEventRolePermissions(item, e))) continue; item.QueueEvent(e); @@ -6878,6 +6903,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 DirectRefreshProbe?.Invoke(false, Monitor.IsEntered(Lock)); foreach (var e in events) { + // #855 review 3: never replay a condition a deploy removed or replaced since the walk collected it. + if (ConditionOf(e) is AlarmConditionState walked && !IsLiveCondition(walked)) continue; if (!IsDirectRefreshEventAllowed(item, e, enforced)) continue; item.QueueEvent(e); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderConditionRefreshWireTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderConditionRefreshWireTests.cs index 59287d809..b6deba5e2 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderConditionRefreshWireTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderConditionRefreshWireTests.cs @@ -489,6 +489,49 @@ public sealed class FolderConditionRefreshWireTests after.ShouldBeEmpty("and none of its deferred events follows"); } + /// + /// #855 review 3: the plain refresh path (nothing deferred — the only path production takes) collects live + /// conditions off Lock and queues them afterwards; a condition removed in between must not be replayed. + /// + /// Refresh a Server-object item instead of a folder item. + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task A_condition_removed_during_a_plain_refresh_is_not_replayed(bool serverObject) + { + var ct = TestContext.Current.CancellationToken; + await using var rig = await ServerEventRig.StartAsync("Refresh855RemovePlain", encrypted: false, ct); + rig.KeyRoles = []; + rig.MaterialiseNative(); + rig.Enforce(false, [ServerEventRig.Key()], KeyEquipmentGrant(EqA)); + + using var session = await rig.ConnectAsync(anonymous: false, ct); + var events = new RecordedEvents(); + var subscription = await RecordedEvents.SubscribeAsync(session, events, ct); + var item = RecordedEvents.AddEventItem(subscription, serverObject ? ObjectIds.Server : ServerEventRig.UnsNodeId(session, EqA)); + await subscription.ApplyChangesAsync(ct); + + rig.Project(native: true, Active("seed")); + await RecordedEvents.WaitUntilAsync(() => events.Has(item, "seed")); + + var removed = 0; + void Remove(bool cut, bool _) + { + if (!cut && Interlocked.Exchange(ref removed, 1) == 0) + rig.Nm.RemoveAlarmConditionNode(ServerEventRig.RawAlarmPath, AddressSpaceRealm.Raw).ShouldBeTrue(); + } + + if (serverObject) rig.Nm.ServerObjectRefreshProbe = Remove; + else rig.Nm.DirectRefreshProbe = Remove; + await session.CallAsync(ObjectTypeIds.ConditionType, MethodIds.ConditionType_ConditionRefresh, ct, subscription.Id); + await RecordedEvents.WaitUntilAsync(() => events.HasType(item, ObjectTypeIds.RefreshEndEventType)); + rig.Nm.ServerObjectRefreshProbe = null; + rig.Nm.DirectRefreshProbe = null; + + Volatile.Read(ref removed).ShouldBe(1, "the plain path ran"); + Window(events.Of(item)).Replayed.ShouldBeEmpty("the condition was removed before the replay was queued"); + } + /// /// #855 review: a delivery that fails inline (here: a hop that throws) is over, not suspended — it must not be booked /// as a deferral episode (which moved the episode a concurrent plain refresh checks, and cleared cuts under the