Migrating from Newtonsoft.Json to System.Text.Json reduces quality of error messages

[SigmundVik]

We are in the process of migrating from Newtonsoft.Json to System.Text.Json following similar steps as described here.

Unfortunately System.Text.Json exception messages are of lower quality than the corresponding exception messages produced by Newtonsoft.Json. In our ASP.NET Core application this gets exacerbated for the validation errors and the end result is an API which is less user friendly. Please see this issue for more details.

In the following examples we try to deserialize a JSON string to this class:

    public class Model
    {
        public Guid Id { get; set; }
        public int Integer { get; set; }
        public string String { get; set; }
    }

For the different jsonString values listed below, this code was executed:

            try
            {
                Model model = Newtonsoft.Json.JsonConvert.DeserializeObject<Model>(jsonString);
            }
            catch (Newtonsoft.Json.JsonException oldEx)
            {
                // The value of oldEx.Message will be listed below.
            }

            try
            {
                Model model = System.Text.Json.JsonSerializer.Deserialize<Model>(jsonString);
            }
            catch (System.Text.Json.JsonException newEx)
            {
                // The value of newEx will be listed below.
            }

Example 1:

            string jsonString =
                @"{
                    ""Id"": ""00000000-0000-0000-0000-00000000000x""
                }";
=>
oldEx: "Error converting value \"00000000-0000-0000-0000-00000000000x\" to type 'System.Guid'. Path 'Id', line 2, position 64."
newEx: "The JSON value could not be converted to System.Guid. Path: $.Id | LineNumber: 1 | BytePositionInLine: 64."

Example 2:

            string jsonString =
                @"{
                    ""Integer"": 2x
                }";
=>
oldEx: "Input string '2x' is not a valid integer. Path 'Integer', line 2, position 33."
newEx: "'x' is an invalid end of a number. Expected a delimiter. Path: $.Integer | LineNumber: 1 | BytePositionInLine: 32."

Example 3:

            string jsonString =
                @"{
                    ""string"": 2 x
                }";
=>
oldEx: "After parsing a value an unexpected character was encountered: x. Path 'string', line 2, position 32."
newEx: "'x' is invalid after a value. Expected either ',', '}', or ']'. Path: $ | LineNumber: 1 | BytePositionInLine: 32."

All of these error messages are now harder to understand. In addition, the paths are less helpful and humans do not start counting line numbers from zero. It would be great if System.Text could produce exception error messages that were of the same quality as Newtonsoft.Json.

Also, with this JSON string:

            string jsonString =
                @"{
                    ""string"": 2
                }";

then there is no exception thrown for neither Newtonsoft.Json nor System.Text.Json, but the deserialized values differ: model.string is "2" for Newtonsoft.Json (this kind of leniency and the lack of a strict mode in Newtonsoft.Json is one the reasons we wanted to migrate in the first place). However, System.Text.Json leaves model.string as null, which is odd (I would have expected an exception here). In the ASP.NET Core application this case results in a validation error when using System.Text.Json.

[ahsonkhan]

Also, with this JSON string:

            string jsonString =
                @"{
                    ""string"": 2
                }";

then there is no exception thrown for neither Newtonsoft.Json nor System.Text.Json, but the deserialized values differ: model.string is "2" for Newtonsoft.Json (this kind of leniency and the lack of a strict mode in Newtonsoft.Json is one the reasons we wanted to migrate in the first place). However, System.Text.Json leaves model.string as null, which is odd (I would have expected an exception here). In the ASP.NET Core application this case results in a validation error when using System.Text.Json.

For this particular case, it is because there is no case-sensitive matching property within the Model class that exactly matches with the name string. S.T.J is case-sensitive by default. It would be equivalent to passing in something like "{\"foo\": 2}". That property is just ignored.

You could annotate your model with the expected name, or add a case-insensitive option to the serializer, and/or use a built-in/custom naming policy.

public class Model
{
    public Guid Id { get; set; }
    public int Integer { get; set; }
    [JsonPropertyName("string")]
    public string String { get; set; }
}

OR

var options = new JsonSerializerOptions { PropertyNameCaseInsensitive = true };
Model model = System.Text.Json.JsonSerializer.Deserialize<Model>(jsonString, options);

OR

var options = new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase };
Model model = System.Text.Json.JsonSerializer.Deserialize<Model>(jsonString, options);

And then you get the exception:

System.Text.Json.JsonException: The JSON value could not be converted to System.String. Path: $.string | LineNumber: 1 | BytePositionInLine: 31.
 ---> System.InvalidOperationException: Cannot get the value of a token type 'Number' as a string.

[osexpert]

humans do not start counting line numbers from zero

Good point. But I don't think machines count line numbers from zero either:-D Looks like a bug to me.

[ghost]

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of the experimental issue cleanup initiative we are currently trialing in a limited number of areas. Please share any feedback you might have in the linked issue.

[bart-degreed]

This is just great. Even today, STJ lacks many critical features from Newtonsoft and is still full of bugs and design flaws. Lots of issues were opened by the community, which often got a response along the lines of "we'll wait and see if more people need this." But you forgot that those issues get auto-locked or auto-closed, so no one can upvote them anymore today. We're all very disappointed with .NET 6, which should have addressed the big STJ issues above all else. Traditionally, it always takes Microsoft a few versions before getting it right, so the masses wait for that. You should have gotten it right by now. If we could upvote, you'd be shocked by the number of developers struggling with adopting STJ in real projects. But now all innovation has stopped in Newtonsoft and we are stuck with an inferior replacement. Thanks a lot!

[eiriktsarpalis]

From the OP I can deduce the following actionable suggestions:

• Change the lineNumber and bytePositionInLine reporting from 0-based to 1-based. I think that's doable however I suspect it would be a breaking since this merely reflects the API of JsonException which clearly states that these numbers are 0-based. I think there's an argument to be made about these figures needing to be human readable, but we still need to be careful about this. cc @ahsonkhan @bartonjs @layomia for thoughts.
• Have Utf8JsonReader include the raw JSON value in the exception message whenever a FormatException is thrown. I get that it can be useful when debugging code, but from a security perspective it can sometimes be considered harmful (e.g. because PII from prod might leak to an insecure log aggregator). I'm not sure what our policy is from the .NET perspective, and if eliding raw content from our FormatExceptions is intentional. cc @GrabYourPitchforks

[NinoFloris]

Another actionable thing would be to have 'public' messages on JsonException (probably as a new property). Today it mentions 'change the reader options' (trailing comma error), full type names and other such internal concerns. It's hardly feasible to have to manually match every json exception message and build a nice one from constituent parts again.

Asp.net core mvc model binding is also notorious for spewing internal details like that.
I can't ship a public api that responds with those kind of messages to clients (not end users). While recovering the diagnostic value another way amounts to writing your own serializer. That or painstakingly finding all relevant messages to match on and hoping they don't change.

Having a nullable property for a public message would help a lot, we'll defer to a generic message if it's null.

[danmoseley]

@blowdart the issue of policy around sensitive info in exception messages again.

[bkoelman]

How about adding a boolean property JsonSerializerOptions.IncludeSensitiveDataInExceptions (false by default), that enables application developers to get more details? This is quite common to provide.

Then anyone can decide when its appropriate to turn it on, for example:

• wrapped in a #if DEBUG
• based on app.Environment.IsDevelopment() or ASPNETCORE_ENVIRONMENT environment variable
• based on externally stored configuration, so it can be turned on in prod for a short amount of time (in extreme scenarios)