[Docs] Document that non-2xx HTTP status codes do not trigger error callback

[jinder]

I'm using traverson-angular with the useAngularHttp() option, and non-success HTTP codes don't activate the error callback:

 this.traverson
            .from(this.baseUrl)
            .useAngularHttp()
            .follow()
            .post({ /* payload */ })
            .result
            .then(() => {   /* Called even on failure */  }, () => { /* never called on failure */});

This occurs in Angular 1.4.3 (I've not tested older versions).

[basti1302]

There are basically two different approaches to classifying HTTP status codes when it comes to libraries handling AJAX requests:

1. Interpret all status codes outside the range 200 - 299 as an error. In this approach, you call the error callback or reject the promise when a 4xx or 5xx happens.
2. Do not interpret the HTTP status code at alll, that is, only call the error callback/reject the promise if the host is not reachable, a network error occurs or the request times out or something like this.

There are well known libraries that are examples for both approaches. A quite popular AJAX library recently even changed their approach to that, see ladjs/superagent#283 for a lenghty discussion about the pros and cons of both approaches.

AngularJS uses the approach 1) while Traverson (and thus, traverson-angular) uses approach 2).

.useAngularHttp() is there to enable developers to use AngularJS http interceptors etc. I think it would not be a good idea when .useAngularHttp() would change how HTTP status codes are interpreted.

This also plays into our discussion over at traverson/traverson#44

If I tell Traverson/traverson-angular that I expect a JSON response and that I want it already transformed into a JS object, than a 4xx or 5xx status could probably interpreted as an error right away. If I simply issue a GET/POST/... on an URL, Traverson should not try to guess if any non 2xx status code is an error or maybe an expected outcome.

[jinder]

I think this makes the error handling slightly more complex for most typical use cases although I completely understand the reasoning. Could you clarify this in the documentation? It was a bit of a surprise to me and caused all sorts of weirdness when I presumed the error callback was being called! :)

[basti1302]

You are right, this should be documented in a prominent place. Also for https://github.com/basti1302/traverson, I think.

Thanks for pointing it out.

[basti1302]

Note to self/for future reference: If we would like to implement HTTP status checking (might be reasonable with getReource() or post/put/patch/delete when automatic JSON parsing is enabled) then https://github.com/angular/angular.js/blob/master/src/ng/http.js#L235 would be the reference implementation for this kind of check:

function isSuccess(status) {
  return 200 <= status && status < 300;
 }

[basti1302]

You know what's weird? Triggered by this issue I started to transfer some integration tests (that talk to a real server) from traverson over to traverson-angular. I also copied your test case snippet. For me, it calls the errorCallback on 404. Which is wrong in the context of Traverson, so I'm going to fix it, but anyway, I really wonder what's happening here.

[cwmrowe]

I find this a bit counter intuitive. I think if you choose to apply useAngularHttp(), then it should resolve and reject in the same circumstances as angular's $http.

[basti1302]

That's a valid viewpoint. From my perspective however, I would like Traverson to always behave in the same way, no matter which HTTP lib is used internally.