Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport/pr 42384 to release/7.0 preview7 #42741

Merged

Conversation

brunolins16
Copy link
Member

@brunolins16 brunolins16 commented Jul 15, 2022

Backport of #42384 to release/7.0-preview4

/cc @brunolins16

Adding ProblemDetailsService

Adding a new ProblemDetailsService responsible for consistent Problem Details generation.

Description

API Controllers have a mechanism to auto generated Problem Details (https://datatracker.ietf.org/doc/html/rfc7807) for API Controller ActionResult. The mechanism is enabled by default for all API Controllers, however, it will only generates a Problem Details payload when the API Controller Action is processed and produces a HTTP Status Code 400+ and no Response Body, that means, scenarios like - unhandled exceptions, routing issues - won't produce a Problem Details payload.

Here is overview of when the mechanism will produce the payload:

❌ = Not generated
✅ = Automatically generated

Routing issues: ❌

Unhandled Exceptions: ❌

MVC

  • StatusCodeResult 400 and up: ✅ (based on SuppressMapClientErrors)
    • BadRequestResult and UnprocessableEntityResult are StatusCodeResult
  • ObjectResult: ❌ (Unless a ProblemDetails is specified in the input)
    • eg.: BadRequestObjectResult and UnprocessableEntityObjectResult
  • 415 UnsupportedMediaType: ✅ (Unless when a ConsumesAttribute is defined)
  • 406 NotAcceptable: ❌ (when happens in the output formatter)

Minimal APIs won't generate a Problem Details payload as well.

Here are some examples of reported issues by the community:

This API introduce a new service that is consumed by Diagnostics Middlewares to have the ProblemDetails generated, for all Status 400+ and also have a mechanism that allows devs or library authors (eg. API Versioning) generate ProblemDetails responses when opted-in by the users.

Sample Usage

Default options
var builder = WebApplication.CreateBuilder(args);

// Add services to the containers
builder.Services.AddControllers();
builder.Services.AddProblemDetails();

var app = builder.Build();

// When problemdetails is enabled this overload will work even
// when the ExceptionPath or ExceptionHadler are not configured
app.UseExceptionHandler();

// Configure the HTTP request pipeline.
if (app.Environment.IsDevelopment())
{
    app.UseDeveloperExceptionPage();
}

//Generate PD for 400+
app.UseStatusCodePages();

app.MapControllers();
app.Run();

Fixes #42212

Customer Impact

This issue has been reported for years, including a community middleware is available to try fix the gap in the framework. This change does not replace the community middleware totally, however, enable the core functionality in the framework.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

This is a new API. The change to existing code is very small and non-impactful unless the new AddProblemDetails method is called.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@brunolins16 brunolins16 marked this pull request as ready for review July 15, 2022 07:50
@brunolins16 brunolins16 requested a review from a team July 15, 2022 07:50
@brunolins16 brunolins16 added the area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Jul 15, 2022
@brunolins16 brunolins16 added the Servicing-consider Shiproom approval is required for the issue label Jul 18, 2022
@ghost
Copy link

ghost commented Jul 18, 2022

Hi @brunolins16. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@brunolins16 brunolins16 added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Jul 19, 2022
@brunolins16
Copy link
Member Author

Approved via email.

@brunolins16 brunolins16 merged commit 44b3ddd into dotnet:main Jul 19, 2022
@ghost ghost added this to the 7.0-rc1 milestone Jul 19, 2022
@brunolins16 brunolins16 deleted the backport/pr-42384-to-release/7.0-preview7 branch August 2, 2022 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow consistent Problem Details generation
2 participants