Skip to content
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

HttpErrorResponse statusText always default to 'OK' regardless of statusCode #23334

Open
bagage opened this issue Apr 12, 2018 · 13 comments
Open
Labels
area: common/http breaking changes design complexity: low-hanging freq1: low P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: confirmed type: bug/fix
Milestone

Comments

@bagage
Copy link
Contributor

bagage commented Apr 12, 2018

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

When server response does not contain any statusText, HttpErrorResponse.statusText is automatically set to OK regardless of the status code.
Thus you can end up with 403 OK error response for instance, which I believe is weird. In the previous documentation it was clearly stated that OK was the default value, but I'm not sure to understand the reason behind it -- an empty string would be less confusing I think.

Expected behavior

Either the statusString should be left empty, or even better check for status code and either use the proper http status for it, or show ok only for success status code only (and.. something has to be done for others)?

Minimal reproduction of the problem with instructions

What is the motivation / use case for changing the behavior?

While debugging some 403 errors on a website, I looked at HttpErrorResponse object with a 403 OK status which made me think there was something going wrong with the server and/or the error handling, but actually that was not the error at all. I know that documentation states Do not depend on this., but this is still misleading when you see that in logs IMO.

Environment


Angular version: 5.2.9, still applicable on latest version


Browser:
- [ ] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [x] Firefox version 59.0.2
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
 
For Tooling issues:
- Node version: XX  
- Platform:  

Others:

@ngbot ngbot bot added this to the needsTriage milestone Apr 12, 2018
@trotyl
Copy link
Contributor

trotyl commented Apr 18, 2018

It's NOT caused by Angular, but a XHR spec defined behavior.

It would still be OK even after removing the || 'OK' code in Angular.

bagage added a commit to bagage/angular that referenced this issue Apr 22, 2018
@bagage
Copy link
Contributor Author

bagage commented Apr 22, 2018

Oh, I didn't though it could have been something from the XHR spec, thanks @trotyl for letting me know!

I slightly modified the doc to add the default value then (as the original documentation actually), in case someone else encounters the same "issue".

@alxhub
Copy link
Member

alxhub commented Apr 24, 2018

Closing as @trotyl is correct, it's specified behavior.

@alxhub alxhub closed this as completed Apr 24, 2018
@hhu94
Copy link

hhu94 commented Oct 5, 2018

The XHR spec cited by @trotyl no longer defines OK to be the default status text.

A response has an associated status message. Unless stated otherwise it is the empty byte sequence.

This issue should be reopened. When the response has an empty status message Angular should leave it that way instead of setting it to OK.

@trotyl
Copy link
Contributor

trotyl commented Oct 6, 2018

Confirmed, the spec change was made at whatwg/fetch#600.

@n0nick
Copy link

n0nick commented Dec 10, 2018

Any update on this issue, should it be reopened?

@sbgreene1307
Copy link

FWIW If you're using the angular-cli proxy, it will add the correct statusText. We discovered this because for local development we use the proxy to forward to our deployed backend service. 4xx responses always had the right text. But once we deployed the same code, the response statusText was always 'OK'. Confirmed this to be true by bypassing the proxy.conf.json on a locally running UI. Just provide the direct URL for one of the http requests.

@alxhub
Copy link
Member

alxhub commented May 29, 2020

Yep, we should change to an empty string I suppose. It'll require a little thought to ensure this can happen without breaking existing consumers.

@SchnWalter
Copy link
Contributor

SchnWalter commented Jun 15, 2020

FYI: The HTTP/2 spec removed this "statusText" ("reason phrase"). So if you add an HTTP/2 proxy or load ballancer in between your HTTP/1.1 API and your Angular application, you will encounter the default OK value; thus you might encounter issues on your PROD environments, even if everything worked OK on UAT. (pun intended)

HTTP/2 does not define a way to carry the [...] reason phrase that is included in an HTTP/1.1 status line.
https://http2.github.io/http2-spec/#rfc.section.8.1.2.4

@jelbourn jelbourn added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed severity3: broken labels Oct 1, 2020
@tomicluka24
Copy link

@jovanarsov Is there any update about issue? Im having same problem in my App.

@jovanarsov
Copy link

Hi @tomicluka24,
I couldn't find real solution for this, when I was working on it.
Here is my ErrorInterceptor.ts file (https://github.com/jovanarsov/DatingApp/blob/master/client/src/app/_interceptors/error.interceptor.ts), I found this solution somewhere in comments and used it just so I could return different(correct) kind of toastr message to the user instead the Ok() result

@tomicluka24
Copy link

@jovanarsov Thank you for the answer!

khaledkoka added a commit to khaledkoka/DatingApp that referenced this issue Aug 2, 2022
The error status text shows: 'OK'. It should be showing 'Bad Request' in case of 400 and 'Unauthorized' in case of 401. This happens because of http/2 spec and the way the Angular client defaults to displaying an OK if there is nothing in the statusText field as described here: angular/angular#23334
@umairmushtaq109
Copy link

@jovanarsov Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: common/http breaking changes design complexity: low-hanging freq1: low P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: confirmed type: bug/fix
Projects
None yet
Development

No branches or pull requests