Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

$http .success/.error return original promise, dissimilar from how .then works. Clarify in docs? #10508

Closed
@glebec

Description

@glebec

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?

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions