Support more ProblemDetails Titles out-of-the-box #43232
Conversation
- Use `ReasonPhrases.GetReasonPhrase()` to extend the default titles for problem details. - Apply some minor code clean-ups.
- Update links returned for problem details from RFC7231 to RFC9110. - Update other references from RFC7231 to RFC9110.
Add titles and types for HTTP 408, 412, 426 and 502-504.
Update remaining references from RFC7231 to RFC9110.
Do not set the problem details title to the empty string when the HTTP status code does not have a default reason phrase and leave it as null.
martincostello
requested review from
Tratcher,
halter73,
BrennanConroy,
JamesNK,
a team and
Pilchie
as code owners
ghost
added
the
community-contribution
|
Thanks for your PR, @martincostello. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
| @@ -24,6 +24,7 @@ | |||
|
|
|||
| <ItemGroup> | |||
| <Reference Include="Microsoft.AspNetCore.Http.Abstractions" /> | |||
| <Reference Include="Microsoft.AspNetCore.WebUtilities" /> | |||
There was a problem hiding this comment.
Added as suggested by #36417 (comment)
Revert over-zealous update to RFC links as HTTP 422 wasn't RFC7231 in the first place.
Pilchie
added
the
old-area-web-frameworks-do-not-use
| "An error occurred while processing your request." | ||
| ), | ||
|
|
||
| [502] = |
There was a problem hiding this comment.
I was hoping to be able to consolidate both lists (ProblemDetailsDefaults.Defaults and ReasonPhrases.Phrases , including the reference URL, to avoid this adding new status codes here. But we probably will need a public API change :(.
Is there a reason for this? |
|
Just some additional statuses that I consider common I thought I'd add as I was touching the code. Happy to remove if they're not wanted. |
| "https://tools.ietf.org/html/rfc9110#section-15.5.22", | ||
| "Upgrade Required" | ||
| ), | ||
|
|
There was a problem hiding this comment.
429? I think it'll be even more common with rate limiting built into the framework.
There was a problem hiding this comment.
Makes sense to add if the team is happy with the other additions I already made.
There was a problem hiding this comment.
I don't see why not add 429 as well.
There was a problem hiding this comment.
I remember why I didn't add it now - it's not in the RFC 😄
@martincostello Sorry for the delay in reply here. I don't see a problem with that but as I said I was looking for removing all status (consolidating into a single dictionary) here instead adding more. Maybe we can do this for .NET 8. |
- Update comment. - Rename test. - Refactor code style. Co-authored-by: Bruno Oliveira <[email protected]>
|
@martincostello Thanks again for the contribution! |
|
Hi @brunolins16. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
Support more ProblemDetails Titles out-of-the-box
Add support for more default HTTP status titles with problem details.
Description
ReasonPhrases.GetReasonPhrase()to extend the default titles for problem details.Fixes #36417.