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

Document empty element behavior for DateTimeOffset and TimeSpan #56440

Open
StephenMolloy opened this issue Jul 28, 2021 · 1 comment
Open

Document empty element behavior for DateTimeOffset and TimeSpan #56440

StephenMolloy opened this issue Jul 28, 2021 · 1 comment
Labels
area-Serialization documentation Documentation bug or enhancement, does not impact product or test code
Milestone

Comments

@StephenMolloy
Copy link
Member

This is non-obvious. We need to do something about this. Either through documentation, or by letting it fail and have an AppContext switch to add this check. I'll let you decide which as it can be argued both ways whether we should be breaking people to alert them that they aren't getting the data they expect vs not introducing breaks and it's a philosophical more than technical question. But this behavior needs to be discoverable either through an exception or documentation. I'm going to approve this based on the presumption you will go the documentation route.

Originally posted by @mconnew in #55101 (comment)

The gist is that for most primitive types, an empty element is impermissible. Nulls are represented with xsi:nil and 'default' values are implied by the abscence of the element. Empty elements result in exceptions for most types... except DateTimeOffset and TimeSpan.

The reasoning for these two exceptions is because previous versions of XmlSerializer would unfortunately emit empty elements for these two types that it did not understand. When understanding of TimeSpan was added, the choice was made to not be noisy when encountering these empty elements which probably should not have been there in the first place, but they are, so lets just be graceful about it instead of throwing a tantrum. Or something. DateTimeOffset handling was added following the precedent of TimeSpan.

The argument could be made that we should throw exceptions when parsing these empty elements, as they likely are the result of an XmlSerializer that has silently lost data by writing an empty element. The flip side of that argument is that those empty elements aren't being written by the most recent XmlSerializer anymore, and being noisy about it is just going to annoy people who have already been working with a serializer that may or may not have been losing data... and they don't seem to mind. Sometimes "fixing a bug" is changing a behavior that people have come to depend on. I didn't make the TimeSpan change, but I think that is the principle being applied there.

Anyway, that's where we are. So we should document the special behavior of these two types.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jul 28, 2021
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@StephenMolloy StephenMolloy added documentation Documentation bug or enhancement, does not impact product or test code and removed untriaged New issue has not been triaged by the area owner labels Jul 28, 2021
@StephenMolloy StephenMolloy added this to the Future milestone Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Serialization documentation Documentation bug or enhancement, does not impact product or test code
Projects
None yet
Development

No branches or pull requests

1 participant