-
Notifications
You must be signed in to change notification settings - Fork 27.3k
$http .success/.error return original promise, dissimilar from how .then works. Clarify in docs? #10508
Description
Hello. $http returns a normal promise for a server response object (with data, status, headers etc.). As a normal promise, returning values from a handler in .then results in a new exported promise for the value (or assimilation, if the return value is a promise, or rejection in case of error, etc.). That is all well and good.
For convenience/abstraction, AngularJS provides two custom methods on promises returned by $http: .success and .error. Each takes a single handler with the data, status etc. as parameters.
The problem is that people familiar with promises will likely try to chain off of .success, perhaps by transforming and returning new values or new promises. However, .success does not return a new promise; rather, it returns the original promise:
angular.module('promisesApp')
.controller('MainCtrl', function ($http, $log) {
$http.get('/api/things')
.success(function (data) {
$log.debug('in success:', data); // these are the things from the API
return 'a new value';
})
.then(function (data) {
$log.debug('in chained .then:', data);
// data is the original server response object, NOT 'a new value'
// data.data are the things from the API
});
});If you were trying to make .success chainable in the same way .then is, it would be easy to change in the AngularJS source, and indeed I had started work on a pull request:
// in theory, could change this:
promise.success = function(fn) {
promise.then(function(response) {
fn(response.data, response.status, response.headers, config);
});
return promise;
};
// to this:
promise.success = function(fn) {
return promise.then(function(response) {
return fn(response.data, response.status, response.headers, config);
});
};However, I then realized that this would mean you could not attach both .success and .error to a single promise. The reason you can write this code:
$http.get('api/things')
.success( successHandler )
.error( errorHandler )…is because .success and .error return the original promise, not a new promise representing the result of the previous method.
This leaves me in a bit of a quandary. The recommendation must therefore be that .success and .error are only suitable as "end of the road" promise consumers, when you do not intend to do any more post-processing / chaining. If that is the case, I think the AngularJS docs should reflect this, and make it explicitly clear that chaining off of .success/.error does not work in the normal promises fashion, since the original promise is returned.
What do you think?