Aspnetcore: Calling BadRequest in controller is inconsistent with attribute validation

[sugarjig]

Describe the bug

After upgrading to ASP.NET Core 2.2, I noticed that attribute-based model validation produces 400 response bodies like the following:

{
    "errors": {
        "Model.Field": [
            "error message"
        ]
    },
    "title": "One or more validation errors occurred.",
    "status": 400,
    "traceId": "|e43bcb39071100489dc1590f71370445.4fc1232b_"
}

I have some additional validation in my controller action based on a database lookup that looks something like this:

try
{
    this.service.ServiceCall(model);
}
catch (InvalidModelException e)
{
    this.ModelState.AddModelError(nameof(Model.Field), e.Message);
    return this.BadRequest(this.ModelState);
}

The resulting response body looks like this:

{
    "Model.Field": [
        "error message"
    ]
}

Expected behavior

I would expect that calling ModelState.AddModelError from a controller action, then passing ModelState to BadRequest would give a response body similar to that of attribute-based model validation.

Additional context

I noticed that calling

return this.BadRequest(new ValidationProblemDetails(this.ModelState));

gives a response body very close to the desired one, minus the traceId field.

[ghost]

Not a solution, but you can call this.ValidationProblem() to get the same response as this.BadRequest(new ValidationProblemDetails(this.ModelState)). If you don't pass in a ModelState argument, then this.ModelState is automatically used. At least it saves some typing when trying to be more consistent.

If you don't like the new format, you can also opt out of using ValidationProblemDetails by calling

.ConfigureApiBehaviorOptions(options => {
    options.SuppressUseValidationProblemDetailsForInvalidModelStateResponses = false;
})

after AddMvc() in Startup.cs (I find this option's name confusing as it seems to imply the opposite of its value). It's odd that this setting only affects the validation filter and not BadRequest(ModelState).

[durgeshkumar14]

Hi guys,

Any idea how can I remove traceId from the default response body using ValidationProblemDetails. I am using
[ProducesResponseType(typeof(ValidationProblemDetails), 400)]

This generates the following schema for response body validation using Swagger:
{
"errors": {
"additionalProp1": [
"string"
],
"additionalProp2": [
"string"
],
"additionalProp3": [
"string"
]
},
"type": "string",
"title": "string",
"status": 0,
"detail": "string",
"instance": "string",
"additionalProp1": {},
"additionalProp2": {},
"additionalProp3": {}
}

But, the response body somehow contains an additional field traceId. This causes the validation of response body to fail :(

Is there any way to remove traceId field from the default response body.

Thanks,
Durgesh

[khellang]

@durgeshkumar14 There's no out-of-the-box option to turn it off.

For regular client error mapping, you need to provide your own implementation of IClientErrorFactory. It's probably best to just use the default as a starting point:

https://github.com/aspnet/AspNetCore/blob/09b50850bc428119ea5adfdbceb0470b7a5c2546/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ProblemDetailsClientErrorFactory.cs#L10-L53

And leave out the call to SetTraceId. Add it like this:

services.TryAddSingleton<IClientErrorFactory, MyCustomProblemDetailsClientErrorFactory>();

---

For validation errors, MVC uses the ApiBehaviorOptions.InvalidModelStateResponseFactory option to convert the validation errors into a result. The default implementation looks like this:

https://github.com/aspnet/AspNetCore/blob/708dc5cb5abd1fe0aa409f355b9e0cea8f6846d4/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/ApiBehaviorOptionsSetup.cs#L106-L121

You can set it by doing something like this:

services.Configure<ApiBehaviorOptions>(options =>
{
    options.InvalidModelStateResponseFactory = context =>
    {
        var problemDetails = new ValidationProblemDetails(context.ModelState);

        var result = new BadRequestObjectResult(problemDetails); 

        result.ContentTypes.Add("application/problem+json"); 
        result.ContentTypes.Add("application/problem+xml"); 

        return result;
    };
});

[durgeshkumar14]

@khellang . It didn't work. I still get traceId in my response.

[khellang]

Are you calling Configure<ApiBehaviorOptions> before or after AddMvc?

[durgeshkumar14]

Thanks @khellang . Earlier I was using it before AddMvc. Using it after MVC worked for me.

One more question. How can I do similar thing for 404/403 response body. It also contains a traceId field.

[durgeshkumar14]

Hi @khellang , I found the answer in your first response. I created a sub class from IClientErrorFactory, and removed the call to setTraceId.

Thanks.

[khellang]

Glad you got it working :+1:

[akalcik]

Not a solution, but you can call this.ValidationProblem() to get the same response as this.BadRequest(new ValidationProblemDetails(this.ModelState)). If you don't pass in a ModelState argument, then this.ModelState is automatically used. At least it saves some typing when trying to be more consistent.

If you don't like the new format, you can also opt out of using ValidationProblemDetails by calling

.ConfigureApiBehaviorOptions(options => {
    options.SuppressUseValidationProblemDetailsForInvalidModelStateResponses = false;
})

after AddMvc() in Startup.cs (I find this option's name confusing as it seems to imply the opposite of its value). It's odd that this setting only affects the validation filter and not BadRequest(ModelState).

But the traceId is still missing!

[bbarry]

@sugarjig In order to mimic the behavior of other validation errors I am doing this and changing the type of my controllers accordingly (I mean I already do for other reasons...) :

public class ApiControllerBase : ControllerBase
{
    public override ActionResult ValidationProblem()
    {
        var options = HttpContext.RequestServices.GetRequiredService<IOptions<ApiBehaviorOptions>>();
        return (ActionResult)options.Value.InvalidModelStateResponseFactory(ControllerContext);
    }
}

From my action I call it like so:

ModelState.AddModelError(nameof(request.MyProperty), "There was a problem here");
return ValidationProblem();

@durgeshkumar14 For improved swagger documentation I am using Swashbuckle annotations(5 rc2 at the moment) along with a custom schema filter which is improving my schema documentation so my full controller looks something like this (basically every controller/action does in order to maintain compat):

[Route("api/[controller]/[action]")]
[ApiController]
[Authorize("api_access")]
public class FooController : ApiControllerBase
{
    private readonly IMapper _mapper;
    private readonly IMediator _mediator;

    public FooController(IMapper mapper, IMediator mediator)
    {
        _mapper = mapper;
        _mediator = mediator;
    }

    [HttpPost]
    [SwaggerResponse(200, "Success", typeof(BarResult))]
    [SwaggerResponse(400, "Bad Request", typeof(DocumentedValidationProblemDetails))]
    public async Task<ActionResult<BarResult>> Bar([FromForm] BarRequest request)
    {
        var result = await _mediator.Send(_mapper.Map<BarCommand>(request));
        // ReSharper disable once InvertIf
        if (!result.Success)
        {
            ModelState.AddModelError(nameof(request.MyProperty), "There was a problem here");
            return ValidationProblem();
        }

        return Ok(_mapper.Map<BarResult>(result));
    }
}

[rcollette]

The solution from @bbarry https://github.com/aspnet/AspNetCore/issues/6077#issuecomment-489798989

is dead simple and does include the trace id.

{
  "errors": {
    "id-mismatch": [
      "The Id passed in the path and in the body do not match."
    ]
  },
  "title": "One or more validation errors occurred.",
  "status": 400,
  "traceId": "0HLN95M3PEC65:00000001"
}

[kthayer424]

Did y'all ever find out what object has the property traceId since it isn't on ProblemDetails? I'd like to use the automatic 400 handling, keep traceId as it is on the result, and also have the correct type identified for the Swagger documentation if possible.

[khellang]

It's part of the extension data. See ProblemDetailsClientErrorFactory.SetTraceId 😉