.NET 7 Preview 7 ProblemDetailsService is not working as expected.

[TanvirArjel]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

The release note says that:

.NET 7 Preview 7 introduces a new problem details service based on the IProblemDetailsService interface for generating consistent problem details responses in your app.

To add the problem details service, use the AddProblemDetails extension method on IServiceCollection.

builder.Services.AddProblemDetails();

I have done so. Now I have the following endpoint:

[HttpPost]
public ActionResult Post(CreateWFModel model)
{
    if (model.Summary?.Length < 100)
    {
        ModelState.AddModelError("Summary", "The summary should be at least 100 characters.");
        return BadRequest(ModelState);
    }

    return Ok();
}

public class CreateWFModel
{
    public DateTime Date { get; set; }

    public int TemperatureC { get; set; }

    [Required]
    public string? Summary { get; set; }
}

Now if I don't provide the value of the summary then the response shows as follows:

{
  "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1",
  "title": "One or more validation errors occurred.",
  "status": 400,
  "traceId": "00-2759a2ba3ce779e64e8ec4f21e4565fc-7b9e6059c062c576-00",
  "errors": {
    "Summary": [
      "The Summary field is required."
    ]
  }
}

This is fine!

But when I provide less than 100 chars, the response shows as follows:

{
  "Summary": [
    "The summary should be at least 100 characters."
  ]
}

Which is totally inconsistent. I want the response should be:

{
  "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1",
  "title": "One or more validation errors occurred.",
  "status": 400,
  "traceId": "00-2759a2ba3ce779e64e8ec4f21e4565fc-7b9e6059c062c576-00",
  "errors": {
    "Summary": [
       "The summary should be at least 100 characters."
    ]
  }
}

I have also tried the following but the result is the same:

public class CustomWriter : IProblemDetailsWriter
{
    // Indicates that only responses with StatusCode == 400
    // will be handled by this writer. All others will be
    // handled by different registered writers if available.
    public bool CanWrite(ProblemDetailsContext context) 
        => context.HttpContext.Response.StatusCode == 400;

    public ValueTask WriteAsync(ProblemDetailsContext context)
    {
        //Additional customizations

        // Write to the response
        context.HttpContext.Response.WriteAsJsonAsync(context.ProblemDetails);
       return ValueTask.CompletedTask;
    }        
}

builder.Services.AddSingleton<IProblemDetailsWriter, CustomWriter>();
builder.Services.AddProblemDetails();

.NET Version

.NET 7.0 Preview 7

[davidfowl]

cc @brunolins16

[brunolins16]

@TanvirArjel I have very happy that you have tried the new ProblemDetailsService 😁.

If you take a look at the same release notes (Diagnostics middleware updates) we mentioned about updates in the middleware to make the response consistent and there we have mentioned a key important aspect of this change.

The following sample configures the app to generate a problem details response for all HTTP client and server error responses that do not have a body content yet.

Basically, it means that once you have a body defined the middleware will not call the problem details service to generate a Problem Details payload. It is similar to what you have in your scenario, but in your case the MVC engine is responsible to autogenerate the payload when the body is empty.

In your code return BadRequest(ModelState); call will serialize the ModelStateDictionary to the body avoid the Problem Details autogeneration.

[TanvirArjel]

In your code return BadRequest(ModelState); call will serialize the ModelStateDictionary to the body avoid the Problem Details autogeneration.

Yes! That's why this behavior is inconsistent and error-prone and the response for the bad requests always should be the same.

[brunolins16]

With all that said, that is the current expected behavior. However, I can understand your expectation once the service is registered and I think since now we have a service that allow a consistent generation might be an interesting change to generate PD in these scenarios as well,

[TanvirArjel]

@brunolins16

I think since now we have a service that allows a consistent generation might be an interesting change to generate PD in these scenarios as well,

Yes! otherwise, the Client App calling the ASP.NET Core API will be broken while handling the bad request error response.

[brunolins16]

In your code return BadRequest(ModelState); call will serialize the ModelStateDictionary to the body avoid the Problem Details autogeneration.

Yes! That's why this behavior is inconsistent and error-prone and the response for the bad requests always should be the same.

Totally agree! I think we have commented at same time. In my opinion, adding the new service was a start point to make the Problem Details generation consistent, but we still need to make additional changes to allow it to be customizable and consistent in scenarios as you mentioned.

[brunolins16]

For now, to make it consistent I recommend you keep using return ValidationProblem(ModelState);

[TanvirArjel]

@brunolins16 This is a very crucial bug. I am surprised that this very basic requirement is not noticed by ASP.NET Core Team while they are going to release version 7.0 soon.

[TanvirArjel]

@brunolins16

For now, to make it consistent I recommend you keep using return ValidationProblem(ModelState);

Thanks! I am aware of this.

[brunolins16]

@brunolins16

For now, to make it consistent I recommend you keep using return ValidationProblem(ModelState);

Thanks! I am aware of this. But writing it everywhere will be ugly.

I understand you, but I don't see, in your specific example, how uglier it is since it is basically a validation problem already,

@brunolins16 This is a very crucial bug. I am surprised that this very basic requirement is not noticed by ASP.NET Core Team while they are going to release version 7.0 soon.

I was the team member who worked on this and to be honest probably I missed because the main scenario was the auto generation (when a body was not defined already) that we used to have no control and, in your case, you do have control what you want to generate (calling ValidationProblem/ Problem/etc...), and this current behavior is very well documented

Unfortunately, we are very late in .NET 7 development and might not be possible to have it done :(

[TanvirArjel]

@brunolins16 Wish you all the very best for dotNET 8.0 :) But please don't miss this.

[TanvirArjel]

@brunolins16

I understand you, but I don't see, in your specific example, how uglier it is since it is basically a validation problem already,

I see! It's a good workaround. Thanks again.

[pinkfloydx33]

DefaultApiProblemDetailsWriter also doesn't support the CustomizeProblemDetails callback which seems somewhat inconsistent--though I suppose you could just customize it at creation time.

But would that writer even be invoked? It seems like the DefaultProblemDetailsWriter would always return true from CanWrite for the cases that one would.... so I guess it depends on the order you add the services.

If you want the behavior you're after you should check out the ProblemDetailsMiddleware nuget which supports that and more. My hope was to migrate off it but I'm not sure it'll be possible as I rely on some of the features that aren't included in the new services

[brunolins16]

@pinkfloydx33 it does, let me double check and share with you where it is called.

[brunolins16]

It is called here

aspnetcore/src/Mvc/Mvc.Core/src/Infrastructure/DefaultProblemDetailsFactory.cs

Line 101 in 0eaabe0

_configure?.Invoke(new() { HttpContext = httpContext!, ProblemDetails = problemDetails });

[brunolins16]

Any writer registered before the call to the AddProblemDetails will be evaluated before the DefaultProblemDetailsWrapper

[pinkfloydx33]

It is called here

Ahh my mistake... It's tucked away in the factory; I was looking for it in the writer similar to the default one. Good to know it's there.

Any writer registered before the call to the AddProblemDetails will be evaluated before the DefaultProblemDetailsWrapper

Ok...so if using MVC Controllers (and not minimal API) you'd have to be sure to call AddMvc/AddControllers before AddProblemDetails if you want it to use the code in DefaultApiProblemDetailsWriter.

FWIW the outputs from the two are mostly the same anyways, though by default DefaultApiProblemDetailsWriter will add traceId extension (via the factory) while the DefaultProblemDetailsWriter won't. A properly constructed customization callback could rectify that (if that's what one wants/expects).

[brunolins16]

Ok...so if using MVC Controllers (and not minimal API) you'd have to be sure to call AddMvc/AddControllers before AddProblemDetails if you want it to use the code in DefaultApiProblemDetailsWriter.

Exactly.

FWIW the outputs from the two are mostly the same anyways, though by default DefaultApiProblemDetailsWriter will add traceId extension (via the factory) while the DefaultProblemDetailsWriter won't. A properly constructed customization callback could rectify that (if that's what one wants/expects).

The biggest difference between them is the MVC implementation supports content-negotiation, so, we can produce XML Problem Details.

[jchoca]

Hi, I think this may be related, but the casing for ProblemDetails is also inconsistent. ProblemDetails itself defaults to camel case, but as you can see above when describing the "Summary" field, it's capitalized. Shouldn't that be lower case, assuming @TanvirArjel is using the default naming policy?

I've also noticed ProblemDetails does not obey JsonOptions settings, e.g., setting

services.Configure<JsonOptions>(opt =>
{
    opt.JsonSerializerOptions.PropertyNamingPolicy = null; // don't convert to camel case
});

should give a response like

{
    "Type": "https://tools.ietf.org/html/rfc7231#section-6.5.4",
    "Title": "Not Found",
    "Status": 404,
    "Detail": "Account 1 not found.",
    "TraceId": "00-50f63c5d7921484e4797d82687f43033-915b95474bf55ff2-00"
}

but it does not, at least from what I can tell. Maybe this is by design since the spec (https://www.rfc-editor.org/rfc/rfc7807) explicitly lists the properties lower/regular camel case, but it also doesn't seem to explicitly call out camelcase must be used. I think anyone who is using problem details should be able to use whatever casing their API uses already rather than being forced to either a) have inconsistent casing returned or b) use camel case.

[brunolins16]

Hi, I think this may be related, but the casing for ProblemDetails is also inconsistent. ProblemDetails itself defaults to camel case, but as you can see above when describing the "Summary" field, it's capitalized. Shouldn't that be lower case, assuming @TanvirArjel is using the default naming policy?

In MVC for scenarios where the Validation Problem Details is auto generated, by default, it does not change it unless (.NET 7 only) you:

1. Adds SystemTextJsonValidationMetadataProvider (Use JSON Property Name attributes when creating ModelState Validation errors #39010) (recommended)

services..AddControllers(options => options.ModelMetadataDetailsProviders.Add(new SystemTextJsonValidationMetadataProvider()))

2. Set JsonSerializerOptions.DictionaryKeyPolicy (Using DictionaryKeyPolicy during ProblemDetails.Errors conversion to JSON #38853)

services.AddControllers().AddJsonOptions(options => 
{ 
    options.JsonSerializerOptions.DictionaryKeyPolicy = System.Text.Json.JsonNamingPolicy.CamelCase;
});

We have plans to make the provider default for ApiControllers, but it will only come for .NET 8 (#40408)

[brunolins16]

I've also noticed ProblemDetails does not obey JsonOptions settings, e.g., setting

@jchoca Interesting, let me investigate but problem details serialization should use the naming policy. Can you share a simplified repro?

[brunolins16]

@jchoca I just took a look at our ProblemDetailsConverter and I think you might be right since I don't see any usage of the options when we write, other than for Extensions. Unless it is used internally in the Utf8JsonWriter (that I need to investigate)

aspnetcore/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs

Line 103 in 7fac52d

internal static void WriteProblemDetails(Utf8JsonWriter writer, ProblemDetails value, JsonSerializerOptions options)

[jchoca]

@brunolins16 thanks for checking! I created a simple web app here that illustrates the problem:

https://github.com/jchoca/ProblemDetailsJsonSettings

if you hit the root endpoint it returns pascal case, but if you hit /error it returns camel case.

[zwoolli]

Any writer registered before the call to the AddProblemDetails will be evaluated before the DefaultProblemDetailsWrapper

I had to register my custom IProblemDetailsWriter before the call to AddRazorPages AND the call to AddProblemDetails for it to be evaluated before the DefaultProblemDetailsWriter.

[brunolins16]

Any writer registered before the call to the AddProblemDetails will be evaluated before the DefaultProblemDetailsWrapper

I had to register my custom IProblemDetailsWriter before the call to AddRazorPages AND the call to AddProblemDetails for it to be evaluated before the DefaultProblemDetailsWriter.

@zwoolli If I am not mistaken AddRazorPages probably calls AddMVCCore (need to double check) that register a default writer for API Controllers. I believe your case the problem is not the DefaultProblemDetailsWriter but the DefaultApiProblemDetailsWriter. In both cases you are right and AddProblemDetails must be called before.

@Rick-Anderson maybe we could add something about it in the docs if not there yet.

[Rick-Anderson]

@Rick-Anderson maybe we could add something about it in the docs if not there yet.

See #29152

[zwoolli]

@zwoolli If I am not mistaken AddRazorPages probably calls AddMVCCore (need to double check) that register a default writer for API Controllers. I believe your case the problem is not the DefaultProblemDetailsWriter but the DefaultApiProblemDetailsWriter. In both cases you are right and AddProblemDetails must be called before.

@brunolins16 - You are correct. I looked into it and AddRazorPages does indeed call AddMVCCore,

• which calls AddMvcCoreServices,
• which registers the DefaultApiProbtailsWriter

[mitchdenny]

@TanvirArjel is this still an issue for you. If it is could you post a repro of the original issue including an example of the HTTP requests that you are firing into the controller? I tried to repro the issue you were describing and couldn't which makes me think that there is some context missing.

[ghost]

Hi @TanvirArjel. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[ghost]

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.