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

fix($http): promise chain for helper methods #11097

Closed
wants to merge 1 commit into from

Conversation

PatrickJS
Copy link
Member

can has less promise chain breaking

angular.module('app', [])
.factory('Auth', function($http) {
  return {
    login: function(data) {
      return $http.post('/signup', data)
        .success(function(data) {
          return data;
        })
        .error(function(err) {
          return $q.reject(err);
        });
    } // end login
  }; // end module
}) // end factory
.controller('MainCtrl', function($scope, $state, Auth) {

  $scope.login = function(user) {
    Auth.login(user).then(function(data) {
      $state.go('dashboard');
    })
    .catch(function(err) {
      alert(err);
    });
  }; // end login
}); // end ctrl

since the promise chain is broken when using Auth.login().then(otherStuff).catch(errorStuff) you get the result from the http and not the error from .error.

can has less promise chain breaking

```javascript
angular.module('app', [])
.factory('Auth', function() {
  return {
    login: function(data) {
      return $http.post('/signup', data)
        .success(function(data) {
          return data;
        })
        .error(function(err) {
          return $q.reject(err);
        });
    } // end login
  }; // end module
}) // end factory
    
```
since the promise chain is broken when using `Auth.login().then(otherStuff).catch(errorStuff)` you get the result from the http and not the error from `.error`.
@gkalpak
Copy link
Member

gkalpak commented Feb 18, 2015

Based on the discussion at #10508 (especially #10508 (comment)), it seems like success() and error() are heading for deprecation.

Note that fixing their "brokenness" would be quite of a breaking change.

@pkozlowski-opensource
Copy link
Member

I'm not big fan of this change - for a start it breaks the existing tests, so this would have to be looked into. Besides it tries to pretend that success and error are promises (or promise-like) while those are ordinary callbacks.

I think that the current situation - although broken - is easier to explain: then is for promises while error / success calbacks are ... plain old callbacks that can be used by people who don't want mess with promises.

My best advice - let go of error / success....

@Narretz
Copy link
Contributor

Narretz commented Feb 20, 2015

I agree with @pkozlowski-opensource . Let's not shoehorn promise behavior onto callbacks.

@pkozlowski-opensource pkozlowski-opensource modified the milestones: Ice Box, Backlog Feb 20, 2015
@pkozlowski-opensource
Copy link
Member

Yeh, thnx for the suggestion @gdi2290 but we are going to pass on this one...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants