Skip to content

Conversation

@joegoldman2
Copy link
Contributor

@joegoldman2 joegoldman2 commented Sep 26, 2024

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Description

This PR adds additional problem details titles and types for HTTP statuses 407, 410, 411, 413, 414, 416, 417, 421, 501, and 505.

@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Sep 26, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Sep 26, 2024
@joegoldman2 joegoldman2 requested a review from a team as a code owner September 27, 2024 06:18
@martincostello martincostello added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework labels Sep 27, 2024
@adityamandaleeka
Copy link
Member

Thanks @joegoldman2. This seems fine to me... @martincostello @captainsafia, do we have any policy for which statuses we include here?

@martincostello
Copy link
Member

Not that I'm aware of - I guess as long as it's an official code in the RFC it should be fine?

@captainsafia
Copy link
Member

Yep, the Problem Details spec is very permissive about what status codes can be referenced in the payload (ref). The only thing it cautions about is that you make sure the status code in the Problem Details payload matches what is actually returned in the HTTP response (accounting for any proxies and whatnot).

With that in mind, we might want to consider if we want to go ahead and support all the status codes or just the finite set added here. @joegoldman2 Was there a particular reason you chose this subset to add?

@joegoldman2
Copy link
Contributor Author

joegoldman2 commented Oct 9, 2024

@joegoldman2 Was there a particular reason you chose this subset to add?

I needed to use a few (not all I added to this PR) and realized that default values were missing. I decided to add all missing standard (from the RFC) statuses.

Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I double checked and this does cover all the status codes currently under Client Error and Server Error in the RFC.

@captainsafia captainsafia merged commit 44a9f8a into dotnet:main Oct 9, 2024
27 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-preview1 milestone Oct 9, 2024
@joegoldman2 joegoldman2 deleted the problem-details-defaults branch October 9, 2024 17:57
captainsafia pushed a commit that referenced this pull request Dec 31, 2024
* Add additional ProblemDetails default types and titles

* Fix unit test

---------

Co-authored-by: joegoldman2 <[email protected]>
captainsafia pushed a commit that referenced this pull request Feb 11, 2025
* Add additional ProblemDetails default types and titles

* Fix unit test

---------

Co-authored-by: joegoldman2 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants