JsonInputFormatter, ModelState and error response message.

[MicahZoltu]

I have been digging through the aspnet/mvc issues backlog as well as StackOverflow/Google and I have learned that when a client calls a controller/action that uses [FromBody], the JsonInputFormatter is used rather than the MVC Model binding stuff. This results in ModelState.IsValid returning true even when the model state isn't valid, because the actual model validation occurs as part of the input formatter processing, after the ModelState is already populated. This, of course, is problematic because it doesn't align with other properties like FromRoute where deserialization/parsing errors are included in the ModelState.

I can accept that this is how things work though and move on. The problem I am now running into is that I can't figure out any way to surface the JSON deserializer error in the BadRequest response. Looking at https://github.com/aspnet/Mvc/blob/master/src/Microsoft.AspNet.Mvc.Formatters.Json/JsonInputFormatter.cs#L105, I can see that the ModelState is actually updated (Yay!), unfortunately because the Input Formatter proceeds with surfacing the error through its failure mechanisms as well (https://github.com/aspnet/Mvc/blob/master/src/Microsoft.AspNet.Mvc.Formatters.Json/JsonInputFormatter.cs#L133) there is no opportunity for user code to actually look at the ModelState and report the error it has in the usual ways (HttpBadRequest(ModelState)).

What is the ASP.NET Core 1.0 release plan for this? Unless I am missing something, it seems that I can't really leverage input formatters unless I am okay with never giving a useful response to my users. If I want to give them information to help troubleshoot what is wrong with their request I have to take in a [FromBody] String and deserialize by hand inside each of my actions, or build my own InputFormatter that doesn't actually return an error on failure and instead just populates the ModelState (as JsonInputFormatter does), then claim success.

Also, are there any workarounds short of writing my own input formatter that doesn't behave correctly (always returns success with Model errors)? I also tried adding an exception filter but it doesn't appear to catch InputFormatter problems, which isn't surprising as the ExceptionFilter is much later in the pipeline.

[rynowak]

This results in ModelState.IsValid returning true even when the model state isn't valid,

If this is the case this is a very serious bug that would break everyone's app. Have a repro for this?

the actual model validation occurs as part of the input formatter processing, after the ModelState is already populated

This isn't exactly accurate, model validation (data-annotations attributes) is a separate pass that takes place after formatters/model-binding run.

There is some input validation that takes place during formatters/model-binding - this includes things like parsing JSON and converting strings to various types.

What is the ASP.NET Core 1.0 release plan for this? Unless I am missing something, it seems that I can't really leverage input formatters unless I am okay with never giving a useful response to my users

Can you provide a repro and example of the kinds of problems you're seeing? It's definitely the intent that errors from a formatter would work the same as errors from old-school model binding.

[MicahZoltu]

"Microsoft.AspNet.Mvc": "6.0.0-rc1-final"

public class MyController : Controller
{
    public class MyModel
    {
        [JsonProperty(Required = Required.Always)]
        public String Foo { get; set; }
    }

    [HttpPut]
    [Route("")]
    public async Task<IActionResult> GetWee([FromBody] MyModel myModel)
    {
        if (!ModelState.IsValid)
            return HttpBadRequest(ModelState);

        return Ok("Wee!");
    }

    public class MyOtherModel
    {
        [BindRequired]
        public String Foo { get; set; }
    }

    [HttpPut]
    [Route("other")]
    public async Task<IActionResult> GetOther([FromBody] MyOtherModel myModel)
    {
        if (!ModelState.IsValid)
            return HttpBadRequest(ModelState);

        return Ok("Wee!");
    }
}

For both requests, submit with a payload of {}.

The first action (GetWee) will not hit the HttpBadRequest. It will also not give you a useful error message in the 400 response. No action code will be executed, so there is no opportunity to improve the error or check the ModelState for the error (which should be there). Instead it will return the following:

{
  "": [
    "The input was not valid."
  ]
}

The second action (GetOther) will have a valid ModelState because the [BindRequired] attribute is used instead of the JsonProperty attribute. This will result in the model being valid and the action sending back a 200 response with:

Wee!

[dougbu]

@Zoltu have you debugged this scenario? From your description, it sounds like everything is working as expected. In particular, the JSON response for the first case looks like the expected ModelStateDictionary content given an error on the root ("") object.

The [BindRequired] attribute isn't relevant in the second (GetOther) case because that attribute is part of the model binding system. That system never sees the Foo property.

I suspect the [Required] attribute is what you're looking for. That's executed as part of MVC validation and will give you more control over the error message.

[MicahZoltu]

When I drop into a debugger and look at the exception thrown by the JSON deserializer it has a much better message. The message on the exception says something along the lines of, "Expected String Foo" and gives a line/column number of where it expected a Foo and didn't find one. In this case, since the object is so simple that is line 1 character 2 I think (I don't have the code in front of me).

@dougbu Are you suggesting that I can put [Required] on the property instead of [JsonProperty(Required = Required.Always)] and I will get a model validation failure if Foo is null? I'll try this tomorrow, as that would resolve my current issue. However, I still think there is significant value in exposing the JsonDeserializer exception message to the client since that will handle scenarios other than just missing, such as syntax errors and such.

[dougbu]

exposing the JsonDeserializer exception message to the client

Exception messages are scary because they often expose implementation details. MVC logs those details for developer use but does not expose them to untrusted clients.

@blowdart anything to add?

@dougbu Are you suggesting that I can put [Required] on the property instead of [JsonProperty(Required = Required.Always)]

Yup.

[MicahZoltu]

I think JSON.NET deserialization exceptions are always safe to show the the client, but I can appreciate that such things shouldn't be assumed.

It's there some other way to expose better error messages to the user? The crux of this issue is that it is currently difficult to write a quality public API with asp.net core because it is hard to get quality error messages back to the user.

[blowdart]

Nothing to add. I'd be ok with someone being able to configure detailed exceptions if they wanted to shoot themselves in the foot, but it can't be the default. In binding shouldn't these things show up as model state errors, or does the exception happen before then?

[dougbu]

In binding shouldn't these things show up as model state errors

@blowdart these Exceptions do show up as errors in the ModelStateDictionary. However errors can contain a message string or an Exception. If they contain no message, SerializableError substitutes a generic message -- The input was not valid. as shown a few messages up.

W.r.t. configuring detailed exceptions in a response, someone could write their own SerializableError class and return one of those in a BadRequest payload.

[MicahZoltu]

In binding shouldn't these things show up as model state errors, or does the exception happen before then?

@blowdart The exception occurs before the action so there is no opportunity for user-code to see/handle the error. Looking at the code, it appears that they are added to the ModelState as an error, but unless I'm missing something that doesn't do any good if the exception prevents the action from executing.

someone could write their own SerializableError class and return one of those in a BadRequest payload

For the same reason as I just mentioned, I don't believe this is currently possible unless there is some way to stop the JsonInputFormatter from throwing and bypassing all user code?

[dougbu]

@Zoltu your action is executing. It calls HttpBadRequest(ModelState) and that results in the JSON response you showed above. Set a breakpoint in the action and continue after the JSON deserializer throws if you need to confirm this.

[MicahZoltu]

Hmm, I thought I did that and didn't see the breakpoint hit. I'll try again this evening though, it could be that I am mis-remembering. Thanks!

[rynowak]

In particular, the JSON response for the first case looks like the expected ModelStateDictionary content given an error on the root ("") object.

Putting errors on the 'root' object is a bug in RC1 that's been fixed. This can have an effect on validation when you need to validate multiple parameters, and might describe some of the badness you're seeing.

There's a somewhat related discussion here: #3743 (comment)

If you're seeing issues where model validation isn't triggered for other parameters, you might want to use the [ModelBinder(Name = "whatever")] workaround described in the link

[MicahZoltu]

While not ideal, I was able to take the suggestions here and get a better error message out with the following:

if (!ModelState.IsValid)
{
    var error = ModelState.SelectMany(x => x.Value.Errors).First();
    if (error.ErrorMessage != null && error.ErrorMessage != String.Empty)
        return HttpBadRequest(error.ErrorMessage);
    else if (error.Exception?.Message != null)
        return HttpBadRequest(error.Exception.Message);
    else
        return HttpBadRequest(ModelState);
}

I can probably figure out a relatively clean way to throw that into an extension method and then JSON validation failure responses will be improved across my application as long as I use it everywhere instead of HttpBadRequest(ModelState).

I also tried switching over to using [Required] instead of [JsonProperty(Required = Required.Always)]. This solves the problem of null checking, but I still don't get quality error messages when the payload contains malformed JSON, so I'll probably stick with the first solution (though I may switch to using Required for brevity and to make the wire model not tightly coupled with JSON).

I still think there would be value in making the JsonInputFormatter populate the ErrorMessage field with a sanitized error message. The most naive approach would be to just catch JsonReaderException explicitly and use the exception.Message off of it. If it turns out (research required) that JsonReaderException can sometimes be thrown for things other than parsing errors then more advanced measures could be taken to ensure that implementation details weren't leaking out to the client, perhaps by inspecting the message for certain patterns.

At this point I suppose this may have turned into a feature rather than a bug and I'm not sure if this is the right place to track such things so feel free to close if it isn't.

[rynowak]

At this point I suppose this may have turned into a feature rather than a bug and I'm not sure if this is the right place to track such things so feel free to close if it isn't.

I think this is quality feedback in an area we know that we want to improve (error reporting for JSON/API clients). I appreciate your patience and thoughts on this.

[markgarcia]

I think there should be a way to customize the resulting JSON message generated by calling BadRequest(ModelState) (or is there?). It's particularly annoying to detect errors when using API clients that bind the JSON result to a class object with members having corresponding names as with the expected JSON result. I have a particular problem when using RestSharp in that it (correctly) throws cast errors because the validation errors are contained in the same field names as the expected successful data. I got to work-around this by checking first if the server returns a 4XX, but that is just clumsy to handle in client code (what if the errors actually successfully binds to the expected result?).

A good way would be to wrap them in an error field (e.g. { "error" : { validation errors here } }, but I guess that would be a huge breaking change. A way to customize this would be a much much better solution.

[rynowak]

@markgarcia - there's nothing magic about BadRequest(...) under the covers it just constructs a new SerializableError - which is really just a Dictionary<> https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.Core/SerializableError.cs

I'd suggest overriding the requisite method on ControllerBase and coming up with a new implementation that does what you want.

[shawnmclean]

Should the BadRequestObjectResult also use the same settings for the json serialization? For eg. a model with the Title prop is required will have pascal casing json response as below:

{
  "Title": [
    "The Title field is required."
  ]
}

Should be:

{
  "title": [
    "The Title field is required."
  ]
}

Request and Response payloads works with camel casing.

[Luis-Palacios]

is there a simple way to convert your response object when using context.Result = new BadRequestObjectResult(context.ModelState); on a filter to lower camel case? I'm getting pascal case and I don't know how to change this

[alexsandro-xpt]

@LRPalacios Good question, there are way?

[alexb5dh]

@LRPalacios , if you mean dictionary keys, they're serialized pascal case by default. You can change this as described here.

services.AddMvc().AddJsonOptions(x =>
{
    x.SerializerSettings.ContractResolver = new DefaultContractResolver
    {
        NamingStrategy = new CamelCaseNamingStrategy
        {
            ProcessDictionaryKeys = true
        }
    };
});

[Luis-Palacios]

@alexb5dh thanks, I'm going to try it

[rynowak]

It sounds like the important takeaway from this is that we don't have a good default error message when the JSON is invalid. This is definitely something we should improve for 2.1.0

[rynowak]

@SteveSandersonMS assigned to you as a companion to #4862

When you get an input error in JSON it lands here: https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.Formatters.Json/JsonInputFormatter.cs#L155

Assume the usage of [ApiController] which implies that validation errors get sent back to clients by default

A few issues:

• Are we providing good error messages at all when JSON is invalid or not convertible to the objects
• Is the approach right for returning validation messages - is the right data structure (string, string[])[] or something else
• If the approach is right, can we capture the keys when we have JSON and use those instead of property paths
• Anything else you can think of

[SteveSandersonMS]

OK, I've been investigating what happens in different JSON [FromBody] scenarios, assuming you return BadRequest(ModelState):

1. Missing or empty request body - works fine - we return {"":["A non-empty request body is required."]}

2. Malformed JSON - not ideal. We return {"someProperty":["The input was not valid."]}.

   • It's good that we pass through the Path data from Json.NET's JsonReaderException, but not so good that we don't pass through the LineNumber or Position data.
   • Note that the ModelStateDictionary key here comes from Path, which is the incoming JSON key, not any associated .NET property (which in general we don't know about here).
   • We are (by design) discarding potentially valuable information from the JsonReaderException's Message such as "Unexpected end when reading JSON" or "Invalid character after parsing property name. Expected ':' but got: \"". There's no metadata to let us differentiate these scenarios other than the free-text Message value itself.

3. MVC model validation error - works OK - we return {"Name":["The Name field is required."]} or similar.

   • Note that the ModelStateDictionary key here comes from the .NET property, not the incoming JSON key. This is because model validation happens after JSON parsing is finished, so by that time we don't know anything about the original raw JSON data or [JsonProperty(PropertyName = "someName")] etc.

4. Json.NET validation error (e.g., missing [JsonProperty(Required = Required.Always)] property) - pretty bad - we return things like {"":["The input was not valid."]}, entirely ignoring the Member property on the error context, so we don't reveal how the property was invalid, nor even which property it was... 😕

   • Note that the ModelStateDictionary key here comes from Path, which Json.NET sets as the incoming JSON key at the parent level rather than the level where the validation failed. You need to also see the Member (which we don't currently expose) to know which property the failure occurred on.
   • Note that Json.NET supplies a JsonSerializationException for this (not a JsonReaderException) so although we have Path and Member info, we don't have LineNumber or Position.

5. Unrecognised property supplied - this is fine, we just ignore it (assuming it's legal JSON)

6. Unconvertible value for property - this is not too bad, as we return {"age":["The input was not valid."]} or similar.

   • We can't say why the input was invalid because there's no metadata about that. The only info about this is in the free-text exception Message property.
   • Sometimes Json.NET uses a JsonReaderException for this (e.g., trying to convert a nonnumeric string to a number), and sometimes it uses a JsonSerializationException (e.g., trying to convert a number input to an object type). The distinction doesn't really matter to us, although only JsonReaderException gives LineNumber/Position metadata outside its Message string.
   • The ModelStateDictionary key here comes from Path, which is the incoming JSON key, not any associated .NET property (which in general we don't know about here). Unlike in the malformed JSON case, Json.NET sets Path to be the complete path including the property that failed (not just the path to the parent level), so the key we already use is good, but we could still expand the info further and specify the LineNumber/Position if we have it.

Proposal

A. Make it simple to opt into exposing the raw exception message to clients

Example: return BadRequest(ModelState, exposeExceptionMessages: true);

This exposes maximum info to clients, including details from Json.NET about why properties were invalid or what aspect of JSON parsing failed. This would work by changing SerializableError so that when it populates itself, if string.IsNullOrEmpty(error.ErrorMessage) but !string.IsNullOrEmpty(error.Exception?.Message), then we use error.Exception.Message.

B. By default, add more info to the generic error message

Instead of just defaulting to "The input was not valid.", we could append whatever combination of Path, Member, LineNumber, and Position information we have. AFAIK this is all safe to expose, because it's all already known by the client who sent the request (the Path and Member come from incoming JSON keys). @blowdart - please let us know if you think there are any issues with that.

This would work by expanding ModelStateDictionary's existing logic for detecting specific Exception subclasses so that it also knows about JsonReaderException and JsonSerializationException, and for these, populating the ErrorMessage property on the ModelError (though somehow we have to know not to do this in case [A] where you actually want to surface the raw exception message).

@rynowak's questions

• Are we providing good error messages at all when JSON is invalid or not convertible to the objects

Sometimes yes, sometimes no. Examples above.

• Is the approach right for returning validation messages - is the right data structure (string, string[])[] or something else

This data structure does match the reality of what we are doing. Plus it just so happens to be precisely what some third-party components such as React-Formsy are looking for (so they can make validation messages appear at the right place in the UI). I don't have any reason to propose changing this.

At some point we might want to support the HTTP Problem Details spec. This is a different-looking response structure, but it's possible to produce valid problem details JSON objects using only the information we normally have in a BadRequestObjectResult. So arguably it's a different serialization format for BadRequestObjectResult rather than a different way of populating a BadRequestObjectResult. Whether we do that or not I don't mind, but it should at least be separate from this issue, as we'd want to improve the existing scenarios without forcing people to adopt this new response format as well. I've not yet heard of any customers asking for Problem Details, nor do I know of any existing client technologies that make use of it, so it's unclear that the spec will ever gain traction and be a desirable feature.

• If the approach is right, can we capture the keys when we have JSON and use those instead of property paths

We already do use the JSON keys for all the errors that come from Json.NET. We only use .NET property names for model validation errors, by which time we don't know about JSON keys.

• Anything else you can think of

Mainly I think we just need to expose all the information that's safe to expose by default (Path, Member, LineNumber, Position), because the humans receiving this info will be in a much better position to know what went wrong. I don't think we should change which ModelStateDictionary keys we use in any case because (1) there's not much to improve here - not seeing any benefits, and (2) any changes would be extremely subtle and super annoying breaking changes, or require some new option that would be very hard to discover anyway.

Plus also give an easy and reasonably-discoverable solution to the people asking for the raw message info from Json.NET as per proposal [A].

[blowdart]

I think as long as you don't reflect back the actual input, and it's just positional information it'd be fine.

[SteveSandersonMS]

it's just positional information it'd be fine.

It would be positional information (LineNumber/Position integers) and the Path/Member strings, which are from the JSON keys supplied by the client in the request body.

[blowdart]

I guess if someone is silly enough to output the error message without encoding they're doing work to shoot themselves in the foot, so ... sure, fine by me.

[dougbu]

@SteveSandersonMS is anything in your description different after @kichalla's work in 83c3ac6 (adding InputFormatterExceptionModelStatePolicy and properties of that type) or @pranavkm's various commits (e.g. 7f21449) related to serializing ModelState according to the HTTP Problem Details spec?

[rynowak]

is anything in your description different after @kichalla's work in 83c3ac6

Yes, this is a totally different workitem. Kiran's bug dealt with formatters swallowing totally unrelated exceptions such as TypeLoadException.

This work item is about improving the user experience for bona-fide input validation errors. The hope is that returning the model state errors to a user will make it possible to troubleshoot your problems as client of an API

[rynowak]

Great notes @SteveSandersonMS !

Some thoughts:

Make it simple to opt into exposing the raw exception message to clients

I'm not crazy about this idea.

If the developer has to make a code change to turn this on, then it's not on by default, then clients have to contact the developer to ask for it. At that point it might as well be logging. Unless we're willing to make something like this on by default for new code ([ApiController]) then it's a non-starter to me.

We also have a really long history of conflict, and building bad features that try to turn all exceptions into client errors. I think that if something like this is needed - then it means your second proposal is a failure.

we could append whatever combination of Path, Member, LineNumber, and Position information

IMO we should find a way to make this part of ModelStateDictionary. MSD really only works when you consider value-provides and classic MVC model binding. I think we need to try and capture all of this info in the same data structure.

B. By default, add more info to the generic error message

1,000,000 times Yes!

This would work by expanding ModelStateDictionary's existing logic for detecting specific Exception subclasses

I'm curious to know if you have any thoughts about how this will work with layering. We have a new InputFormatterException that is intended to be used only for safe exceptions. We might need to wrap JSON.NET's things in this.

Plus it just so happens to be precisely what some third-party components such as React-Formsy are looking for

OK great, that's a good data point. What I was wondering was, do the 'keys' matter? It sounds like they do.

At some point we might want to support the HTTP Problem Details spec.

We do.

The E2E for this feature is to use [ApiController] on the controller. You should see a 'Problem Details' with these validation errors as a response without your action being invoked.

[SteveSandersonMS]

Re: opting into exposing exception messages

If the developer has to make a code change to turn this on, then it's not on by default, then clients have to contact the developer to ask for it. ... I think that if something like this is needed - then it means your second proposal is a failure.

I'm fine with just not doing this, assuming it's possible for developers to expose the exception message on their own by writing a reasonably simple result filter (or similar). Or maybe, as per below, we might be able to just expose this info by default.

IMO we should find a way to make this part of ModelStateDictionary. MSD really only works when you consider value-provides and classic MVC model binding. I think we need to try and capture all of this info in the same data structure.

Strictly speaking this information already is in the ModelStateDictionary, in that the ModelError objects have an Exception property. If the value is a JsonReaderException or JsonSerializerException, you can get the Path, LineNumber, etc., from them.

Are you thinking we should turn Path, LineNumber, etc. into native concepts on the ModelError so they can get populated by JsonInputFormatter? At first I was thinking that was over-specific to JSON, but then I guess the majority of input that arrives via HTTP will be in a text-based format, so those concepts are quite widely applicable.

I don't know what use cases there are for these extra properties on ModelError if we find a way to expose the info to clients by default anyway.

We have a new InputFormatterException that is intended to be used only for safe exceptions. We might need to wrap JSON.NET's things in this.

Yes, we could use InputFormatterException as a signal that the exception.InnerException.?Message ?? exception.Message is safe to return to clients (and if JsonReaderException etc got wrapped in InputFormatterException, we'd then return its full Message string by default, which includes not only Path etc but also free-text details about why deserialization failed).

It looks like all the existing default usages of InputFormatterException would be safe to expose - the only existing usages are in XmlDataContractSerializerInputFormatter and XmlSerializerInputFormatter when specific deserialization errors occur.

As for whether we can add all JSON deserialization errors into this privileged group: AFAIK this would be fine in practice today. Theoretically, a future version of Json.NET could start putting sensitive info into its exception messages, but theoretically so could System.FormatException etc. What do you think - can we do this? Are we looking for sign-off from Barry or have you already got agreement on this?

If we decide we can't do this for Json.NET exceptions by default, we could instead put special-case logic into JsonInputFormatter to detect JsonReaderException etc., and from that construct an InputFormatterException whose Message contains info from the Path, LineNumber, etc. properties.

I guess this would count as a breaking change since people might have existing logic that looks in ModelStateDictionary for entries with exceptions of type JsonReaderException etc. If we were determined to avoid this, we could instead add an IsSafeException flag to ModelError, or even trigger the change based on whether you're an ApiController or not (though I don't like that, as it means the whole framework becomes more fragmented and hard to understand forever).

The E2E for this feature is to use [ApiController] on the controller. You should see a 'Problem Details' with these validation errors as a response without your action being invoked.

And this happens by default because of @pranavkm's work on serializing ModelStateDictionary this way, does it?

[rynowak]

Are you thinking we should turn.... At first I was thinking that was over-specific to JSON

It's an idea I had 💡 - ModelState captures validation error concepts today, we could add additional concepts that allow us to track input errors. It doesn't seem stricly necessary, but I'm open to it if it provides value. Basically I think that ModelState in its current form satisfies about 60% of the requirements we have.

What do you think - can we do this? Are we looking for sign-off from Barry or have you already got agreement on this?

We should bring this up with him again and try to get a real sign off. We've had some hallway discussions about it, but I want to make sure it doesn't take anyone by surprise.

I guess this would count as a breaking change since people might have existing logic

I'm still believe it's the kind of change we'd be willing to make in a minor release. We already put a generic error in the model state when we can't process the body, in this case we'd be adding a specific error.

And this happens by default because of @pranavkm's work on serializing ModelStateDictionary this way, does it?

Yes, [ApiController] add a filter that responds for you on model state errors

[dougbu]

Fodder for

What do you think - can we do this?

Json.NET includes details of the request content in a few JsonReaderExceptions it throws today e.g. here:

throw JsonReaderException.Create(this, "Could not convert string to integer: {0}.".FormatWith(CultureInfo.InvariantCulture, s));

The above is usually just a single character or value. But, I haven't attempted to find everything.

Json.NET also includes potentially-sensitive server configuration details e.g. here:

throw JsonReaderException.Create(this, "The reader's MaxDepth of {0} has been exceeded.".FormatWith(CultureInfo.InvariantCulture, _maxDepth));

[SteveSandersonMS]

@dougbu Thanks for that info! This does need to get fed into the decision about whether Json.NET exception messages are exposed by default or not.

@rynowak It sounds like a plan is settling. To summarise:

• For ModelStateDictionary entries whose Exception is an InputFormatterException, change the fallback error message string that we report from "The input was not valid." to exception.Message. That is, we regard InputFormatterException as being a signal that its message is safe to return to clients.
• Change JsonInputFormatter so that the ModelStateDictionary entries it adds have exceptions of type InputFormatterException.
  • If we decide we can expose Json.NET exceptions directly, then the Message property will be taken directly from the JsonReaderException/JsonSerializerException's Message property.
  • Or if we decide we can't expose Json.NET exceptions directly, then the Message property will be constructed to include the Position, Member, LineNumber, and Position data from the Json.NET exception.

I don't see any value right now in adding LineNumber/Position/etc. properties to ModelError at this stage, since nobody would be using them (and for any developers who want to in custom code, they can get it by casting the Exception to JsonReaderException or whatever it is). Let me know if you disagree, but it seems unrelated to the actual goal of exposing useful info by default.

So @rynowak - do you agree with this plan? If so I'll get on with it. Also I will get in touch with Barry to make sure we have a final position on exposing the Json.NET messages or not.

[rynowak]

Yes, I think this is a great plan. 👍

[SteveSandersonMS]

Implemented in #7015

[janis-veinbergs]

@SteveSandersonMS does #7015 implementation only reates to JsonInputFormatter or was it intended for all input formatters (including Custom)? Failure (Whether exception or InputFormatterResult.FailureAsync()) doesn't make ModelState invalid. Also I don't have any indication whether my parameter should be null or it is null because of parsing failures.

I'm on dotnet 2.1.301