From 3b8b5a77e1e742f811762bcebbb93464a48f32cb Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 06:23:08 -0400 Subject: [PATCH] fix(mtconnect): share MTConnectError text extraction; pin hollow-error refusal --- .../MTConnectAgentClient.cs | 27 ++++------ .../MTConnectXml.cs | 38 ++++++++++++-- .../MTConnectAgentClientWriteTests.cs | 49 +++++++++++++++++++ 3 files changed, 93 insertions(+), 21 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs index a3b39450..f40bf3e1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs @@ -334,7 +334,10 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable /// Read by only, like every other document this driver parses: /// the namespace URI carries the MTConnect version, so a qualified match would silently stop /// recognising errors the day an Agent is upgraded — and an unrecognised error reads as a - /// successful write. + /// successful write. The error-text walk itself is + /// , shared with the read paths' + /// so the two cannot drift on what counts + /// as an Agent objection. /// private static string? DescribeAgentError(string body) { @@ -360,23 +363,11 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable return null; } - var errors = root.Descendants() - .Where(e => MTConnectXml.IsNamed(e, "Error")) - .Select(e => - { - var code = MTConnectXml.OptionalAttribute(e, "errorCode"); - var text = e.Value.Trim(); - - return code is null ? text : $"{code}: {text}"; - }) - .Where(text => text.Length > 0) - .ToList(); - - // An error document with no readable detail is still a refusal; reporting it as null would - // turn it back into an acceptance. - return errors.Count == 0 - ? "The Agent answered with an MTConnectError document carrying no error detail." - : string.Join("; ", errors); + // An error document with no readable detail is still a refusal, so the shared helper's null + // — which means "nothing readable in here", NOT "this was not an error" — must become text + // rather than being passed on. Returning it as null would turn the refusal into acceptance. + return MTConnectXml.ExtractErrorText(root) + ?? "The Agent answered with an MTConnectError document carrying no error detail."; } /// 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 63c31262..d43c4f65 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs @@ -127,6 +127,40 @@ internal static class MTConnectXml return message; } + var errors = ExtractErrorText(root); + + return errors is null ? message : $"{message} The Agent reported: {errors}"; + } + + /// + /// The Agent's own words from an MTConnectError document: every <Error> + /// beneath , rendered errorCode: text where a code is present + /// and joined with "; ". null when the document carries no readable error + /// detail at all (no <Error> elements, or only blank ones). + /// + /// + /// + /// Shared because both callers must read the same document the same way. An + /// Agent's objection reaches this driver through two unrelated doors — a read path that + /// got an error document where it expected a model + /// (), and the write path deciding whether a 200 + /// actually accepted the observation. Two copies of this walk would drift on the same + /// two silent rules the type-level remarks describe, and on the write side a missed + /// <Error> does not merely degrade a message: it reads as a successful + /// write. + /// + /// + /// null means "no readable detail", NOT "not an error document". Only the + /// caller knows which document it is holding — check the root name first. A caller that + /// treats a null here as "no error occurred" turns a hollow + /// <MTConnectError/> into a success. + /// + /// + /// The MTConnectError document's root element. + public static string? ExtractErrorText(XElement root) + { + ArgumentNullException.ThrowIfNull(root); + var errors = root.Descendants() .Where(e => IsNamed(e, "Error")) .Select(e => @@ -139,8 +173,6 @@ internal static class MTConnectXml .Where(text => text.Length > 0) .ToList(); - return errors.Count == 0 - ? message - : $"{message} The Agent reported: {string.Join("; ", errors)}"; + return errors.Count == 0 ? null : string.Join("; ", errors); } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs index cadf4af1..ea7d20ec 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs @@ -212,6 +212,55 @@ public sealed class MTConnectAgentClientWriteTests outcome.AgentError.ShouldContain("Cannot find device data item: nosuchitem"); } + /// + /// An MTConnectError document carrying no readable <Error> detail is + /// still a refusal. The Agent said no; that it said so unhelpfully changes nothing. The trap + /// this pins is that the natural implementation — "return the joined error text, or null if + /// there is none" — hands back the same null that means "this body was not an error + /// document at all", silently converting the refusal into an acceptance. + /// + [Fact] + public async Task An_error_document_with_no_readable_detail_is_still_a_refusal() + { + const string HollowError = + """ + + +
+ + + """; + + using var client = new MTConnectAgentClient( + Options(), new StubHandler((_, _) => Task.FromResult(XmlOk(HollowError)))); + + var outcome = await client.WriteObservationAsync("StubDevice", "dev1_msg", "hi", CancellationToken.None); + + outcome.Accepted.ShouldBeFalse(); + outcome.AgentError.ShouldNotBeNullOrWhiteSpace(); + } + + /// The same, for an <Error> element that is present but blank. + [Fact] + public async Task An_error_document_whose_error_element_is_blank_is_still_a_refusal() + { + const string BlankError = + """ + + + + + """; + + using var client = new MTConnectAgentClient( + Options(), new StubHandler((_, _) => Task.FromResult(XmlOk(BlankError)))); + + var outcome = await client.WriteObservationAsync("StubDevice", "dev1_msg", "hi", CancellationToken.None); + + outcome.Accepted.ShouldBeFalse(); + outcome.AgentError.ShouldNotBeNullOrWhiteSpace(); + } + [Fact] public async Task A_non_success_status_is_a_refusal_naming_the_status() {