Runtime: System.Text.Json Add a feature that allows gathering all exceptions encountered when parsing json

[snappyfoo]

Newtonsoft.Json has a ErrorHandler call back that lets you inspect and continue on error. MVC uses this to report all the errors during JSON deserialization

• https://www.newtonsoft.com/json/help/html/P_Newtonsoft_Json_JsonSerializerSettings_Error.htm
• https://github.com/dotnet/aspnetcore/blob/master/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonInputFormatter.cs#L175
• https://github.com/dotnet/aspnetcore/blob/master/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonInputFormatter.cs#L224-L276

The ask is to consider doing something similar for S.T.J

---

Original issue text

Describe the bug

If my request model has a DateTime but an invalid date string is supplied in the JSON, an error is added to the ModelState, e.g. "The JSON value could not be converted to System.DateTime. Path: $.dateProperty | LineNumber: 1 | BytePositionInLine: 43."

No other validation messages are added because model binding appears to completely halt at this step. If I disable the automatic 400 response I can see the model is null.

This differs from Newtonsoft - invalid dates would add an error message but the model binding and validation process would continue. This allows you to catch and report all validation errors to the user. With STJ, if there are two invalid dates, the user can only find out about one at a time.

As the JSON as a whole is not malformed, it's just that the format of one of the values is wrong, I really think the model binding/validation should continue.

Further technical details

• ASP.NET Core version: 3.1
  .NET Core SDK (reflecting any global.json):
  Version: 3.1.101
  Commit: b377529961

Runtime Environment:
OS Name: Windows
OS Version: 10.0.18362
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\3.1.101\

Host (useful for support):
Version: 3.1.1
Commit: a1388f194c

.NET Core SDKs installed:
2.0.3 [C:\Program Files\dotnet\sdk]
2.1.102 [C:\Program Files\dotnet\sdk]
2.1.104 [C:\Program Files\dotnet\sdk]
2.1.200 [C:\Program Files\dotnet\sdk]
2.1.202 [C:\Program Files\dotnet\sdk]
2.1.402 [C:\Program Files\dotnet\sdk]
2.1.508 [C:\Program Files\dotnet\sdk]
2.1.701 [C:\Program Files\dotnet\sdk]
2.2.108 [C:\Program Files\dotnet\sdk]
2.2.301 [C:\Program Files\dotnet\sdk]
3.0.100 [C:\Program Files\dotnet\sdk]
3.1.101 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
Microsoft.AspNetCore.All 2.1.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.App 2.1.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 2.0.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.0.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.1.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

• Visual Studio 2019

[snappyfoo]

Digging around the source code a bit, the Newtonsoft deserializer doesn't stop on exceptions and it allows you to attach an error handler to deal with exceptions that occur during deserialization. The input formatter makes use of that error handler to add errors to the ModelState while the deserialization process continues to the end.

The STJ deserializer stops on the first exception, and while that exception is added to the ModelState (if an appropriate exception type) it means you cannot build up a complete ModelState of all the errors.

I would really like to see the Newtonsoft feature come to STJ / the STJ input formatter.

[pranavkm]

@ericstj would you consider a feature like this for System.Text.Json?

Newtonsoft.Json has a ErrorHandler call back that lets you inspect and continue on error. MVC uses this to report all the errors during JSON deserialization

• https://www.newtonsoft.com/json/help/html/P_Newtonsoft_Json_JsonSerializerSettings_Error.htm
• https://github.com/dotnet/aspnetcore/blob/master/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonInputFormatter.cs#L175
• https://github.com/dotnet/aspnetcore/blob/master/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonInputFormatter.cs#L224-L276

[ericstj]

I'm not sure we've considered such a feature before. @sharter @layomia what do you think?

[steveharter]

This was discussed early on when we evaluated Newtonsoft features for priority and this feature was discouraged, although the discussion did say that MVC uses it. @JamesNK @glennc

[JamesNK]

I'm not a fan of ErrorHandler. Attempting to resume after an error is too unreliable.

[snappyfoo]

Surely one can conceive of there existing a set of errors that can be considered safe to resume from, making it then a case of deciding which errors fall in this set. Newtonsoft may have tried to resume from too many different types of error, making it unreliable. Could STJ implement this feature more conservatively and therefore more reliably?

The particular DateTime example I've given here seems a good example of a resumable error. If I wanted to safely convert a string to a DateTime I would just use one of the TryParse methods - can't the deserialization process take a similar approach?

This extends to JsonConverters too, where there is no obvious direct means of reporting what I would class as a validation error. Ideally, as you're using a strongly typed language, you want a date and time property on your model to be a DateTime, therefore you rely on the string in the JSON to be converted at some point. But that means the point of conversion is really the only point at which you can validate the string format.

You might want to enforce a particular ISO 8601 compliant representation in requests, so you create a JsonConverter to parse the string with an exact format. If it fails, there is no real option other than to let the FormatException be caught by the MVC input formatter so it can be added to the model state. If the user has made multiple date format mistakes it seems reasonable that these all be reported at once, which isn't possible with the current approach of bombing out entirely.

A workaround might be to have this as a string property on your model and validate it after model minding, but not using a DateTime just to get around this doesn't seem like a reasonable workaround.

I'm considering DateTime in particular because it's the example I've come across but I think it could extend to other sorts of conversions too.

[snappyfoo]

Just wondering if this came up in “next sprint planning”?

[pranavkm]

@snappyfoo not as yet. That said, it's fairly unlikely MVC could \ would do anything if the feature does not exist in the formatter.

[snappyfoo]

@pranavkm Thanks for the update. Is there a more appropriate repo to raise the issue with the formatter, and do you think it’s worth doing that?

[pranavkm]

Let me transfer the issue to the runtime repo with a rewording. If they decide to add a feature, we could take advantage of it in MVC.

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label.