Skip to content

Conversation

@brunolins16
Copy link
Member

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

* MVC Changes

* ExceptionHandler changes

* Routing changes

* Http.Extensions changes

* Minimal APi draft changes

* HttpResults changes

* New ProblemDetails project

* Using ProblemDetailsOptions in mvc

* ProblemDetails project simplification

* Using metadata

* Using metadata

* Latest version

* Removing changes

* Initial cleanup

* Clean up

* Public api clean up

* Updating public API

* More clean ups

* Adding initial unit tests

* Updating unit tests

* Adding more unit tests

* Cleanup

* Clean up

* clean up

* clean up

* Removing nullable

* Simplifying public api

* API Review feedback

* API review feedback

* Clean up

* Clean up

* clean up

* Reusing Endpoint & EndpointMetadataCollection

* Fix build issues

* Updates based on docs/Trimming.md

* Adding Functional tests

* Fix unittest

* Seal context

* Seal ProblemMetadata

* PR Feeback

* API Review

* Clean  up

* Fixing publicapi  warnings

* Clean up

* PR Feedback

* Fixing build

* Fix unit test

* Fix unit tests

* PR review

* Fixing JsonSerializationContext issues

* Adding statuscode 405

* Update src/Http/Http.Extensions/src/ProblemDetailsDefaultWriter.cs

Co-authored-by: Brennan <[email protected]>

* PR review

* Apply suggestions from code review

Co-authored-by: Stephen Halter <[email protected]>

* Fixing bad merge

* Fix bad merge

* Adding analysis.NextMiddlewareName

* PR review

* Changing to CanWrite/WriteAsync

Co-authored-by: Brennan <[email protected]>
Co-authored-by: Stephen Halter <[email protected]>
@brunolins16 brunolins16 added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Jul 19, 2022
@brunolins16 brunolins16 requested a review from a team July 19, 2022 15:40
@brunolins16 brunolins16 changed the title Adding ProblemDetailsService (#42384) Backport/pr 42384 to release/7.0 preview7 Jul 19, 2022
@brunolins16
Copy link
Member Author

brunolins16 commented Jul 19, 2022

The original approved via email backport PR (#42741) was merged targeting main instead of release/7.0-preview-7

@brunolins16 brunolins16 added the Servicing-approved Shiproom has approved the issue label Jul 19, 2022
@brunolins16
Copy link
Member Author

@dotnet/aspnet-build Can I get help merging?

@dougbu
Copy link
Contributor

dougbu commented Jul 19, 2022

@mmitche this was servicing-approved by Steve yesterday. Can we get it in now w/o disrupting your world❔

@mmitche
Copy link
Member

mmitche commented Jul 19, 2022

Yep go for it.

@dougbu dougbu merged commit 8869a47 into release/7.0-preview7 Jul 19, 2022
@dougbu dougbu deleted the backport-problem-details branch July 19, 2022 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

old-area-web-frameworks-do-not-use *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.

4 participants