diff --git a/archreview/plans/R2-04-failure-visibility-plan.md.tasks.json b/archreview/plans/R2-04-failure-visibility-plan.md.tasks.json index f0274d82..3e79ca98 100644 --- a/archreview/plans/R2-04-failure-visibility-plan.md.tasks.json +++ b/archreview/plans/R2-04-failure-visibility-plan.md.tasks.json @@ -3,7 +3,7 @@ "lastUpdated": "2026-07-12", "tasks": [ { "id": "T1", "subject": "Extend AddressSpaceApplyOutcome with RebuildFailed/FailedNodes + SafeRebuild returns bool (01/S-1)", "status": "completed", "blockedBy": [] }, - { "id": "T2", "subject": "Safe* helpers return bool; Apply tallies removal-pass failures into FailedNodes", "status": "pending", "blockedBy": ["T1"] }, + { "id": "T2", "subject": "Safe* helpers return bool; Apply tallies removal-pass failures into FailedNodes", "status": "completed", "blockedBy": ["T1"] }, { "id": "T3", "subject": "Materialise* passes return swallowed-failure counts (void -> int, five methods)", "status": "pending", "blockedBy": ["T2"] }, { "id": "T4", "subject": "OpcUaApplyFailed counter + OpcUaPublishActor Error/meter surfacing of degraded applies", "status": "pending", "blockedBy": ["T3"] }, { "id": "T5", "subject": "Galaxy internal IMxWriteOps seam under GatewayGalaxyDataWriter (sealed SDK session untestable) (06/S-1)", "status": "pending", "blockedBy": [] }, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs index 763ec95e..f14f480f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs @@ -82,14 +82,17 @@ public sealed class AddressSpaceApplier var ts = DateTime.UtcNow; var removedCount = 0; + // Swallowed removal-pass condition-write failures are tallied here (not lost to a per-node Warning) + // so a degraded removal is operator-visible via AddressSpaceApplyOutcome.FailedNodes (archreview 01/S-1). + var failedNodes = 0; foreach (var eq in plan.RemovedEquipment) { - SafeWriteAlarmCondition(eq.EquipmentId, RemovedConditionState, ts); + if (!SafeWriteAlarmCondition(eq.EquipmentId, RemovedConditionState, ts)) failedNodes++; removedCount++; } foreach (var alarm in plan.RemovedAlarms) { - SafeWriteAlarmCondition(alarm.ScriptedAlarmId, RemovedConditionState, ts); + if (!SafeWriteAlarmCondition(alarm.ScriptedAlarmId, RemovedConditionState, ts)) failedNodes++; removedCount++; } // Removed equipment tags / VirtualTags are plain variable nodes (no Part 9 condition to write @@ -213,7 +216,7 @@ public sealed class AddressSpaceApplier // currently-historized set. Same non-blocking + throw-safe discipline as the provisioning hook. FeedHistorizedRefs(plan); - return new AddressSpaceApplyOutcome(removedCount, addedCount, changedCount, rebuilt, rebuildFailed); + return new AddressSpaceApplyOutcome(removedCount, addedCount, changedCount, rebuilt, rebuildFailed, failedNodes); } /// @@ -617,16 +620,24 @@ public sealed class AddressSpaceApplier .Select(a => a.EquipmentId).Distinct(StringComparer.Ordinal).Count()); } - private void SafeEnsureFolder(string nodeId, string? parentNodeId, string displayName) + /// Ensure a folder node, swallowing (and Warning-logging) any sink fault. Returns true + /// on success, false when the sink threw — callers tally the false into their pass's failed-node + /// count so a swallowed materialise failure is operator-visible (archreview 01/S-1). + /// true when the folder was ensured; false when the sink threw. + private bool SafeEnsureFolder(string nodeId, string? parentNodeId, string displayName) { - try { _sink.EnsureFolder(nodeId, parentNodeId, displayName); } - catch (Exception ex) { _logger.LogWarning(ex, "AddressSpaceApplier: EnsureFolder threw for {Node}", nodeId); } + try { _sink.EnsureFolder(nodeId, parentNodeId, displayName); return true; } + catch (Exception ex) { _logger.LogWarning(ex, "AddressSpaceApplier: EnsureFolder threw for {Node}", nodeId); return false; } } - private void SafeEnsureVariable(string nodeId, string? parentNodeId, string displayName, string dataType, bool writable, string? historianTagname = null, bool isArray = false, uint? arrayLength = null) + /// Ensure a variable node, swallowing (and Warning-logging) any sink fault. Returns true + /// on success, false when the sink threw — callers tally the false into their pass's failed-node + /// count (archreview 01/S-1). + /// true when the variable was ensured; false when the sink threw. + private bool SafeEnsureVariable(string nodeId, string? parentNodeId, string displayName, string dataType, bool writable, string? historianTagname = null, bool isArray = false, uint? arrayLength = null) { - try { _sink.EnsureVariable(nodeId, parentNodeId, displayName, dataType, writable, historianTagname, isArray, arrayLength); } - catch (Exception ex) { _logger.LogWarning(ex, "AddressSpaceApplier: EnsureVariable threw for {Node}", nodeId); } + try { _sink.EnsureVariable(nodeId, parentNodeId, displayName, dataType, writable, historianTagname, isArray, arrayLength); return true; } + catch (Exception ex) { _logger.LogWarning(ex, "AddressSpaceApplier: EnsureVariable threw for {Node}", nodeId); return false; } } // A VirtualTag's materialised OPC UA node (MaterialiseEquipmentVirtualTags) is derived ONLY from @@ -682,16 +693,24 @@ public sealed class AddressSpaceApplier Severity: 0, Message: string.Empty); - private void SafeWriteAlarmCondition(string nodeId, AlarmConditionSnapshot state, DateTime ts) + /// Write an alarm-condition snapshot, swallowing (and Warning-logging) any sink fault. + /// Returns true on success, false when the sink threw — the removal pass tallies the + /// false into (archreview 01/S-1). + /// true when the write landed; false when the sink threw. + private bool SafeWriteAlarmCondition(string nodeId, AlarmConditionSnapshot state, DateTime ts) { - try { _sink.WriteAlarmCondition(nodeId, state, ts); } - catch (Exception ex) { _logger.LogWarning(ex, "AddressSpaceApplier: WriteAlarmCondition threw for {Node}", nodeId); } + try { _sink.WriteAlarmCondition(nodeId, state, ts); return true; } + catch (Exception ex) { _logger.LogWarning(ex, "AddressSpaceApplier: WriteAlarmCondition threw for {Node}", nodeId); return false; } } - private void SafeMaterialiseAlarmCondition(string alarmNodeId, string equipmentNodeId, string displayName, string alarmType, int severity, bool isNative) + /// Materialise an alarm-condition node, swallowing (and Warning-logging) any sink fault. + /// Returns true on success, false when the sink threw — callers tally the false into + /// their pass's failed-node count (archreview 01/S-1). + /// true when the condition was materialised; false when the sink threw. + private bool SafeMaterialiseAlarmCondition(string alarmNodeId, string equipmentNodeId, string displayName, string alarmType, int severity, bool isNative) { - try { _sink.MaterialiseAlarmCondition(alarmNodeId, equipmentNodeId, displayName, alarmType, severity, isNative); } - catch (Exception ex) { _logger.LogWarning(ex, "AddressSpaceApplier: MaterialiseAlarmCondition threw for {Node}", alarmNodeId); } + try { _sink.MaterialiseAlarmCondition(alarmNodeId, equipmentNodeId, displayName, alarmType, severity, isNative); return true; } + catch (Exception ex) { _logger.LogWarning(ex, "AddressSpaceApplier: MaterialiseAlarmCondition threw for {Node}", alarmNodeId); return false; } } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierFailureSurfaceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierFailureSurfaceTests.cs index d46cc7fb..7b340bd9 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierFailureSurfaceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierFailureSurfaceTests.cs @@ -43,6 +43,21 @@ public sealed class AddressSpaceApplierFailureSurfaceTests outcome.FailedNodes.ShouldBe(0); } + // ---------------- T2: FailedNodes (removal-pass tally) ---------------- + + /// A removal-pass alarm-condition write that throws is counted into FailedNodes (swallowed today). + [Fact] + public void Apply_WhenRemovalConditionWriteThrows_CountsFailedNodes() + { + var sink = new ConfigurableThrowingSink { ThrowOnAlarmWrite = true }; + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var outcome = applier.Apply(EquipmentRemovalPlan("eq-1")); + + outcome.RemovedNodes.ShouldBe(1); + outcome.FailedNodes.ShouldBe(1); + } + // ---------------- fixtures ---------------- private static AddressSpacePlan AddedEquipmentPlan(string id) => new( @@ -56,6 +71,17 @@ public sealed class AddressSpaceApplierFailureSurfaceTests RemovedAlarms: Array.Empty(), ChangedAlarms: Array.Empty()); + private static AddressSpacePlan EquipmentRemovalPlan(params string[] ids) => new( + AddedEquipment: Array.Empty(), + RemovedEquipment: ids.Select(id => new EquipmentNode(id, id, "line-1")).ToArray(), + ChangedEquipment: Array.Empty(), + AddedDrivers: Array.Empty(), + RemovedDrivers: Array.Empty(), + ChangedDrivers: Array.Empty(), + AddedAlarms: Array.Empty(), + RemovedAlarms: Array.Empty(), + ChangedAlarms: Array.Empty()); + /// An whose every sink call can be independently made to /// throw, so tests can drive each Safe* helper's catch branch. Non-throwing calls are no-ops. private sealed class ConfigurableThrowingSink : IOpcUaAddressSpaceSink