XHR does not treat 4xx responses as errors

[escardin]

When my server returns a 400 response with text, the xhr backend does not route this through to the error observer, and instead calls the success path.

I am porting a service from angular 1 to angular 2, and this was not a problem in angular 1, nor is it a problem with rxjs-dom, which I am using in the meantime.

Affected area:
https://github.com/angular/angular/blob/master/modules/angular2/src/http/backends/xhr_backend.ts#L41

What rxjs-dom does and I think angular 2 should do similarly:
https://github.com/Reactive-Extensions/RxJS-DOM/blob/master/src/ajax/ajax.js#L76

[jeffbcross]

Agreed. I thought there was already an issue for this, but hard to search on my mobile.

[jeffbcross]

Though some have disagreed with this behavior because fetch only errors on network errors.

[escardin]

I was thinking about that when I filed this issue. If it was a promise, it would be success and error, and http codes only consider 2xx to be success. Since subscribe is analogous to success, it should only be 2xx codes.

Realistically, anything other than 2xx is an error (Or rather, not what I am expecting), and needs special handling, the same sort of handling I am doing when I have a network error.

[robwormald]

Just depends really if we want to stick to the semantics of the fetch specification or not. I'm mostly in agreement here, though perhaps it should be a configurable/injectable option.

Proposed handler from github's fetch polyfill

function checkStatus(response) {
  if (response.status >= 200 && response.status < 300) {
    return response
  } else {
    var error = new Error(response.statusText)
    error.response = response
    throw error
  }
}

[e-oz]

it's really awful, especially because in case of error we can't read error status. Please please fix it.

[danielgerlag]

I don't agree with this. a non 200 response is not an error, can we revert this back to the way it was?

[escardin]

4xx and 5xx are by definition, errors. Says right here: https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

10.4 Client Error 4xx

10.5 Server Error 5xx

They're admittedly different from a lost connection, but they're still errors, handled differently from your 'happy path'. Handling of a lost connection and a server or client error are more likely to be similar than a server/client error and a successful response.

If this is reverted, then everyone has to check status codes on their responses for the happy path, just to be sure it's right, rather than that path having a clear route that is predictable and handling special cases with an error handler.

You can work around this by catching the error and transforming it into a 'success' if that's how you want your code to work.

[danielgerlag]

a 4xx could be a validation error with the data submitted, this is not an exception, with ASP.NET WebAPI you would also get a ModelState in the body content with the 400 response, now I can't access that anymore. Only 5xx responses are truly exceptions. A 4xx response means the communication was successful but the server didn't like the data it got, still a success.

[danielgerlag]

We should revert the default backend to not throw this error and create an alternate ConnectionBackend for those who want to treat non 200 responses as actual errors.

[e-oz]

no

[e-oz]

I like when people decide what other people should do. So funny.

[escardin]

'a 4xx could be a validation error' is still an error. Just because you get content in your 4xx response doesn't mean everyone else does, and is as far as I know, more the exception than the rule. You can catch the error, check if it's 4xx and redirect it through your normal pipe. You could make a wrapper around http and make that the default. You could extend XHR backend and change it back to how you like. There are lots of solutions.

Neither e-oz nor myself are members of the Angular 2 team, but we like it how it is.

I do have to say though, this isn't a democracy, due process is not strictly a consideration. You've made your case, and we are disagreeing with you. If you want consensus, anecdotally, I've seen more complaints about it doing what you ask than doing what I asked.

[danielgerlag]

Your implementation doesn't allow one to differentiate between an error in the communication with the server, or an error that occurred on the server. An error on the server still had a successful communication with the client.

How does one flag this issue so that someone from the core angular team can review it?

[escardin]

This would do exactly what you want: http://plnkr.co/edit/dsNKsUImdbh4uOvaS1kl?p=preview

[danielgerlag]

No, that is a hack. The way the ConnectionBackend originally was the most pure and gave the developer the most flexibility. It should not sniff the contents of the response to decide if an error occurred, it should only care that it got or did not get a response from the server. It's up the the consumer to make decisions based on the contents of the response from the server.

The ConnectionBackend is the mail-man and the mail-man should not be reading your mail, just delivering it. Making decisions based on the content does not belong in this class.

[robwormald]

@danielgerlag I, in fact, agree with you that the ConnectionBackend itself shouldn't validate status codes, and only error on network failures - this is closer to how the fetch specification handles it, and it's likely I'll make that change and have Http itself do the status check.

That said, I'd encourage everybody in this thread to treat each other with civility and respect, per our code of conduct. Angular2 is evolving, and a little bit of patience goes a long way.

[escardin]

I'm happy with that solution.

[danielgerlag]

What do you mean by "Http itself do the status check"?
What if we removed the default "XHRBackend" from the HTTP_PROVIDERS injection and forced the user to choose their own ConnectionBackend implementation when setting up their DI?
Then we have an XHRBackend and an XHRErroringBackend?

[escardin]

@danielgerlag I'm pretty sure with it as it is now, you can still detect a connection failure. It is hard to come up with a sample proving it though. The best I could come up with was a gibberish url that got blocked by cors and gave a 200 response.

[robwormald]

What do you mean be "Http itself do the status check"?

XHRBackend will only error on network failures. Http will do a (configurable) status code check, similar to what the backend is currently doing. This is a happy middle path between the fetch-like low level behavior while maintaining the ng1 / jqXHR behavior of erroring on > 299 errors

[danielgerlag]

Awesome, thank you

[sinedied]

Has anything changed related to this? I'm having this exact issue while from what I understand it should have been fixed?

When the server returns a 404 error, it does not get through the error handler, but the "standard" handler.

I'm using @angular/http 2.4.8.

[DzmitryShylovich]

Has anything changed related to this?

nope https://github.com/angular/angular/blob/master/modules/%40angular/http/src/http_utils.ts#L33

[sinedied]

I found the issue: the MockBackend used for unit testing does not follow the same behavior as the XHRBackend used by the app. Should I raise a separate issue?

[DzmitryShylovich]

@sinedied duplicate of #12857 (comment) ?

[sinedied]

Oups, my bad!

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}