Carry an exception type name, not an Exception, through the compliance harness (GH-3800) - #3844
Merged
Merged
Conversation
…e harness (GH-3800) TransportCompliance injects errors with ErrorCausingMessage, which carried a Dictionary<int, Exception>. 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<int, string> of assembly-qualified type names, with a ThrowOnAttempt<T>() 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WHAuhdWS3XeAk16swV9G8m
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3800.
The defect, demonstrated
TransportComplianceinjects errors viaErrorCausingMessage, which carried aDictionary<int, Exception>.System.Text.Jsoncannot round-trip that. Run directly rather than inferred:It writes the exception's properties and rebuilds it as a bare
System.Exception. The type identity — the only thing an exception-match rule keys on — is gone. So the handler threw the wrong type, the rule could never fire, andwith_cloud_eventsopted out ofwill_move_to_dead_letter_queue_with_exception_matchentirely.The hole is in the shared harness, not in one transport. Anything wired
.InteropWithCloudEvents()and run throughTransportComplianceinherits it, silently. Pulsar is simply the only CloudEvents fixture in the battery, so it is the only place it surfaced.The change
ErrorsbecomesDictionary<int, string>of assembly-qualified type names, with aThrowOnAttempt<T>()helper on the message so the name/instance distinction lives in one place.ErrorCausingMessageHandlerrehydrates it.A type name is a string, so it survives every serializer the battery runs under. A name that cannot be resolved throws loudly rather than falling through to "no error" — a silently wrong exception type is precisely the failure this replaces, and it presents as an error-handling rule that mysteriously never matches.
What this does not fix
with_cloud_events.will_move_to_dead_letter_queue_with_exception_matchstays skipped, but for its siblings' reason rather than a serialization one: Pulsar has not implemented dead-letter routing (#3797). The skip message now matches the other three in that fixture, so when #3797 lands it goes with them instead of needing to be rediscovered.That is also why the new tests exist. Pulsar being the only CloudEvents fixture means this fix would otherwise ship with zero coverage anywhere — the one test that would have exercised it is blocked on an unrelated issue.
Verification
Four new tests in
CoreTests.ErrorHandling.error_injection_survives_serialization_3800pin the harness directly: type survives a round trip, distinct attempts keep distinct types, a no-error attempt still processes, and an unresolvable name fails loudly. The first would have failed on the old shape —Should.Throw<DivideByZeroException>against a deserializedSystem.Exception.CoreTestsErrorHandling— 214/214 existing, plus the 4 new.dotnet build wolverine.slnx -c Release— clean, which matters here becauseWolverine.ComplianceTestsis referenced by every transport test project.🤖 Generated with Claude Code
https://claude.ai/code/session_01WHAuhdWS3XeAk16swV9G8m