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