diff --git a/docs/drivers/OpcUaClient.md b/docs/drivers/OpcUaClient.md index 13cd6db8..d2d7b523 100644 --- a/docs/drivers/OpcUaClient.md +++ b/docs/drivers/OpcUaClient.md @@ -231,6 +231,12 @@ initialise (the AdminUI form drops them on the next save). Pinned by > forgotten when the subscribed set changes (the host rebuilt, recreating the nodes inactive). Index keys are server-stable (`nsu=`), so a namespace reorder across a re-initialise keeps > matching. docker-dev seeds an opc-plc driver (`MAIN-opcua`); the alarm tag is > created only when `@OpcPlcAlarmSourceNodeId` is set in `docker-dev/seed/seed-clusters.sql`. +> +> **An ack that never got a reply fails (review 2026-10-21 O-8).** A transport failure on the `Acknowledge` call +> (closed channel, faulted session) — or a session lost while the ack queued — now throws with its status +> (`ServiceResultException`, e.g. `BadConnectionClosed` / `BadNotConnected`), so the host logs the ack as failed +> instead of applied. A *reply* still returns normally, including per-ack refusals such as +> `BadConditionBranchAlreadyAcked` (logged per ack): a refusal is the server answering. > This driver does **not** implement `IRediscoverable` — there is no > push-driven rediscovery signal from a remote OPC UA server in this driver. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs index a052a286..9a79ac3c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -2496,9 +2496,12 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // Re-read Session inside the critical section: a reconnect completing while we were // blocked on _gate has already swapped in the new session via a re-initialise. // ConditionId parsing is namespace-relative, so it must use the current session's - // namespace table too. - var session = Session; - if (session is null) return; + // namespace table too. A session lost while this ack queued is a failure, not a silent success: the + // host would otherwise log the ack as applied (R06:O-8). + var generation = Volatile.Read(ref _lifecycleGeneration); + var session = Session + ?? throw new ServiceResultException(StatusCodes.BadNotConnected, + "OpcUaClient session was lost before the acknowledgement could be sent."); // OPC UA A&C: call the AcknowledgeableConditionType.Acknowledge method on each // condition node with EventId + Comment arguments. CallAsync accepts a batch — @@ -2543,6 +2546,8 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit requestHeader: null, methodsToCall: callRequests, ct: cancellationToken).ConfigureAwait(false); + // A response — whatever the per-ack statuses say — is the server answering. + RecordDeviceAnswered(generation, Clock.GetUtcNow().UtcDateTime); // Inspect per-ack results — the upstream server can reject individual acks // (BadConditionAlreadyAcked, BadNodeIdUnknown, BadUserAccessDenied) even when @@ -2575,11 +2580,13 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit } catch (Exception ex) { - // Log genuine transport failures rather than swallowing them silently. + // R06:O-8 — a transport failure means NO reply: the acknowledgement was not applied upstream, so the + // caller must see it (with the transport status, e.g. BadConnectionClosed) instead of a success. _logger.LogWarning(ex, "OpcUaClientDriver '{DriverInstanceId}': AcknowledgeAsync failed; " + - "acknowledgements may not have been applied.", + "acknowledgements were not applied.", _driverInstanceId); + throw; } } finally { _gate.Release(); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientAcknowledgeFailureTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientAcknowledgeFailureTests.cs new file mode 100644 index 00000000..63e9b62a --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientAcknowledgeFailureTests.cs @@ -0,0 +1,83 @@ +using Moq; +using Opc.Ua; +using Opc.Ua.Client; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests; + +/// +/// Review 2026-10-21 R06:O-8 — AcknowledgeAsync swallowed every non-cancellation transport failure, so +/// the host logged the Part 9 ack as applied while upstream still showed the condition unacknowledged. A +/// transport failure (no reply) now propagates with its status; a reply — even one carrying per-ack refusals — +/// still returns normally (that is the server answering, logged per ack). +/// +[Trait("Category", "Unit")] +public sealed class OpcUaClientAcknowledgeFailureTests +{ + private const string Condition = "ns=2;s=Pump17"; + + private static Mock NewSession() + { + var mock = new Mock(MockBehavior.Loose); + mock.SetupGet(s => s.MessageContext).Returns(new ServiceMessageContext(SdkTelemetry.Context)); + mock.SetupGet(s => s.NamespaceUris).Returns(new NamespaceTable()); + return mock; + } + + private static AlarmAcknowledgeRequest[] Ack() => [new(Condition, Condition, "operator ack")]; + + /// A transport failure on the Acknowledge call reaches the caller with the transport status. + [Fact] + public async Task A_transport_failure_propagates_with_its_status() + { + using var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions(), "opcua-o8"); + var session = NewSession(); + session.Setup(s => s.CallAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ThrowsAsync(new ServiceResultException(StatusCodes.BadConnectionClosed)); + drv.SetSessionForTest(session.Object); + + var ex = await Should.ThrowAsync(() => + drv.AcknowledgeAsync(Ack(), TestContext.Current.CancellationToken)); + + ex.StatusCode.ShouldBe(StatusCodes.BadConnectionClosed); + } + + /// Positive control: a reply returns normally and the call reached the wire. + [Fact] + public async Task Positive_control_a_reply_returns_normally() + { + using var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions(), "opcua-o8-ok"); + var session = NewSession(); + session.Setup(s => s.CallAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new CallResponse + { + ResponseHeader = new ResponseHeader(), + Results = [new CallMethodResult { StatusCode = StatusCodes.BadConditionBranchAlreadyAcked }], + DiagnosticInfos = [], + }); + drv.SetSessionForTest(session.Object); + + await drv.AcknowledgeAsync(Ack(), TestContext.Current.CancellationToken); + + session.Verify(s => s.CallAsync(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Once); + } + + /// A session lost while the ack queued on the gate fails the ack rather than reporting it applied. + [Fact] + public async Task A_session_lost_while_queued_fails_the_ack() + { + var ct = TestContext.Current.CancellationToken; + using var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions(), "opcua-o8-lost"); + drv.SetSessionForTest(NewSession().Object); + + await drv.Gate.WaitAsync(ct); + var call = drv.AcknowledgeAsync(Ack(), ct); + drv.SetSessionForTest(null); + drv.Gate.Release(); + + await Should.ThrowAsync(() => call); + } +}