fix(opcuaclient): HistoryRead honours the per-node StatusCode (BadAggregateNotSupported + other Bad) instead of returning empty data (#649)
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
@@ -123,6 +123,13 @@ 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
|
||||
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`.
|
||||
Event-history (`ReadEventsAsync`) is left at the interface default — the
|
||||
interface doesn't yet carry the EventFilter surface needed to forward it.
|
||||
|
||||
|
||||
@@ -185,6 +185,26 @@ public sealed class HistoryAggregateNotSupportedException : NotSupportedExceptio
|
||||
=> Aggregate = aggregate;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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 <c>HistoryReadResult.StatusCode</c> other than
|
||||
/// <c>BadAggregateNotSupported</c> (which has its own <see cref="HistoryAggregateNotSupportedException"/>).
|
||||
/// The Server HistoryRead path surfaces <see cref="StatusCode"/> verbatim as that node's result status
|
||||
/// instead of the generic <c>BadHistoryOperationUnsupported</c>, so a refused read is never mistaken for
|
||||
/// an empty Good one.
|
||||
/// </summary>
|
||||
public sealed class HistoryReadStatusException : Exception
|
||||
{
|
||||
/// <summary>The raw OPC UA status code the upstream server returned for the node.</summary>
|
||||
public uint StatusCode { get; }
|
||||
|
||||
/// <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>
|
||||
public HistoryReadStatusException(uint statusCode)
|
||||
: base($"The upstream server refused the history read with status 0x{statusCode:X8}.")
|
||||
=> StatusCode = statusCode;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// One row returned by <see cref="IHistoryProvider.ReadEventsAsync"/> — a historical
|
||||
/// alarm/event record, not the OPC UA live-event stream. Fields match the minimum set the
|
||||
|
||||
@@ -2315,7 +2315,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);
|
||||
}
|
||||
|
||||
@@ -2335,11 +2335,16 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
|
||||
/// <summary>
|
||||
/// 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 <see cref="DataValueSnapshot"/>.
|
||||
/// Session.HistoryReadAsync call, per-sample StatusCode passthrough (no translation per §8
|
||||
/// cascading-quality rule), and HistoryData unwrap into <see cref="DataValueSnapshot"/>. A Bad
|
||||
/// per-NODE <c>HistoryReadResult.StatusCode</c> is never read as empty data: <c>BadAggregateNotSupported</c>
|
||||
/// throws <see cref="HistoryAggregateNotSupportedException"/>, any other Bad throws
|
||||
/// <see cref="HistoryReadStatusException"/> (Gitea #649). Good / GoodNoData / GoodMoreData / Uncertain are
|
||||
/// unchanged.
|
||||
/// </summary>
|
||||
private async Task<Core.Abstractions.HistoryReadResult> 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
|
||||
@@ -2375,6 +2380,15 @@ 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.
|
||||
if (StatusCode.IsBad(r.StatusCode) && r.StatusCode.Code != StatusCodes.BadNoData)
|
||||
{
|
||||
if (r.StatusCode.Code == 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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
+49
-10
@@ -17,6 +17,8 @@ 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 BadUserAccessDenied = 0x801F0000u;
|
||||
private const uint BadHistoryOperationUnsupported = 0x80720000u;
|
||||
|
||||
private static readonly DateTime T0 = new(2026, 1, 1, 12, 0, 0, DateTimeKind.Utc);
|
||||
|
||||
@@ -50,6 +52,9 @@ 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);
|
||||
if (aggregate != HistoryAggregateType.Average)
|
||||
throw new HistoryAggregateNotSupportedException(aggregate);
|
||||
|
||||
@@ -162,7 +167,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 +179,59 @@ 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<HistoryAggregateNotSupportedException>(() =>
|
||||
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<HistoryReadStatusException>(() =>
|
||||
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_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<HistoryReadStatusException>(() =>
|
||||
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");
|
||||
|
||||
Reference in New Issue
Block a user