diff --git a/docs/drivers/OpcUaClient.md b/docs/drivers/OpcUaClient.md index 3820ec0b..97a82664 100644 --- a/docs/drivers/OpcUaClient.md +++ b/docs/drivers/OpcUaClient.md @@ -124,6 +124,21 @@ DurationGood/Bad, PercentGood/Bad, WorstQuality, NumberOfTransitions, Annotation population StandardDeviation/Variance — each mapped to its OPC UA Part 13 standard aggregate NodeId; #285), and at-time reads are supported; each returned `DataValue` keeps its upstream `StatusCode` and timestamps verbatim. +**The per-node `HistoryReadResult.StatusCode` is honoured (#649).** The driver never reads a Bad node status as +empty data: `BadAggregateNotSupported` on a Processed read throws `HistoryAggregateNotSupportedException`, and any +other Bad status (`BadUserAccessDenied`, `BadNodeIdUnknown`, `BadHistoryOperationUnsupported`, ...) throws +`HistoryReadStatusException` carrying the code (the constructor rejects non-Bad codes). `Good`, `GoodNoData`, +`GoodMoreData` (continuation point) and `Uncertain` are unchanged, and a node-level `BadNoData` is still an empty +result. The status is matched on its code bits, so an info/overflow sub-status does not dodge the match. + +⚠️ **This is currently latent.** The OtOpcUa server does not dispatch HistoryRead to a driver's `IHistoryProvider` +(it serves only the server-wide `IHistorianDataSource`, see `docs/Historian.md`), so no OPC UA client of OtOpcUa +sees this driver's history results or its refusals today. The server's HistoryRead arms (`ServeNodeAsync`, +`ServeRawPagedAsync`) do map a `HistoryReadStatusException` thrown by an `IHistorianDataSource` to that node's +status verbatim, and the driver/wire tests exercise that path; if driver history is ever routed to the server, +refusals will reach clients correctly instead of as empty Good data. A routed deterministic refusal would also +need excluding from the HistoryRead pipeline's retry/breaker handling. + Event-history (`ReadEventsAsync`) is left at the interface default — the interface doesn't yet carry the EventFilter surface needed to forward it. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IHistoryProvider.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IHistoryProvider.cs index 5b8be190..310ac626 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IHistoryProvider.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IHistoryProvider.cs @@ -185,6 +185,31 @@ public sealed class HistoryAggregateNotSupportedException : NotSupportedExceptio => Aggregate = aggregate; } +/// +/// Thrown by a history backend that is a passthrough to another OPC UA server when that server answered +/// a node with a Bad per-node HistoryReadResult.StatusCode other than +/// BadAggregateNotSupported (which has its own ). +/// The Server HistoryRead path surfaces verbatim as that node's result status +/// instead of the generic BadHistoryOperationUnsupported, so a refused read is never mistaken for +/// an empty Good one. +/// +public sealed class HistoryReadStatusException : Exception +{ + /// The raw OPC UA status code the upstream server returned for the node. + public uint StatusCode { get; } + + /// Creates the exception for the upstream . + /// The Bad OPC UA status code to surface for the node. + /// is not a Bad code (severity bits 10/11). + public HistoryReadStatusException(uint statusCode) + : base(IsBad(statusCode) + ? $"The upstream server refused the history read with status 0x{statusCode:X8}." + : throw new ArgumentException($"0x{statusCode:X8} is not a Bad status code.", nameof(statusCode))) + => StatusCode = statusCode; + + private static bool IsBad(uint statusCode) => (statusCode & 0x80000000u) != 0; +} + /// /// One row returned by — a historical /// alarm/event record, not the OPC UA live-event stream. Fields match the minimum set the 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 5b0220c2..539d5b0b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -2275,7 +2275,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit ProcessingInterval = interval.TotalMilliseconds, AggregateType = [aggregateId], }; - return await ExecuteHistoryReadAsync(fullReference, new ExtensionObject(details), cancellationToken) + return await ExecuteHistoryReadAsync(fullReference, new ExtensionObject(details), cancellationToken, aggregate) .ConfigureAwait(false); } @@ -2295,11 +2295,16 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit /// /// Shared HistoryRead wire path — used by Raw/Processed/AtTime. Handles NodeId parse, - /// Session.HistoryReadAsync call, Bad-StatusCode passthrough (no translation per §8 - /// cascading-quality rule), and HistoryData unwrap into . + /// Session.HistoryReadAsync call, per-sample StatusCode passthrough (no translation per §8 + /// cascading-quality rule), and HistoryData unwrap into . A Bad + /// per-NODE HistoryReadResult.StatusCode is never read as empty data: BadAggregateNotSupported + /// throws , any other Bad throws + /// (Gitea #649). Good / GoodNoData / GoodMoreData / Uncertain are + /// unchanged. /// private async Task ExecuteHistoryReadAsync( - string fullReference, ExtensionObject historyReadDetails, CancellationToken ct) + string fullReference, ExtensionObject historyReadDetails, CancellationToken ct, + HistoryAggregateType? aggregate = null) { // Make sure a session exists before queuing on the gate, but do NOT bind the wire call // (or the namespace-relative NodeId parse) to this reference — a reconnect can swap @@ -2335,6 +2340,16 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit if (resp.Results.Count == 0) return new Core.Abstractions.HistoryReadResult([], null); var r = resp.Results[0]; + // The per-node status decides whether the payload means anything. BadNoData is the one Bad code + // that legitimately means "nothing in the window", so it stays an empty result. + // CodeBits (not Code) so an info/overflow sub-status bit on the upstream code does not dodge the match. + if (StatusCode.IsBad(r.StatusCode) && r.StatusCode.CodeBits != StatusCodes.BadNoData) + { + if (r.StatusCode.CodeBits == StatusCodes.BadAggregateNotSupported && aggregate is { } agg) + throw new HistoryAggregateNotSupportedException(agg); + throw new HistoryReadStatusException(r.StatusCode.Code); + } + // Unwrap HistoryData from the ExtensionObject-encoded payload the SDK returns. // Samples stay in chronological order per OPC UA Part 11; cascading-quality // rule: preserve each DataValue's upstream StatusCode + timestamps verbatim. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 3584a5f3..01f81a8f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -5419,6 +5419,14 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 errors[handle.Index] = StatusCodes.BadAggregateNotSupported; results[handle.Index] = new SdkHistoryReadResult { StatusCode = StatusCodes.BadAggregateNotSupported }; } + catch (HistoryReadStatusException ex) + { + // A passthrough backend (OpcUaClient) relayed the upstream server's Bad per-node status - surface + // it verbatim for THIS node rather than collapsing it to BadHistoryOperationUnsupported. + _logger.LogDebug(ex, "OtOpcUaNodeManager: backend refused history read for node {NodeId}", handle.NodeId); + errors[handle.Index] = new StatusCode(ex.StatusCode); + results[handle.Index] = new SdkHistoryReadResult { StatusCode = new StatusCode(ex.StatusCode) }; + } catch (Exception ex) { // One node's backend failure (throw / timeout) must not throw out of the batch — surface a @@ -5620,6 +5628,13 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 errors[handle.Index] = StatusCodes.BadTimeout; results[handle.Index] = new SdkHistoryReadResult { StatusCode = StatusCodes.BadTimeout }; } + catch (HistoryReadStatusException ex) + { + // Upstream Bad per-node status relayed by a passthrough backend - surface it verbatim (see ServeNodeAsync). + _logger.LogDebug(ex, "OtOpcUaNodeManager: backend refused HistoryReadRaw for node {NodeId}", handle.NodeId); + errors[handle.Index] = new StatusCode(ex.StatusCode); + results[handle.Index] = new SdkHistoryReadResult { StatusCode = new StatusCode(ex.StatusCode) }; + } catch (Exception ex) { // One node's backend failure must not throw out of the batch — Bad for THIS node only. diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/Historian/HistoryReadStatusExceptionTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/Historian/HistoryReadStatusExceptionTests.cs new file mode 100644 index 00000000..c943a8c1 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/Historian/HistoryReadStatusExceptionTests.cs @@ -0,0 +1,21 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests.Historian; + +public sealed class HistoryReadStatusExceptionTests +{ + [Theory] + [InlineData(0x801F0000u)] // BadUserAccessDenied + [InlineData(0xC0000000u)] // severity 11 (reserved Bad) + public void A_Bad_code_is_carried_verbatim(uint code) => + new HistoryReadStatusException(code).StatusCode.ShouldBe(code); + + [Theory] + [InlineData(0x00000000u)] // Good + [InlineData(0x00A70000u)] // GoodNoData + [InlineData(0x40000000u)] // Uncertain + public void A_non_Bad_code_is_rejected(uint code) => + Should.Throw(() => new HistoryReadStatusException(code)); +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/OpcUaClientDriverHistoryWireTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/OpcUaClientDriverHistoryWireTests.cs index 8be1a1d0..b1afc571 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/OpcUaClientDriverHistoryWireTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/OpcUaClientDriverHistoryWireTests.cs @@ -17,6 +17,9 @@ public sealed class OpcUaClientDriverHistoryWireTests private const string HistRef = "Sim/dev1/Hist"; private const string HistTag = "Plant.Hist.Series"; private const uint Uncertain = 0x40000000u; + private const uint BadNoData = 0x809B0000u; + private const uint BadUserAccessDenied = 0x801F0000u; + private const uint BadHistoryOperationUnsupported = 0x80720000u; private static readonly DateTime T0 = new(2026, 1, 1, 12, 0, 0, DateTimeKind.Utc); @@ -50,6 +53,12 @@ public sealed class OpcUaClientDriverHistoryWireTests HistoryAggregateType aggregate, CancellationToken ct) { lock (Calls) Calls.Add($"processed:{fullReference}:{aggregate}"); + // Minimum stands in for "the backend refuses with some OTHER Bad status" (#649). + if (aggregate == HistoryAggregateType.Minimum) + throw new HistoryReadStatusException(BadUserAccessDenied); + // Maximum stands in for a node-level BadNoData ("nothing in the window"), which must stay empty. + if (aggregate == HistoryAggregateType.Maximum) + throw new HistoryReadStatusException(BadNoData); if (aggregate != HistoryAggregateType.Average) throw new HistoryAggregateNotSupportedException(aggregate); @@ -162,7 +171,7 @@ public sealed class OpcUaClientDriverHistoryWireTests } [Fact] - public async Task ReadProcessed_serves_a_supported_aggregate_and_returns_nothing_for_an_unsupported_one() + public async Task ReadProcessed_serves_a_supported_aggregate_and_surfaces_BadAggregateNotSupported_for_an_unsupported_one() { var ct = TestContext.Current.CancellationToken; var (rig, fake, drv) = await StartAsync("hist-processed", ct); @@ -174,25 +183,74 @@ public sealed class OpcUaClientDriverHistoryWireTests avg.Samples.Select(s => Convert.ToDouble(s.Value)).ShouldBe([1.5, 3.5, 5.0]); avg.Samples.Select(s => s.SourceTimestampUtc).ShouldBe([At(0), At(2), At(4)]); - // The backend refuses Count: the server maps that to BadAggregateNotSupported for the node, which the - // driver surfaces as no samples (it does not expose the per-node status). The Average above is the - // positive control that the same path serves data, and the call must have REACHED the backend. - var count = await drv.ReadProcessedAsync(HistRef, At(0), At(6), TimeSpan.FromSeconds(2), HistoryAggregateType.Count, ct); - count.Samples.ShouldBeEmpty(); + // The backend refuses Count: the server maps that to BadAggregateNotSupported for the node, and the + // driver must raise it (not return empty Good data) so its own server surfaces it per node (#649). + // The Average above is the positive control that the same path serves data, and the call must have + // REACHED the backend. + var ex = await Should.ThrowAsync(() => + drv.ReadProcessedAsync(HistRef, At(0), At(6), TimeSpan.FromSeconds(2), HistoryAggregateType.Count, ct)); + ex.Aggregate.ShouldBe(HistoryAggregateType.Count); lock (fake.Calls) fake.Calls.ShouldContain($"processed:{HistTag}:Count"); } [Fact] - public async Task A_read_of_a_non_historized_node_returns_nothing_and_never_reaches_the_historian() + public async Task ReadProcessed_surfaces_any_other_Bad_node_status_instead_of_empty_data() + { + var ct = TestContext.Current.CancellationToken; + var (rig, fake, drv) = await StartAsync("hist-badstatus", ct); + await using var _r = rig; + await using var _d = drv; + + var ex = await Should.ThrowAsync(() => + drv.ReadProcessedAsync(HistRef, At(0), At(6), TimeSpan.FromSeconds(2), HistoryAggregateType.Minimum, ct)); + + ex.StatusCode.ShouldBe(BadUserAccessDenied); + lock (fake.Calls) fake.Calls.ShouldContain($"processed:{HistTag}:Minimum"); + } + + [Fact] + public async Task A_node_level_BadNoData_stays_an_empty_result_not_an_error() + { + var ct = TestContext.Current.CancellationToken; + var (rig, fake, drv) = await StartAsync("hist-badnodata", ct); + await using var _r = rig; + await using var _d = drv; + + var result = await drv.ReadProcessedAsync( + HistRef, At(0), At(6), TimeSpan.FromSeconds(2), HistoryAggregateType.Maximum, ct); + + result.Samples.ShouldBeEmpty(); + lock (fake.Calls) fake.Calls.ShouldContain($"processed:{HistTag}:Maximum", "the call must have reached the backend"); + } + + [Fact] + public async Task A_Good_read_with_no_samples_in_the_window_is_still_empty_data_not_an_error() + { + var ct = TestContext.Current.CancellationToken; + var (rig, _, drv) = await StartAsync("hist-goodnodata", ct); + await using var _r = rig; + await using var _d = drv; + + // Historized node, window past the series: the server answers GoodNoData, which must stay a normal empty result. + var result = await drv.ReadRawAsync(HistRef, At(100), At(110), 0, ct); + + result.Samples.ShouldBeEmpty(); + result.ContinuationPoint.ShouldBeNull(); + } + + [Fact] + public async Task A_read_of_a_non_historized_node_surfaces_its_Bad_status_and_never_reaches_the_historian() { var ct = TestContext.Current.CancellationToken; var (rig, fake, drv) = await StartAsync("hist-none", ct); await using var _r = rig; await using var _d = drv; - var result = await drv.ReadRawAsync(OpcUaClientDriverWireTests.SpeedRef, At(0), At(10), 0, ct); - - result.Samples.ShouldBeEmpty(); + // The server answers BadHistoryOperationUnsupported for a node with no historian tagname; that Bad + // node status is surfaced (#649), no longer read as empty Good data. + var ex = await Should.ThrowAsync(() => + drv.ReadRawAsync(OpcUaClientDriverWireTests.SpeedRef, At(0), At(10), 0, ct)); + ex.StatusCode.ShouldBe(BadHistoryOperationUnsupported); // Positive control: the historized sibling does reach the historian on the same session. (await drv.ReadRawAsync(HistRef, At(0), At(10), 0, ct)).Samples.Count.ShouldBe(5); lock (fake.Calls) fake.Calls.Count.ShouldBe(1, "only the historized read dispatches"); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadPagingTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadPagingTests.cs index 16d8bf29..7e9bb01e 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadPagingTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadPagingTests.cs @@ -312,6 +312,61 @@ public sealed class NodeManagerHistoryReadPagingTests : IDisposable await host.DisposeAsync(); } + /// A passthrough backend's Bad status on a RESUME read surfaces verbatim for that node only (#649): + /// no outbound continuation point, the cursor is consumed (no stray cursor), and a sibling node in the same + /// batch is unaffected. + [Fact] + public async Task Raw_resume_read_that_throws_HistoryReadStatusException_surfaces_the_code_and_isolates_the_sibling() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + nm.HistoryContinuationStore = new InMemoryHistoryContinuationStore(); + var fake = new SeriesHistorianDataSource(MakeSeries(count: 250, stepSeconds: 1)); + nm.HistorianDataSource = fake; + + nm.EnsureVariable("eq-1/bad", parentFolderNodeId: null, displayName: "Bad", dataType: "Double", + writable: false, historianTagname: "WW.Bad", realm: AddressSpaceRealm.Uns); + nm.EnsureVariable("eq-1/ok", parentFolderNodeId: null, displayName: "Ok", dataType: "Double", + writable: false, historianTagname: "WW.Ok", realm: AddressSpaceRealm.Uns); + var badId = nm.TryGetVariable("eq-1/bad")!.NodeId; + var okId = nm.TryGetVariable("eq-1/ok")!.NodeId; + + // Page 1 succeeds and emits a continuation point. + var (_, _, cp1) = ReadRaw(nm, badId, Epoch, Epoch.AddHours(1), max: 100, inboundCp: null); + cp1.ShouldNotBeNull(); + + // The backend now refuses the bad node; one batch resumes it and freshly reads the sibling. + fake.FailTagname = "WW.Bad"; + var context = new OperationContext( + new RequestHeader(), secureChannelContext: null, RequestType.HistoryRead, identity: null); + var details = new ReadRawModifiedDetails + { + StartTime = Epoch, EndTime = Epoch.AddHours(1), NumValuesPerNode = 100, IsReadModified = false, + }; + var nodesToRead = new List + { + new() { NodeId = badId, ContinuationPoint = cp1 }, + new() { NodeId = okId }, + }; + var results = new List { null!, null! }; + var errors = new List { null!, null! }; + nm.HistoryRead(context, details, TimestampsToReturn.Both, + releaseContinuationPoints: false, nodesToRead, results, errors); + + errors[0].StatusCode.Code.ShouldBe(StatusCodes.BadUserAccessDenied); + results[0].ContinuationPoint.ShouldBeNull(); + errors[1].StatusCode.Code.ShouldBe(StatusCodes.Good); + ((HistoryData)ExtensionObject.ToEncodeable(results[1].HistoryData)).DataValues.Count.ShouldBe(100); + results[1].ContinuationPoint.ShouldNotBeNull("the sibling is full-page and pages normally"); + + // The consumed cursor is gone: nothing stray to resume. + fake.FailTagname = null; + var (_, e, _) = ReadRaw(nm, badId, Epoch, Epoch.AddHours(1), max: 100, inboundCp: cp1); + e.StatusCode.Code.ShouldBe(StatusCodes.BadContinuationPointInvalid); + + await host.DisposeAsync(); + } + // --- helpers -------------------------------------------------------------------------------- /// Issue a single-node Raw HistoryRead and return that node's decoded sample values, error, @@ -412,6 +467,9 @@ public sealed class NodeManagerHistoryReadPagingTests : IDisposable { private int _readCount; public int ReadCount => _readCount; + + /// When set, a Raw read of this tagname throws (#649). + public volatile string? FailTagname; public void ResetReadCount() => _readCount = 0; public Task ReadRawAsync( @@ -419,6 +477,8 @@ public sealed class NodeManagerHistoryReadPagingTests : IDisposable CancellationToken cancellationToken) { Interlocked.Increment(ref _readCount); + if (FailTagname is { } bad && bad == fullReference) + throw new HistoryReadStatusException(StatusCodes.BadUserAccessDenied); var window = series .Where(s => (s.SourceTimestampUtc ?? DateTime.MinValue) >= startUtc && (s.SourceTimestampUtc ?? DateTime.MinValue) <= endUtc)