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

Added note about breaking changes in 10.0.0 #467

Merged
merged 2 commits into from
Mar 2, 2023

Conversation

cmeeren
Copy link
Contributor

@cmeeren cmeeren commented Mar 1, 2023

Changing the default serializer is a breaking change and should be explicitly called out.

We were hit by outages in production because while I had carefully reviewed the changelog, this particular change was not mentioned, and therefore not tested. (Shame on us for not having a sufficiently comprehensive test suite to catch this, of course.)

@abatishchev
Copy link
Member

An outage in production? Oh no! I'm sorry to hear/cause this, my apologies for not calling it out clearly. I'll update the changelog accordingly, thanks for the contribution!

For my better understanding, can you explain what was the scenario and how it got broken?

@abatishchev abatishchev changed the title Add note about breaking change in v10 Added note about breaking changes in 10.0.0 Mar 2, 2023
@abatishchev abatishchev merged commit b68577a into jwt-dotnet:main Mar 2, 2023
@cmeeren
Copy link
Contributor Author

cmeeren commented Mar 2, 2023

I'm sorry to hear/cause this

Again, not your fault. Shame on us for not having a sufficiently comprehensive test suite to catch this. Btw if it were mentioned in the changelog, I would likely have checked a few more things manually and caught this, which is why I'm adding it. 🙂

For my better understanding, can you explain what was the scenario and how it got broken?

I parsed the token to Dictionary<string, obj> and cast relevant payload items to string when getting them from the dictionary. Not sure why I went with that solution originally, but it worked, because apparently Newtonsoft.Json deserializes to a suitable underlying type when using obj. However, System.Text.Json deserialized to JsonElement or something like that, which made the code fail due to invalid cast.

I now use the non-generic Decode overload and deserialize the JSON myself using my own serialization settings.

@cmeeren cmeeren deleted the patch-1 branch March 2, 2023 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants