From e5b1724bf66044f7541bbfecd290461193cb1342 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= Date: Mon, 10 Aug 2026 07:46:27 +0200 Subject: [PATCH] Suppress the duplicate failure, never the only one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review of #1061. The FailureAlreadyReported marker was set unconditionally — including on the path where the typed NACK's Post THREW and the exception was swallowed and logged: try { Post(new DeliveryFailure(delivery) { ErrorType = ... }); } catch (Exception ex) { logger.LogError(...); } // swallowed return delivery.Failed(reason).WithProperty(FailureAlreadyReported, true); In that window the marker suppressed the unclassified follow-up while no typed failure had gone out, so the caller got NO failure response at all — a silent park, which is precisely the wedge this NACK path exists to prevent. #1061 traded a wrong classification for a possible hang; narrow, but the wrong direction. The marker is now set only when the Post actually returned, in both NACK branches (MessageService's RawJson path and MessageHub's unhandled-message path). Suppress the DUPLICATE, never the only answer. MeshWeaver.Messaging.Hub.Test 167/167. OrleansBrokenNodeTypeAccessTest 2/2, also under DOTNET_PROCESSOR_COUNT=4. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AU2kBewK9hnv2WLQB6H8GH --- src/MeshWeaver.Messaging.Hub/MessageHub.cs | 15 ++++++++++----- src/MeshWeaver.Messaging.Hub/MessageService.cs | 13 +++++++++---- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/MeshWeaver.Messaging.Hub/MessageHub.cs b/src/MeshWeaver.Messaging.Hub/MessageHub.cs index 9be104daa6..841d1e75dc 100644 --- a/src/MeshWeaver.Messaging.Hub/MessageHub.cs +++ b/src/MeshWeaver.Messaging.Hub/MessageHub.cs @@ -792,6 +792,7 @@ private IMessageDelivery FinishDelivery(IMessageDelivery delivery) logger.LogWarning( "Unhandled {MessageType} (ID: {MessageId}) in fallback hub {Address} - answering {ErrorType} NACK: {Reason}", delivery.Message.GetType().Name, delivery.Id, Address, nackPolicy.ErrorType, nackPolicy.Reason); + var nackPosted = false; try { Post(new DeliveryFailure(delivery) @@ -800,17 +801,21 @@ private IMessageDelivery FinishDelivery(IMessageDelivery delivery) NodeTypePath = nackPolicy.NodeTypePath, Message = nackPolicy.Reason }, o => o.ResponseFor(delivery)); + nackPosted = true; } catch (Exception ex) { logger.LogError(ex, "Failed to post fallback NACK for {MessageType} (ID: {MessageId}) in {Address}", delivery.Message.GetType().Name, delivery.Id, Address); } - // The typed NACK above IS this request's failure response — mark it so no - // unclassified follow-up is posted for the same delivery (see - // MessageService.FailureAlreadyReported). - return delivery.Failed(nackPolicy.Reason) - .WithProperty(MessageService.FailureAlreadyReported, true); + // 🚨 Mark ONLY when the typed NACK actually went out — see + // MessageService.FailureAlreadyReported. Marking after a THROWN post would suppress + // the follow-up and leave the caller with NO failure response, which is the silent + // park this NACK path exists to prevent. + var failed = delivery.Failed(nackPolicy.Reason); + return nackPosted + ? failed.WithProperty(MessageService.FailureAlreadyReported, true) + : failed; } // Check if this is a request that expects a response diff --git a/src/MeshWeaver.Messaging.Hub/MessageService.cs b/src/MeshWeaver.Messaging.Hub/MessageService.cs index 312ddda822..2f03074ced 100644 --- a/src/MeshWeaver.Messaging.Hub/MessageService.cs +++ b/src/MeshWeaver.Messaging.Hub/MessageService.cs @@ -1413,6 +1413,7 @@ private IMessageDelivery DeserializeDelivery(IMessageDelivery delivery) var reason = $"{nackPolicy.Reason} (inbound type '{jsonType}' is not registered in this hub's TypeRegistry)"; logger.LogWarning("Unhandled {JsonType} in fallback hub {Address} - answering {ErrorType} NACK: {Reason}", jsonType, Address, nackPolicy.ErrorType, reason); + var nackPosted = false; try { Post(new DeliveryFailure(delivery) @@ -1421,16 +1422,20 @@ private IMessageDelivery DeserializeDelivery(IMessageDelivery delivery) NodeTypePath = nackPolicy.NodeTypePath, Message = reason }, new PostOptions(Address).ResponseFor(delivery)); + nackPosted = true; } catch (Exception ex) { logger.LogError(ex, "Failed to post fallback NACK for '{JsonType}' (ID: {MessageId}) in {Address}", jsonType, delivery.Id, Address); } - // The typed NACK above IS this request's failure response. Mark it so the - // Failed-state check on the way out does not post an unclassified second one and - // race it to the caller's subject. - return delivery.Failed(reason).WithProperty(FailureAlreadyReported, true); + // 🚨 Mark ONLY when the typed NACK actually went out. The marker suppresses the + // unclassified follow-up, so setting it after a THROWN post would leave the caller + // with no failure response at all — turning a swallowed exception into the silent + // park this whole NACK path exists to prevent. Suppress the duplicate, never the + // only answer. + var failed = delivery.Failed(reason); + return nackPosted ? failed.WithProperty(FailureAlreadyReported, true) : failed; } return ReportFailure(delivery.Failed(failureMessage));