8000 fix(common): Don't include statusText in HttpErrorResponse.message, deprecate HttpResponseBase.statusText by kescherCode · Pull Request #58080 · angular/angular · GitHub
[go: up one dir, main page]

Skip to content

fix(common): Don't include statusText in HttpErrorResponse.message, deprecate HttpResponseBase.statusText #58080

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kescherCode
Copy link
@kescherCode kescherCode commented Oct 4, 2024

Since HTTP/2, responses no longer contain a status text besides the status code, which caused our default value of 'OK' to be used in HttpErrorResponse.message. As discussed in #57580, we are opting to not change the default value to match the fetch spec (https://fetch.spec.whatwg.org/#concept-response-status-message), instead, we are deprecating the statusText field, and not using it in our own code.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.dev application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #23334

What is the new behavior?

The HttpResponseBase.statusText field is deprecated, and it is not used in HttpErrorResponse.message anymore.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

This was discussed in #57580.

@pullapprove pullapprove bot requested a review from alxhub October 4, 2024 12:15
@angular-robot angular-robot bot added the area: common Issues related to APIs in the @angular/common package label Oct 4, 2024
@ngbot ngbot bot added this to the Backlog milestone Oct 4, 2024
@JeanMeche
Copy link
Member

We'll need to update our golden files, for this please run yarn bazel run //packages/common:common_api.accept.

Also please include a DEPRECATED: ... mention in the footer of the commit message, as mentionned by our guidelines.

@angular-robot angular-robot bot added the detected: deprecation PR contains a commit with a deprecation label Oct 4, 2024
…eprecate HttpResponseBase.statusText

Since HTTP/2, responses no longer contain a status text besides the status code, which caused our default value of 'OK' to be used in HttpErrorResponse.message.
As discussed in angular#57580, we are opting to not change the default value to match the fetch spec (https://fetch.spec.whatwg.org/#concept-response-status-message), instead, we are deprecating the statusText field, and not using it in our own code.

DEPRECATED: HttpResponseBase.statusText

This field is usually populated with OK by default, and overwritten by what a remote HTTP server responds with on the status line.
Since HTTP/2, we aren't guaranteed to have a value for this on the status line, meaning this value gets stuck at OK.
If you need this field for display purposes, it is recommended to retrieve a selection from RFC9110 (or future RFCs) yourself.
For other purposes, an HttpResponseCode enum is also provided at common/http.


Fixes angular#23334
@gorvk
Copy link
gorvk commented Dec 5, 2024

any updates on this PR ?

@kescherCode
Copy link
Author

@gorvk not from my end. All reque 8000 sted changes have been made so far, so it has just been sitting here, waiting for further feedback, approval or a close.

@thePunderWoman thePunderWoman marked this pull request as draft May 21, 2025 15:28
@thePunderWoman thePunderWoman removed the request for review from alxhub June 3, 2025 11:01
Copy link
Contributor
@thePunderWoman thePunderWoman left a comment

Choose a reason for hiding this comment

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

LGTM

reviewed-for: fw-general, public-api

@thePunderWoman thePunderWoman marked this pull request as ready for review June 27, 2025 14:30
Copy link
Member
@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

Reviewed-for: public-api

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: common Issues related to APIs in the @angular/common package detected: deprecation PR contains a commit with a deprecation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0