From 81d7074cbaf86f160f8ac0b543a749673072884a Mon Sep 17 00:00:00 2001 From: "Jeremy D. Miller" Date: Wed, 5 Aug 2026 07:57:25 -0500 Subject: [PATCH] Carry an exception type name, not an Exception, through the compliance harness (GH-3800) TransportCompliance injects errors with ErrorCausingMessage, which carried a Dictionary. System.Text.Json cannot round-trip that. Demonstrated directly: serialized: {"1":{"TargetSite":null,"Message":"Attempted to divide by zero.",...}} deserialized runtime type: System.Exception is DivideByZeroException? False It writes the exception's properties and rebuilds it as a bare System.Exception, so the type identity -- the only thing an exception-match rule keys on -- is gone. The handler threw the wrong type and the rule could never fire, so with_cloud_events opted out of will_move_to_dead_letter_queue_with_exception_match altogether. The hole was in the shared harness, not in one transport: anything wired .InteropWithCloudEvents() and run through TransportCompliance inherited it, silently. Pulsar is just the only CloudEvents fixture in the battery, so it is where it showed. Errors is now Dictionary of assembly-qualified type names, with a ThrowOnAttempt() helper so the name/instance distinction lives in one place, and the handler rehydrates. A name that will not resolve throws loudly rather than falling through to "no error" -- a silently wrong exception type is the exact failure this replaces. with_cloud_events keeps the skip on that test but for its siblings' reason: the serialization limit is gone, Pulsar's unimplemented dead-lettering (GH-3797) is not. When GH-3797 lands the skip goes with the other three rather than needing separate attention. Four new tests pin the harness directly, including the unresolvable-name case. They are the only coverage this fix can have until GH-3797 unblocks the CloudEvents path. CoreTests ErrorHandling 214/214 (existing) + 4 new; dotnet build wolverine.slnx -c Release clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WHAuhdWS3XeAk16swV9G8m --- .../ErrorHandling/ErrorHandlingContext.cs | 2 +- ...r_injection_survives_serialization_3800.cs | 85 +++++++++++++++++++ .../Compliance/TransportCompliance.cs | 4 +- .../ErrorHandling/ErrorCausingMessage.cs | 29 ++++++- .../ErrorCausingMessageHandler.cs | 30 ++++++- .../Wolverine.Pulsar.Tests/WithCloudEvents.cs | 15 ++-- 6 files changed, 149 insertions(+), 16 deletions(-) create mode 100644 src/Testing/CoreTests/ErrorHandling/error_injection_survives_serialization_3800.cs diff --git a/src/Testing/CoreTests/ErrorHandling/ErrorHandlingContext.cs b/src/Testing/CoreTests/ErrorHandling/ErrorHandlingContext.cs index 5c6f0fcd7..63a75d8a0 100644 --- a/src/Testing/CoreTests/ErrorHandling/ErrorHandlingContext.cs +++ b/src/Testing/CoreTests/ErrorHandling/ErrorHandlingContext.cs @@ -33,7 +33,7 @@ public void Dispose() protected void throwOnAttempt(int attempt) where T : Exception, new() { - theMessage.Errors.Add(attempt, new T()); + theMessage.ThrowOnAttempt(attempt); } protected async Task afterProcessingIsComplete() diff --git a/src/Testing/CoreTests/ErrorHandling/error_injection_survives_serialization_3800.cs b/src/Testing/CoreTests/ErrorHandling/error_injection_survives_serialization_3800.cs new file mode 100644 index 000000000..3728daa91 --- /dev/null +++ b/src/Testing/CoreTests/ErrorHandling/error_injection_survives_serialization_3800.cs @@ -0,0 +1,85 @@ +using System.Text.Json; +using Shouldly; +using Wolverine; +using Wolverine.ComplianceTests.ErrorHandling; +using Xunit; + +namespace CoreTests.ErrorHandling; + +/// +/// GH-3800. The compliance battery injects errors through , and it +/// used to do so by carrying live Exception instances. System.Text.Json cannot +/// round-trip those, so any transport wired .InteropWithCloudEvents() and run through +/// TransportCompliance silently lost dead-lettering-by-exception-type coverage — the handler threw +/// the wrong type and an exception-match rule could never fire. +/// +/// These pin the harness itself rather than a transport. Pulsar is currently the only +/// CloudEvents fixture in the battery, and its DLQ tests are skipped for an unrelated reason +/// (GH-3797), so without these the fix would have no coverage anywhere. +/// +public class error_injection_survives_serialization_3800 +{ + private static ErrorCausingMessage roundTrip(ErrorCausingMessage message) + { + // The same serializer CloudEvents uses internally, with no custom converters -- which is + // exactly the configuration that corrupted the old Dictionary. + var json = JsonSerializer.Serialize(message); + return JsonSerializer.Deserialize(json)!; + } + + private static void handle(ErrorCausingMessage message, int attempt) + { + new ErrorCausingMessageHandler() + .Handle(message, new Envelope { Attempts = attempt }, new AttemptTracker()); + } + + [Fact] + public void the_declared_exception_type_survives_a_json_round_trip() + { + var message = new ErrorCausingMessage(); + message.ThrowOnAttempt(1); + + var received = roundTrip(message); + + // Before GH-3800 this threw *something*, but not this -- which is worse than throwing + // nothing, because an exception-match rule then quietly never fires. + Should.Throw(() => handle(received, 1)); + } + + [Fact] + public void distinct_attempts_keep_their_own_exception_types() + { + var message = new ErrorCausingMessage(); + message.ThrowOnAttempt(1); + message.ThrowOnAttempt(2); + + var received = roundTrip(message); + + Should.Throw(() => handle(received, 1)); + Should.Throw(() => handle(received, 2)); + } + + [Fact] + public void an_attempt_with_no_error_is_processed_normally() + { + var message = new ErrorCausingMessage(); + message.ThrowOnAttempt(1); + + var received = roundTrip(message); + + handle(received, 2); + + received.WasProcessed.ShouldBeTrue(); + } + + [Fact] + public void an_unresolvable_type_name_fails_loudly() + { + // A silently-wrong exception type is the failure this replaced, so a name that cannot be + // resolved in the receiving process must not quietly become "no error". + var message = new ErrorCausingMessage { Errors = { [1] = "Not.A.Real.Type, Nowhere" } }; + + var ex = Should.Throw(() => handle(roundTrip(message), 1)); + ex.Message.ShouldContain("Not.A.Real.Type"); + } +} diff --git a/src/Testing/Wolverine.ComplianceTests/Compliance/TransportCompliance.cs b/src/Testing/Wolverine.ComplianceTests/Compliance/TransportCompliance.cs index 57bb57d70..f100cdcab 100644 --- a/src/Testing/Wolverine.ComplianceTests/Compliance/TransportCompliance.cs +++ b/src/Testing/Wolverine.ComplianceTests/Compliance/TransportCompliance.cs @@ -460,7 +460,9 @@ public async Task schedule_send() protected void throwOnAttempt(int attempt) where TException : Exception, new() { - theMessage.Errors.Add(attempt, new TException()); + // GH-3800: records the type NAME, not an instance -- an Exception does not round-trip + // through System.Text.Json, and this battery runs under serializers that use it. + theMessage.ThrowOnAttempt(attempt); } protected async Task afterProcessingIsComplete() diff --git a/src/Testing/Wolverine.ComplianceTests/ErrorHandling/ErrorCausingMessage.cs b/src/Testing/Wolverine.ComplianceTests/ErrorHandling/ErrorCausingMessage.cs index 2794134c8..a1cfcb663 100644 --- a/src/Testing/Wolverine.ComplianceTests/ErrorHandling/ErrorCausingMessage.cs +++ b/src/Testing/Wolverine.ComplianceTests/ErrorHandling/ErrorCausingMessage.cs @@ -10,7 +10,32 @@ public class ErrorCausingMessage /// public Guid Id { get; set; } = Guid.NewGuid(); - public Dictionary Errors { get; set; } = new(); + /// + /// Which attempt should throw what, keyed by attempt number and carrying the exception's + /// assembly-qualified type NAME rather than a live Exception instance. + /// + /// GH-3800. This used to be a Dictionary<int, Exception>, which only works + /// for serializers that can carry an arbitrary exception graph. System.Text.Json cannot: + /// under CloudEvents the dictionary arrived corrupted, the handler threw the wrong type, and an + /// exception-match rule could never fire — so with_cloud_events opted out of + /// will_move_to_dead_letter_queue_with_exception_match entirely. The hole was in this + /// shared harness, not in one transport: any transport wired .InteropWithCloudEvents() + /// and run through TransportCompliance inherited it. + /// + /// A type name is a string, so it survives every serializer we run the battery under. The + /// handler rehydrates it — see . + /// + public Dictionary Errors { get; set; } = new(); + public bool WasProcessed { get; set; } public int LastAttempt { get; set; } -} \ No newline at end of file + + /// + /// Records that should throw . + /// Kept here rather than at the call sites so the name/instance distinction lives in one place. + /// + public void ThrowOnAttempt(int attempt) where TException : Exception, new() + { + Errors[attempt] = typeof(TException).AssemblyQualifiedName!; + } +} diff --git a/src/Testing/Wolverine.ComplianceTests/ErrorHandling/ErrorCausingMessageHandler.cs b/src/Testing/Wolverine.ComplianceTests/ErrorHandling/ErrorCausingMessageHandler.cs index 81e727abb..e1c3ee177 100644 --- a/src/Testing/Wolverine.ComplianceTests/ErrorHandling/ErrorCausingMessageHandler.cs +++ b/src/Testing/Wolverine.ComplianceTests/ErrorHandling/ErrorCausingMessageHandler.cs @@ -8,16 +8,38 @@ public void Handle(ErrorCausingMessage message, Envelope envelope, AttemptTracke { tracker.LastAttempt = envelope.Attempts; - if (!message.Errors.ContainsKey(envelope.Attempts)) + if (!message.Errors.TryGetValue(envelope.Attempts, out var typeName)) { message.WasProcessed = true; return; } - if (message.Errors.TryGetValue(envelope.Attempts, out var ex)) + throw rehydrate(typeName); + } + + /// + /// GH-3800. The message carries an exception TYPE NAME, not an instance, so that the error + /// injection survives any serializer the compliance battery runs under — System.Text.Json + /// (and therefore CloudEvents) cannot round-trip an Exception, and used to hand this handler a + /// corrupted dictionary that made it throw the wrong type. + /// + /// A failure to resolve or construct the type is thrown loudly rather than swallowed: a + /// silently-wrong exception type is exactly the failure mode this replaced, and it presents as + /// an error-handling rule that mysteriously does not match. + /// + private static Exception rehydrate(string typeName) + { + var type = Type.GetType(typeName) + ?? throw new InvalidOperationException( + $"ErrorCausingMessage asked for exception type '{typeName}', which could not be resolved in the receiving process."); + + if (Activator.CreateInstance(type) is not Exception exception) { - throw ex; + throw new InvalidOperationException( + $"ErrorCausingMessage asked for exception type '{typeName}', which is not an Exception."); } + + return exception; } -} \ No newline at end of file +} diff --git a/src/Transports/Pulsar/Wolverine.Pulsar.Tests/WithCloudEvents.cs b/src/Transports/Pulsar/Wolverine.Pulsar.Tests/WithCloudEvents.cs index a3dc3d543..91a7fa6fa 100644 --- a/src/Transports/Pulsar/Wolverine.Pulsar.Tests/WithCloudEvents.cs +++ b/src/Transports/Pulsar/Wolverine.Pulsar.Tests/WithCloudEvents.cs @@ -50,15 +50,14 @@ public override void BeforeEach() [Collection("acceptance")] public class with_cloud_events : TransportCompliance { - // This test uses ErrorCausingMessage which contains a Dictionary. - // Exception objects don't serialize/deserialize properly with System.Text.Json, - // which CloudEvents uses internally. The test message's Errors dictionary gets - // corrupted during serialization, causing the wrong exception type to be thrown. - // This is a test infrastructure limitation, not a CloudEvents functionality issue. + // GH-3800 removed the CloudEvents-specific reason this test used to carry: ErrorCausingMessage + // now records an exception TYPE NAME rather than a live Exception, so it survives + // System.Text.Json and CloudEvents no longer corrupts it. // - // Skip rather than an empty body: an override that just returns Task.CompletedTask reports as a - // PASS, so the suite counted a test that never ran anything. GH-3763. - [Fact(Skip = "CloudEvents' System.Text.Json serialization corrupts ErrorCausingMessage's Dictionary, so the wrong exception type is thrown -- a test-infrastructure limit, not a CloudEvents defect.")] + // It stays skipped, but for the same reason as its two sibling fixtures below rather than a + // serialization one -- Pulsar has not implemented dead-letter routing. When GH-3797 lands this + // skip goes with the others, not separately. + [Fact(Skip = "Pulsar does not implement this compliance behaviour yet -- see GH-3797. Skipped rather than tagged Flaky: it fails deterministically, on every run, alone or in a suite.")] public override Task will_move_to_dead_letter_queue_with_exception_match() => Task.CompletedTask; // GH-3763. Deterministic failures shared with the other two Pulsar compliance fixtures -- the