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

Json.Deserialize System.Net.Mail; #76798

Closed
RallyTuning opened this issue Oct 9, 2022 · 10 comments
Closed

Json.Deserialize System.Net.Mail; #76798

RallyTuning opened this issue Oct 9, 2022 · 10 comments
Labels
area-System.Text.Json question Answer questions and provide assistance, not an issue with source code or documentation.

Comments

@RallyTuning
Copy link

Description

Deserialize a MailAddress is not possible.

Reproduction Steps

Just run this simple code in .Net 6.0

MailAddress MA = new MailAddress("[email protected]", "test");

var options = new JsonSerializerOptions { WriteIndented = true };

string strm = JsonSerializer.Serialize<MailAddress>(MA, options);
MailAddress AM = JsonSerializer.Deserialize<MailAddress>(strm)!;

Expected behavior

Serialize is OK. But not the Deserialize

Actual behavior

System.NotSupportedException: 'Deserialization of types without a parameterless constructor, a singular parameterized constructor, or a parameterized constructor annotated with 'JsonConstructorAttribute' is not supported. Type 'System.Net.Mail.MailAddress'.

Regression?

No response

Known Workarounds

No response

Configuration

  • .NET 6.0
  • Windows 10 Pro 21H2
  • x64
  • Visual Studio 2022 17.3.5

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 9, 2022
@ghost
Copy link

ghost commented Oct 9, 2022

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Deserialize a MailAddress is not possible.

Reproduction Steps

Just run this simple code in .Net 6.0

MailAddress MA = new MailAddress("[email protected]", "test");

var options = new JsonSerializerOptions { WriteIndented = true };

string strm = JsonSerializer.Serialize<MailAddress>(MA, options);
MailAddress AM = JsonSerializer.Deserialize<MailAddress>(strm)!;

Expected behavior

Serialize is OK. But not the Deserialize

Actual behavior

System.NotSupportedException: 'Deserialization of types without a parameterless constructor, a singular parameterized constructor, or a parameterized constructor annotated with 'JsonConstructorAttribute' is not supported. Type 'System.Net.Mail.MailAddress'.

Regression?

No response

Known Workarounds

No response

Configuration

  • .NET 6.0
  • Windows 10 Pro 21H2
  • x64
  • Visual Studio 2022 17.3.5

Other information

No response

Author: RallyTuning
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@danmoseley
Copy link
Member

Hello, did you try writing a converter?

https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/converters-how-to?pivots=dotnet-6-0

@eiriktsarpalis
Copy link
Member

This is by design -- types containing multiple public constructors without a JsonConstructor attribute are treated as ambiguous and are not supported for deserialization. I would recommend authoring a custom converter for the type.

@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Oct 10, 2022
@eiriktsarpalis eiriktsarpalis added the question Answer questions and provide assistance, not an issue with source code or documentation. label Oct 10, 2022
@danmoseley
Copy link
Member

@RallyTuning would it have helped you discover this if the error message had suggested a custom converter? Or perhaps you were already aware of them and were hoping for another solution?

@RallyTuning
Copy link
Author

RallyTuning commented Oct 10, 2022

@danmoseley no, i didn't have tried a custom converter, i just created a custom MailAddress class to "solve" the problem. But it is strange anyway that a built-in function to deserialize a class, didn't deserialize a simple built-in class (System.Net.Mail.MailAddress). For this reason i reported it.
About the last question, a more detailed message and a suggestion to build a custom MailAddress class would be better in this case. Considering i spent 4 hours trying to find the problem, thinking was my code the problem but then i discovered was the class.
So, in this case is better to specific "this class is not compatible with deserialization".

@eiriktsarpalis yes understood, but it's weird that this applies also with standard .Net classes.

@danmoseley
Copy link
Member

Thanks for the info. This isn't may area, so I defer to @eiriktsarpalis for his thoughts on the exception messages.

@eiriktsarpalis
Copy link
Member

but it's weird that this applies also with standard .Net classes.

Most types aren't designed with serialization in mind, and this applies to the vast majority of types shipped with the BCL. System.Text.Json tries to provide out-of-the-box support for a small number of frequently used types, but ultimately there are limits to how many can be supported: code size concerns, trimability concerns and the general impracticality of taking a dependency of every other library in the stack.

For most types, the serializer will make a best effort to derive a JSON contract but whenever this fails, using either a custom converter or a custom contract is your best way forward. This suggestion applies to both BCL and third-party classes.

a more detailed message and a suggestion to build a custom MailAddress class would be better in this case.

It's something we could consider for all error modes deriving from contract resolution, i.e. a sentence pointing users to either writing custom converters or contract customizers. Is adding links something that we do in exception messages?

@danmoseley
Copy link
Member

We have some exception messages and host/runtime messages that contain links. It's not pretty but if it really helps, it's not bad. They should be aka.ms links though to be stable.

@danmoseley
Copy link
Member

But in this case maybe key words to search for might be enough.

@RallyTuning
Copy link
Author

It's something we could consider for all error modes deriving from contract resolution, i.e. a sentence pointing users to either writing custom converters or contract customizers. Is adding links something that we do in exception messages?

A very long message with tons of information will just confuse the majority of users. The actual exception message is "ok", it just little bit ambiguous.

For most types, the serializer will make a best effort to derive a JSON contract but whenever this fails, using either a custom converter or a custom contract is your best way forward. This suggestion applies to both BCL and third-party classes.

I would ADD this message to the existing one, then it will contains the exception and a workaround.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json question Answer questions and provide assistance, not an issue with source code or documentation.
Projects
None yet
Development

No branches or pull requests

3 participants