From ac01c88c0e18d677fee2d12789c63dbbb0c7c288 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 16:03:38 -0400 Subject: [PATCH] fix(mtconnect): an Agent refusal of /current is a reply, not a comms fault Health contract: an MTConnectError document (NO_DEVICE, INVALID_REQUEST, ...) is the Agent answering. The parsers now mark such an InvalidDataException (MTConnectXml.IsAgentRefusal - the type is sealed, so a Data marker, every other handler unchanged); ReadAsync reports the batch Bad but does not degrade the read path, advances LastSuccessfulRead, clears a prior read degradation and warns once per distinct refusal. An unparseable answer stays a comms fault. Refs #743 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- docs/drivers/MTConnect.md | 7 ++ .../MTConnectDriver.cs | 20 ++++++ .../MTConnectProbeParser.cs | 2 +- .../MTConnectStreamExceptions.cs | 1 + .../MTConnectStreamsParser.cs | 3 +- .../MTConnectXml.cs | 39 ++++++++++- .../MTConnectReadPathTests.cs | 69 +++++++++++++++++++ 7 files changed, 137 insertions(+), 4 deletions(-) diff --git a/docs/drivers/MTConnect.md b/docs/drivers/MTConnect.md index 603cdb6c..0edc52f3 100644 --- a/docs/drivers/MTConnect.md +++ b/docs/drivers/MTConnect.md @@ -123,6 +123,13 @@ read-path degradation an earlier failed `/current` left (the read path is answer checks the answer's `instanceId`: a changed one (an Agent restart seen by a browse-only driver with no pump) drops the cached `/probe` model and raises `OnRediscoveryNeeded` once per distinct id — it used to be detected only by the pump. +**An Agent refusal is a reply (health contract).** An `MTConnectError` document answering `/current` (e.g. +`NO_DEVICE` for a wrong `DeviceName` scope) still fails the batch (`BadCommunicationError` per ref), but it is the +Agent answering, not a comms fault: the read path is not degraded by it (that would let the comms-loss watchdog +reconnect-loop the driver over a configuration problem), `LastSuccessfulRead` advances, a prior read degradation +clears, and the Agent's text is logged once per distinct refusal. An answer that is not an MTConnect document at all +(a proxy's HTML page, truncated XML) stays a comms fault. + ## Minimum deployment ```jsonc diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs index 3990ab31..e7908a08 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs @@ -369,6 +369,9 @@ public sealed partial class MTConnectDriver /// Whether the /current read path is currently failing. See . private volatile bool _readDegraded; + /// The last /current refusal text logged, so a persistent refusal warns once, not per read. + private string? _lastReadRefusal; + /// /// Whether the current-generation /sample pump has delivered since it started (a chunk, a heartbeat or /// a re-baseline) — the precondition, with clear, for @@ -770,6 +773,23 @@ public sealed partial class MTConnectDriver // handled below as the Agent failure it is. throw; } + catch (InvalidDataException ex) when (MTConnectXml.IsAgentRefusal(ex)) + { + // The health contract: an MTConnectError document is the Agent REFUSING the request (a bad device + // scope, an unsupported request) — it answered. The batch is Bad, but this is not a comms fault: the + // read path is not degraded by it (that would reconnect-loop a driver over a config problem), the stamp + // advances, and the Agent's text is logged once per distinct refusal. + _readDegraded = false; + PublishHealthy(DateTime.UtcNow); + if (!string.Equals(Interlocked.Exchange(ref _lastReadRefusal, ex.Message), ex.Message, StringComparison.Ordinal)) + { + _logger.LogWarning( + "MTConnect driver {DriverInstanceId}: {AgentUri} refused /current — {AgentError}. Every read is reported Bad until the Agent accepts the request; the driver stays connected.", + _driverInstanceId, AgentDisplay(options), ex.Message); + } + + return BadBatch(fullReferences.Count, BadCommunicationError); + } catch (Exception ex) { DegradeAfterReadFailure(ex); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectProbeParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectProbeParser.cs index 4d884e1a..0a184e49 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectProbeParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectProbeParser.cs @@ -118,7 +118,7 @@ internal static class MTConnectProbeParser if (!MTConnectXml.IsNamed(root, "MTConnectDevices")) { - throw new InvalidDataException(MTConnectXml.DescribeUnexpectedRoot(root, "MTConnectDevices", Subject)); + throw MTConnectXml.UnexpectedRoot(root, "MTConnectDevices", Subject); } var devicesContainer = MTConnectXml.ChildrenNamed(root, "Devices").FirstOrDefault() diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamExceptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamExceptions.cs index c1a02259..fa41c55a 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamExceptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamExceptions.cs @@ -159,3 +159,4 @@ public sealed class MTConnectStreamNotSupportedException : MTConnectStreamExcept { } } + diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs index dfa943d7..4e32e5f7 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs @@ -295,8 +295,7 @@ internal static class MTConnectStreamsParser // exactly this shape (Agents serve one under HTTP 200), and its elements — the // Agent's own account of what it objected to — are the most useful thing in the whole // response. Materialising the root is how the shared extraction can reach them. - throw new InvalidDataException( - MTConnectXml.DescribeUnexpectedRoot((XElement)XNode.ReadFrom(reader), "MTConnectStreams", Subject)); + throw MTConnectXml.UnexpectedRoot((XElement)XNode.ReadFrom(reader), "MTConnectStreams", Subject); } var observations = new List(); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs index a92d376b..3497d27f 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs @@ -190,7 +190,44 @@ internal static class MTConnectXml /// The document's actual root element. /// The root element name the caller required. /// How to describe the document in the error message. - /// The error message describing the unexpected root. + /// + /// The exception to throw. For an MTConnectError root (the Agent REFUSED the request) it is marked + /// with — see . + /// + public static InvalidDataException UnexpectedRoot(XElement root, string expectedRootName, string subject) + { + var ex = new InvalidDataException(DescribeUnexpectedRoot(root, expectedRootName, subject)); + + // Marked (InvalidDataException is sealed, so not a subtype) so the read path can tell a refusal — the + // Agent answered — from an answer it could not parse at all. Every other handler sees the same exception. + if (IsNamed(root, "MTConnectError")) + { + ex.Data[AgentRefusalDataKey] = true; + } + + return ex; + } + + /// key marking an MTConnectError answer (see ). + internal const string AgentRefusalDataKey = "MTConnect.AgentRefusal"; + + /// + /// Whether is the Agent answering with an MTConnectError document (e.g. + /// NO_DEVICE, INVALID_REQUEST) — a refusal, as opposed to an answer that could not be parsed. + /// + /// The exception a parse raised. + /// for a refusal. + public static bool IsAgentRefusal(Exception ex) => + ex is InvalidDataException && ex.Data[AgentRefusalDataKey] is true; + + /// + /// The text of : the unexpected root, plus the Agent's own error text for an + /// MTConnectError document. + /// + /// The document's root element. + /// The root the caller expected. + /// How to describe the document. + /// The message. public static string DescribeUnexpectedRoot(XElement root, string expectedRootName, string subject) { var message = diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectReadPathTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectReadPathTests.cs index 8c66a278..8ec06742 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectReadPathTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectReadPathTests.cs @@ -159,4 +159,73 @@ public sealed class MTConnectReadPathTests await driver.ShutdownAsync(CancellationToken.None); } } + + private const string NoDeviceError = + """ + + +
+ Could not find the device 'VMC-9' + + """; + + private static Exception ParseFailure(string xml) + { + try + { + MTConnectStreamsParser.Parse(xml); + } + catch (Exception ex) + { + return ex; + } + + throw new InvalidOperationException("expected the parse to fail"); + } + + /// + /// Health contract: an MTConnectError answer to /current is the Agent REFUSING (it answered) — the batch is Bad, + /// but health is restored / kept Healthy with the stamp advanced, and the refusal is logged once, not per read. + /// + [Fact] + public async Task An_agent_refusal_of_current_is_a_reply_not_a_comms_fault() + { + var logger = new RecordingDriverLogger(); + var client = CannedAgentClient.FromFixtures(); + var driver = new MTConnectDriver(Opts(), "mt-refusal", _ => client, logger); + await driver.InitializeAsync("{}", Ct); + try + { + // A comms fault first, so the refusal has a Degraded to restore. + client.CurrentFailure = new HttpRequestException("connection refused"); + await driver.ReadAsync(["dev1_pos"], Ct); + driver.GetHealth().State.ShouldBe(DriverState.Degraded, "positive control: a comms fault does degrade"); + var stamp = driver.GetHealth().LastSuccessfulRead.ShouldNotBeNull(); + + var refusal = ParseFailure(NoDeviceError); + MTConnectXml.IsAgentRefusal(refusal).ShouldBeTrue(); + client.CurrentFailure = refusal; + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(5); + while (DateTime.UtcNow <= stamp && DateTime.UtcNow < deadline) await Task.Yield(); + + var first = await driver.ReadAsync(["dev1_pos"], Ct); + await driver.ReadAsync(["dev1_pos"], Ct); + + first.Single().StatusCode.ShouldBe(0x80050000u, "the batch is still Bad"); + var h = driver.GetHealth(); + h.State.ShouldBe(DriverState.Healthy); + h.LastSuccessfulRead.ShouldNotBeNull().ShouldBeGreaterThan(stamp); + logger.WarningsSnapshot().Count(w => w.Contains("NO_DEVICE")).ShouldBe(1, "a persistent refusal warns once"); + } + finally + { + client.CurrentFailure = null; + await driver.ShutdownAsync(CancellationToken.None); + } + } + + /// An answer that is not an MTConnect document at all is NOT marked a refusal (it stays a comms fault). + [Fact] + public void A_garbage_answer_is_not_an_agent_refusal() => + MTConnectXml.IsAgentRefusal(ParseFailure("proxy error")).ShouldBeFalse(); }