Json parsing error behaviors control in Minimal Api

[ziaulhasanhamim]

Background and Motivation

I was porting one of my API from controllers to minimal API. I noticed an unexpected behavior in minimal API. Like when I have invalid JSON in the request body in controllers the parameter binds to null and there is an error message added to ModelStateDictionary. But in minimal APIs, it throws a JsonException, most importantly the request doesn't even reach the handler method. I wasn't expecting that. Because in my mind minimal API suppose to give me more flexibility.

Proposed API

There can be a simple solution. Instead of throwing a JsonException why not just provide the handler with correct error messages?

public class ParsingError
{
   // necessary infors
}
public class ParsingErrorParamaterAttribute : Attribute
{
   public ParsingErrorParamaterAttribute(string paramName) {}
}

Here ParsingError will contain whole contexts about the wrong request body format. So for this to happen with custom bindings there needs to be a new overload of the BindAsync method.

public class Post
{
    public static ValueTask<BindingResult<T>> BindAsync(HttpContext context, ParameterInfo parameter)
    {
        // binding code
    }
}
public record BindingResult<T>(T? Model, ParsingError? Error);

So now custom bindings can also return ParsingError.

Usage Examples

The usage would be very simple. There will be just one extra parameter.

app.MapPost("api/posts/",  (Post post, [ParsingErrorParamater("post")] ParsingError postParsingError) => 
{
  // handler code
});

Alternative Designs

There can be a configuration option to turn this on or off.

[davidfowl]

Yes, we've discussed this situation many times, but model state is expensive. I like the idea of a generic type that can capture the appropriate binding errors/context. What you have above doesn't quite work as well because a valid, non-null Post object still needs to be provided to the method. Instead, maybe we can have something like this:

app.MapPost("api/posts/", (ModelState<Post> modelState) => 
{
    if (!modelState.IsValid)
    {
        var exception = modelState.Exception;
        return Results.Problem("Something is wrong");
    }
    var post = modelState.Model;

    // Do stuff here ...
});

PS: You can build this today:

record struct ModelState<T>(T? Model, Exception? Exception)
{
    public bool IsValid => Exception is null;

    public static async ValueTask<ModelState<T>> BindAsync(HttpContext httpContext)
    {
        try
        {
            var item = await httpContext.Request.ReadFromJsonAsync<T>();

            return new(item, null);
        }
        catch (Exception ex)
        {
            return new(default, ex);
        }
    }
}

I'm not married to the name ModelState. BindingResult sounds better 😄

[ziaulhasanhamim]

Yeah, that approach is much cleaner.

we've discussed this situation many times, but the model state is expensive

The model state would be expensive because of validations. But this would be inappropriate to call it ModelState. It would just contain the errors regarding the parsing of JSON and nothing else. So I don't think it would be expensive to use BindingResult<Post> over Post. It would rather simplify request body parsing.

[davidfowl]

Correct, if it was simple like the above, then we can have a well-known type/shape for preserving the error that you can opt-into in your handler.

[ziaulhasanhamim]

So is there any plan to add a type like this Parsable<T> or BindingResult<T> in ASP.NET? Because I wouldn't like idea of giving default validation provider in minimal apis. I like the current approach where I can plug in any validators. But for json errors BindingResult<T> is useful.

[davidfowl]

Well you filed an issue, that's the first step. The team will see this issue and make a decision 😄. It's still the weekend.

[ghost]

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[rellis-of-rhindleton]

(Update: I'm looking at using MiniValidation and MinimalApis.Extensions for this. Actually might solve my problem.)

This issue is approaching a year old, but it's still open and I'm having similar problems, so I'll chime in with some context.

(BTW: Using the approach detailed above seems to break OpenAPI for that endpoint, which is a dealbreaker. Am I missing something?)

What I need is the ability to control the error message and make it useful. I'm not in love with ModelState but it allowed me to do that for bad request errors fairly easily, whether from within the handler method or in other ways. I'm not finding a way to do this with minimal APIs.

Say I omit a required property from an input model. A BadHttpRequestException is thrown:

Failed to read parameter "ModelType model" from the request body as JSON.

This message doesn't tell me what was wrong with the request. It also shares internal names, and it sounds like the problem happened on the server side, which is misleading.

The inner exception is a JsonException and looks like this:

JSON deserialization for type 'ModelType' was missing required properties, including the following: propertyName

This message contains internal information which should not be shared, but it also contains information that should be shared. I'd like to tell the client about missing required properties and which ones they are as part of a ProblemDetails response. But I can't get to that without scraping the string, which doesn't seem like a good idea. This seems to be a problem with System.Text.Json's JsonException and ThrowHelper, which starts out with the information I want but doesn't keep it separate. There may not be anything you can do about that.