[System.Text.Json] : More accurate error messages when failing to map fields or parameters

[mathieubergouniouxcab]

Background and motivation

CONTEXT:
The following issue is very common : a developer tries to deserialize some Json data into a class that has an explicit or explicit JsonConstructor, but the deserialization fails because System.Text.Json fails to map the data's property names either to an existing field in the class or to an existing parameter in the class' constructor.

It gives such errors at deserialization:
System.InvalidOperationException: Each parameter in constructor '[...]' on type '[...]' must bind to an object property or field on deserialization. Each parameter name must match with a property or field on the object. The match can be case-insensitive.

Examples of such issues can be listed with a Google query on this very tracker : site:https://github.com/dotnet/runtime/issues/ Each parameter in constructor must bind to an object property or field on deserialization

The root causes can be :

• a discrepancy in the name,
• a casing issue (e.g. camel case) -- that happens less often, because this can be configured
• a type mismatch.

That last one is the trickiest one : The mismatch can be because there's a discrepancy in nullability, or in typing -- because one is an interface (e.g. IEnumerable) while the other one is a class (e.g. List). Newtonsoft was more permissive in that regard and it confuses old developers.

PROBLEM:
The error message does not give a lot of explicit details on the exact offender.

It does give a list of expected constructor parameters types. For example:
...Each parameter in constructor 'Void .ctor(System.Nullable`1[System.Guid], string, Guid, string)' ...

That's good.

PROPOSED FIX:

• When the name cannot be matched, then Give the name of the offender, i.e. the name of the parameter that could not be bound.
  Could not bind parameter 'id' in constructor (Guid, string, string)
• When the type cannot be matched (but the name was found) then give BOTH the expected type and the found type, side by side.
  Parameter 'category' with type Guid could not be bound to property 'Category' with type Nullable<Guid>.
  the code performing those checks can be found here ;
  https://source.dot.net/#System.Text.Json/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs,572

API Proposal

API Usage

Alternative Designs

No response

Risks

No response

[ghost]

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

---

Background and motivation

CONTEXT:
The following issue is very common : a developer tries to deserialize some Json data into a class that has an explicit or explicit JsonConstructor, but the deserialization fails because System.Text.Json fails to map the data's property names either to an existing field in the class or to an existing parameter in the class' constructor.

It gives such errors at deserialization:
System.InvalidOperationException: Each parameter in constructor '[...]' on type '[...]' must bind to an object property or field on deserialization. Each parameter name must match with a property or field on the object. The match can be case-insensitive.

Examples of such issues can be listed with a Google query on this very tracker : site:https://github.com/dotnet/runtime/issues/ Each parameter in constructor must bind to an object property or field on deserialization

The root causes can be :

• a discrepancy in the name,
• a casing issue (e.g. camel case) -- that happens less often, because this can be configured
• a type mismatch.

That last one is the trickiest one : The mismatch can be because there's a discrepancy in nullability, or in typing -- because one is an interface (e.g. IEnumerable) while the other one is a class (e.g. List). Newtonsoft was more permissive in that regard and it confuses old developers.

PROBLEM:
The error message does not give a lot of explicit details on the exact offender.

It does give a list of expected constructor parameters types. For example:
...Each parameter in constructor 'Void .ctor(System.Nullable`1[System.Guid], string, Guid, string)' ...

That's good.

PROPOSED FIX:

• When the name cannot be matched, then Give the name of the offender, i.e. the name of the parameter that could not be bound.
  Could not bind parameter 'id' in constructor (Guid, string, string)
• When the type cannot be matched (but the name was found) then give BOTH the expected type and the found type, side by side.
  Parameter 'category' with type Guid could not be bound to property 'Category' with type Nullable<Guid>.
  the code performing those checks can be found here ;
  https://source.dot.net/#System.Text.Json/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs,572

API Proposal

API Usage

Alternative Designs

No response

Risks

No response

Author:	mathieubergouniouxcab
Assignees:	-
Labels:	api-suggestion, area-System.Text.Json
Milestone:	-

[eiriktsarpalis]

Marking up-for-grabs since it's an easy enhancement deriving from #44428.

[bennatpjose]

Hello, Can I attempt at fixing this? @eiriktsarpalis

[eiriktsarpalis]

Thank you for the offer, but I would recommend holding off for the few weeks, as we're currently busy trying to finalize .NET 8 development. Happy to assign you this issue if you're still available next month.

[vesnx]

I notice that the json constructor is not accepting the jsonpropeyNames

 [System.Text.Json.Serialization.JsonSerializable(typeof(ToDo))]
 [System.Text.Json.Serialization.JsonSourceGenerationOptions(
         GenerationMode = System.Text.Json.Serialization.JsonSourceGenerationMode.Metadata,
         IgnoreReadOnlyProperties = true,    
         IncludeFields = true,
         IgnoreReadOnlyFields = false
     )]
 public partial class ToDoJsonContext : System.Text.Json.Serialization.JsonSerializerContext
 {
 }
public class ToDo
{
    [JsonPropertyName("a")]
    private readonly int _rank;
    [JsonPropertyName("b")]
    private readonly string _task;

    [JsonConstructor]
    public ToDo(int a, string b)
    {
        _rank = a;
        _task = b;

    }
    [JsonIgnore]
    public int Rank => _rank;

    [JsonIgnore]
    public string Task => _task;
}

My simple test class can not round trip this; it can with Netwonsoft json.

internal class Program
{
    static async Task Main(string[] args)
    {
        /*test and work with a single entry*/

        var entry = new ToDo(Random.Shared.Next(1,int.MaxValue), "test a class");
        var json= JsonSerializer.Serialize<ToDo>(entry,ToDoJsonContext.Default.ToDo);
        Console.WriteLine(json);
        Debug.Assert(json.Contains("\"a\":"),"supposed to have \"a\": property");
        var copy= JsonSerializer.Deserialize<ToDo>(json,ToDoJsonContext.Default.ToDo);
        Debug.Assert(copy is not null, "Serialization failed for a single ToDo");
        Debug.Assert(copy.Rank == entry.Rank, $"supposed to have the same rank {copy.Rank} and the original has {entry.Rank}.");
        Debug.Assert(string.Equals(copy.Task, entry.Task, StringComparison.Ordinal),$"Texts is supposed to be the same however copy task = {copy.Task} and entry task = {entry.Task}");
  }
}

[konrad-jamrozik]

@eiriktsarpalis is this still up-for-grabs? Anything to be mindful of if trying to take a stab at it?

[eiriktsarpalis]

Sure, feel free to try it out.

[konrad-jamrozik]

Awesome, thx! If I will have something, I'll submit a PR and link to it here. But I am not doing this yet. Anybody else is welcome to work on this, too.

[SteveDunn]

Just came across this issue while trying to get this to work for Vogen:

public class Hash<T> : IEquatable<T>, IEnumerable<T> where T : IEquatable<T>
{
    private readonly ImmutableEquatableArray<T> _items;
    
    [JsonConstructor]
    public Hash(T[] items) => _items = items.ToImmutableEquatableArray();
...
}

I don't think this fix would work for collections though. Even though I have that attribute on the constructor, and I deserialize with:

SystemTextJsonSerializer.Deserialize<FileHash>([1,2,3])

I still get The collection type 'Vogen.Tests.Types.Hash``1[System.Byte]' is abstract, an interface, or is read only, and could not be instantiated and populated

[eiriktsarpalis]

@SteveDunn I think that's somewhat of a different issue, namely there's no real way to support deserialization for custom collection types that aren't mutable (a.k.a. implementing ICollection<T>.Add(T)). We're using this issue to track potential support for custom collections that apply the CollectionBuilderAttribute.

[vesnx]

Doesn’t seem to match my issue where a field with a name isn’t mapped in the constructor From: Eirik Tsarpalis ***@***.***> Sent: Monday, September 9, 2024 11:54 AM To: dotnet/runtime ***@***.***> Cc: VESNX SA ***@***.***>; Comment ***@***.***> Subject: Re: [dotnet/runtime] [System.Text.Json] : More accurate error messages when failing to map fields or parameters (Issue #88048) @SteveDunn <https://github.com/SteveDunn> I think that's somewhat of a different issue, namely there's no real way to support deserialization for custom collection types that aren't mutable (a.k.a. implementing ICollection<T>.Add(T)). We're using this issue <#82642> to track potential support for custom collections that apply the CollectionBuilderAttribute. — Reply to this email directly, view it on GitHub <#88048 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/A7GVYP3DWYTZUU3IWOWPZP3ZVVV2FAVCNFSM6AAAAAAZUKJWTWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZXGY3DANJVGE> . You are receiving this because you commented. <https://github.com/notifications/beacon/A7GVYP6IQYUQMPSFEKSOJA3ZVVV2FA5CNFSM6AAAAAAZUKJWTWWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTULKXPIO.gif> Message ID: ***@***.*** ***@***.***> >

[SteveDunn]

Thanks @eiriktsarpalis . Maybe that attribute could also be utilised in System.Configration for the work we did on binding to collections.