Skip to content

Fix missing params in Results.Problem and Results.ValidationProblems #36884

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

Closed
wants to merge 1 commit into from

Conversation

captainsafia
Copy link
Member

@captainsafia captainsafia commented Sep 23, 2021

Description

In .NET 6, we introduced new extension methods as shorthands for creating Results types from endpoint. However, due to an error, we missed adding a parameter to the Results.Problem and Results.ValidationProblem to align with the Extensions property available in those types.

As a result of this, customers are not able to return ProblemDetails or HttpValidationProblemDetails with a configured Extensions type from their APIs.

Customer Impact

Without this fix, customers are not able to return to return ProblemDetails objects

There are no easy workarounds for this issue since the Results.Problem and Results.ValidationProblem provide a public abstraction over currently internal functionality, like setting sensible defaults for the title and type of ProblemDetails.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

Low risk because:

  • This change resolves an existing gap in the in the Results extension methods API.
  • The surface area of the change is small so bugs are less likely.
  • The ProblemDetails already supports having an Extensions property that is validated.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

Background and Motivation

The Results.Problem and Results.Validation extension methods do not provide a way for users to customize the Extensions property on the resulting ProblemDetails payload.

To support these scenarios, we are adding the extensions parameter to the existing methods and adding new overloads that takes a ProblemDetails or HttpValidationProblemDetails object to provide further customizability for users.

Proposed API

namespace Microsoft.AspNetCore.Http
{
    public static class Results
    {
        public static IResult Problem(
            string? detail = null,
            string? instance = null,
            int? statusCode = null,
            string? title = null,
            string? type = null,
+           IDictionary<string, object?>? extensions = null)
        {
            ...
        }

+       public static IResult Problem(ProblemDetails problemDetails) { ... }

        public static IResult ValidationProblem(
            IDictionary<string, string[]> errors,
            string? detail = null,
            string? instance = null,
            int? statusCode = null,
            string? title = null,
            string? type = null,
+           IDictionary<string, object?>? extensions = null)
        {
            ...
        }
    }
}

Usage Examples

// {
//   "type": "https://tools.ietf.org/html/rfc7231#section-6.6.1",
//   "title": "An error occurred while processing your request.",
//   "status": 500,
//   "green": "foo"
// }
app.MapGet("/sample1", () => 
    Results.Problem(statusCode: 500, extensions: new Dictionary<string, object?>() { { "green", "foo" } }));
// {
//   "type": "https://tools.ietf.org/html/rfc7231#section-6.6.1",
//   "title": "An error occurred while processing your request.",
//   "status": 500,
//   "green": "foo"
// }
app.MapGet("/sample2", () => 
    Results.Problem(new ProblemDetails() { Status = 500, Extensions = { { "green", "foo"}}}));
var errors = new Dictionary<string, string[]>();
// {
//   "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1",
//   "title": "Bad Request",
//   "status": 400,
//   "green": "foo",
//   "errors": {
    
//   }
// }
app.MapGet("/sample3", () =>
    Results.ValidationProblem(errors, statusCode: 400, extensions: new Dictionary<string, object?>() { { "green", "foo" } }));
// {
//     "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1",
//     "title": "One or more validation errors occurred.",
//     "status": 400,
//     "green": "foo",
//     "errors": {
        
//     }
// }
app.MapGet("/sample4", () => 
    Results.Problem(new HttpValidationProblemDetails(errors) { Status = 400, Extensions = { { "green", "foo"}}}));

Addresses #36848

@captainsafia captainsafia requested a review from a team September 23, 2021 15:52
@ghost ghost added the area-runtime label Sep 23, 2021
@Pilchie Pilchie added the Servicing-consider Shiproom approval is required for the issue label Sep 23, 2021
@ghost
Copy link

ghost commented Sep 23, 2021

"Hi captainsafia. 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.

@jamshedd jamshedd added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Sep 23, 2021
@jamshedd
Copy link
Member

We agreed to take this for RC2.

@captainsafia captainsafia changed the base branch from release/6.0 to release/6.0-rc2 September 23, 2021 18:22
@captainsafia captainsafia changed the base branch from release/6.0-rc2 to release/6.0 September 23, 2021 18:23
@captainsafia
Copy link
Member Author

We agreed to take this for RC2.

Thanks! Closing this in favor of #36856 which already targets the correct branch.

@captainsafia captainsafia deleted the safia/probdet-fix branch October 1, 2021 02:02
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants