Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better enum deserialization exception fixes #1614 #1619

Merged

Conversation

sicklittlemonkey
Copy link
Contributor

@sicklittlemonkey sicklittlemonkey commented Jul 26, 2021

Avoid null exception, and return better error info for client/server enum mismatch.

Description

This simple suggestion fixes #1614.

Purpose

This pull request is a:

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

(I had a quick look, but didn't see a similar test that I could re-use for this case.)

  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Return better error info for client/server enum mismatch.
@stale
Copy link

stale bot commented Oct 30, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Oct 30, 2021
@alexeyzimarev alexeyzimarev merged commit 341b2ed into restsharp:dev Oct 31, 2021
@alexeyzimarev
Copy link
Member

@sicklittlemonkey The test Deserialization_Of_Undefined_Int_Value_Returns_Enum_Default started to fail after the fix, so it's a breaking change. Have you executed the test suite after you made the change? It was returning the "default" value, which is null.

@alexeyzimarev
Copy link
Member

I removed the trow and made it return the default value of a given type. It means that it won't throw the argument out of range exception, but it also won't fail with null reference exception. However, you'd not be aware that the enum doesn't contain the value it must have.

I am not sure how it works with something like System.Text.Json, it might just return null for the whole REST model. At least it is doing so by default when you, for example, put an invalid date to the DateTime field. Overall, I plan to kill SimpleJson and use System.Text.Json as a default serialiser since it's embedded in all the latest .NET versions anyway. The reason for SimpleJson to exist was to make RestSharp not have any dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enum deserialization exception is not very helpful
2 participants