Repository navigation
Report a member's own exception as a failed JSON serialize or deserialize [patch] - #203
Open
matt-edmondson wants to merge 1 commit into
Open
matt-edmondson wants to merge 1 commit into
matt-edmondson wants to merge 1 commit into
Conversation
…lize [patch] JsonSerializationProvider caught only the exception types System.Text.Json throws itself. A getter, setter or constructor runs through a compiled delegate, so whatever it throws (FormatException, NullReferenceException, NotImplementedException) escaped TrySerialize, Serialize and Deserialize, where the other three providers report false or default. Both catches now take every exception but OutOfMemoryException and AccessViolationException. The shared throwing-getter test threw InvalidOperationException, which the JSON provider happened to catch; it now throws FormatException, and a throwing-setter case holds all four providers to Deserialize's contract. Fixes #201 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW3CUXHTMtZvn4DEdny
|
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.
Fixes #201
What was wrong
JsonSerializationProvider.TrySerializeandDeserializecaught only the exception types System.Text.Json throws itself (JsonException,NotSupportedException,IOException,ArgumentException,InvalidOperationException). System.Text.Json runs a type's getters, setters and constructors through compiled delegates, so an exception they throw escapes unwrapped. As a result:TrySerializethrew where it should have returnedfalse.Serializethrew that exception rather than theInvalidOperationExceptionit documents.Deserializethrew where it should have returneddefault.Newtonsoft, YAML and TOML already report these cases as failures.
Change
OutOfMemoryExceptionandAccessViolationException, through a singleIsFailurefilter.Serializegoes throughTrySerialize, so it now throws its documentedInvalidOperationException.Tests
A_Throwing_Getter_Is_A_Failed_Serializegetter threwInvalidOperationException, which JSON already caught because of JsonSerializationProvider.TrySerialize and Deserialize throw InvalidOperationException instead of returning false/default #161. It now throwsFormatException. CA1065 is suppressed on that getter with a justification.A_Throwing_Setter_Is_A_Failed_Deserializeruns against all four providers with a setter that throwsArithmeticException.With the provider change reverted, the JSON case of each test fails (
System.FormatException: unreadable,System.ArithmeticException: 3 is unwritable). With the change, both pass.Full suite on Linux, net10.0: 1418 tests, 0 failed. The JSON provider builds in Release for every target framework with no warnings.
Generated by Claude Code