Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void Dispose()

protected void throwOnAttempt<T>(int attempt) where T : Exception, new()
{
theMessage.Errors.Add(attempt, new T());
theMessage.ThrowOnAttempt<T>(attempt);
}

protected async Task<EnvelopeRecord> afterProcessingIsComplete()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
using System.Text.Json;
using Shouldly;
using Wolverine;
using Wolverine.ComplianceTests.ErrorHandling;
using Xunit;

namespace CoreTests.ErrorHandling;

/// <summary>
/// GH-3800. The compliance battery injects errors through <see cref="ErrorCausingMessage"/>, and it
/// used to do so by carrying live <c>Exception</c> instances. <c>System.Text.Json</c> cannot
/// round-trip those, so any transport wired <c>.InteropWithCloudEvents()</c> 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.
///
/// <para>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.</para>
/// </summary>
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<int, Exception>.
var json = JsonSerializer.Serialize(message);
return JsonSerializer.Deserialize<ErrorCausingMessage>(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<DivideByZeroException>(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<DivideByZeroException>(() => handle(received, 1));
}

[Fact]
public void distinct_attempts_keep_their_own_exception_types()
{
var message = new ErrorCausingMessage();
message.ThrowOnAttempt<DivideByZeroException>(1);
message.ThrowOnAttempt<BadImageFormatException>(2);

var received = roundTrip(message);

Should.Throw<DivideByZeroException>(() => handle(received, 1));
Should.Throw<BadImageFormatException>(() => handle(received, 2));
}

[Fact]
public void an_attempt_with_no_error_is_processed_normally()
{
var message = new ErrorCausingMessage();
message.ThrowOnAttempt<DivideByZeroException>(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<InvalidOperationException>(() => handle(roundTrip(message), 1));
ex.Message.ShouldContain("Not.A.Real.Type");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,9 @@ public async Task schedule_send()

protected void throwOnAttempt<TException>(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<TException>(attempt);
}

protected async Task<EnvelopeRecord> afterProcessingIsComplete()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,32 @@ public class ErrorCausingMessage
/// </summary>
public Guid Id { get; set; } = Guid.NewGuid();

public Dictionary<int, Exception> Errors { get; set; } = new();
/// <summary>
/// Which attempt should throw what, keyed by attempt number and carrying the exception's
/// assembly-qualified type NAME rather than a live Exception instance.
///
/// <para>GH-3800. This used to be a <c>Dictionary&lt;int, Exception&gt;</c>, which only works
/// for serializers that can carry an arbitrary exception graph. <c>System.Text.Json</c> cannot:
/// under CloudEvents the dictionary arrived corrupted, the handler threw the wrong type, and an
/// exception-match rule could never fire — so <c>with_cloud_events</c> opted out of
/// <c>will_move_to_dead_letter_queue_with_exception_match</c> entirely. The hole was in this
/// shared harness, not in one transport: any transport wired <c>.InteropWithCloudEvents()</c>
/// and run through TransportCompliance inherited it.</para>
///
/// <para>A type name is a string, so it survives every serializer we run the battery under. The
/// handler rehydrates it — see <see cref="ErrorCausingMessageHandler"/>.</para>
/// </summary>
public Dictionary<int, string> Errors { get; set; } = new();

public bool WasProcessed { get; set; }
public int LastAttempt { get; set; }
}

/// <summary>
/// Records that <paramref name="attempt"/> should throw <typeparamref name="TException"/>.
/// Kept here rather than at the call sites so the name/instance distinction lives in one place.
/// </summary>
public void ThrowOnAttempt<TException>(int attempt) where TException : Exception, new()
{
Errors[attempt] = typeof(TException).AssemblyQualifiedName!;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/// <summary>
/// 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 — <c>System.Text.Json</c>
/// (and therefore CloudEvents) cannot round-trip an Exception, and used to hand this handler a
/// corrupted dictionary that made it throw the wrong type.
///
/// <para>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.</para>
/// </summary>
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;
}
}
}
15 changes: 7 additions & 8 deletions src/Transports/Pulsar/Wolverine.Pulsar.Tests/WithCloudEvents.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,14 @@ public override void BeforeEach()
[Collection("acceptance")]
public class with_cloud_events : TransportCompliance<PulsarWithCloudEventsFixture>
{
// This test uses ErrorCausingMessage which contains a Dictionary<int, Exception>.
// 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<int, Exception>, 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
Expand Down
Loading