fix(opcuaclient): #649 review minors - CodeBits match, Bad-only exception ctor, BadNoData + Raw-resume tests, latent-impact docs

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe

docs/drivers/OpcUaClient.md

@@ -123,13 +123,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).** A Bad node status is never read as
+**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 server's HistoryRead path relays both verbatim as that node's
-result status (and only that node's). `Good`, `GoodNoData`, `GoodMoreData` (continuation point) and `Uncertain`
-are unchanged, and a node-level `BadNoData` is still an empty result. Callers that previously saw an empty list for
-a refused read now see the Bad status, so an unhistorized upstream node reads `BadHistoryOperationUnsupported`.
+`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.
 

src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IHistoryProvider.cs

@@ -200,9 +200,14 @@ public sealed class HistoryReadStatusException : Exception
 
     /// <summary>Creates the exception for the upstream <paramref name="statusCode"/>.</summary>
     /// <param name="statusCode">The Bad OPC UA status code to surface for the node.</param>
+    /// <exception cref="ArgumentException"><paramref name="statusCode"/> is not a Bad code (severity bits <c>10</c>/<c>11</c>).</exception>
     public HistoryReadStatusException(uint statusCode)
-        : base($"The upstream server refused the history read with status 0x{statusCode:X8}.")
+        : 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;
 }
 
 /// <summary>

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs

@@ -2382,9 +2382,10 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
 
             // 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.
-            if (StatusCode.IsBad(r.StatusCode) && r.StatusCode.Code != StatusCodes.BadNoData)
+            // 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.Code == StatusCodes.BadAggregateNotSupported && aggregate is { } agg)
+                if (r.StatusCode.CodeBits == StatusCodes.BadAggregateNotSupported && aggregate is { } agg)
                     throw new HistoryAggregateNotSupportedException(agg);
                 throw new HistoryReadStatusException(r.StatusCode.Code);
             }

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<ArgumentException>(() => new HistoryReadStatusException(code));
+}

tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/OpcUaClientDriverHistoryWireTests.cs

@@ -17,6 +17,7 @@ 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;
 
@@ -55,6 +56,9 @@ public sealed class OpcUaClientDriverHistoryWireTests
             // 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);
 
@@ -204,6 +208,21 @@ public sealed class OpcUaClientDriverHistoryWireTests
         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()
     {

tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadPagingTests.cs

@@ -312,6 +312,61 @@ public sealed class NodeManagerHistoryReadPagingTests : IDisposable
         await host.DisposeAsync();
     }
 
+    /// <summary>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.</summary>
+    [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<HistoryReadValueId>
+        {
+            new() { NodeId = badId, ContinuationPoint = cp1 },
+            new() { NodeId = okId },
+        };
+        var results = new List<SdkHistoryReadResult> { null!, null! };
+        var errors = new List<ServiceResult> { 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 --------------------------------------------------------------------------------
 
     /// <summary>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;
+
+        /// <summary>When set, a Raw read of this tagname throws <see cref="HistoryReadStatusException"/> (#649).</summary>
+        public volatile string? FailTagname;
         public void ResetReadCount() => _readCount = 0;
 
         public Task<HistorianRead> 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)