Angular $q execute "then" before http request

Question

I'm getting crazy with this since a couple of hours.

I have an angular service factory to get addresses from my API:

App.factory('storesService', ['$http', '$q', 'endpoint', function ($http, $q, endpoint) {

    var deferred = $q.defer();

    return {

        addresses: function (store_id) {
            $http.get(endpoint.store.addresses, {
                params: {
                    id: store_id
                }
            })
            .success(function (data) {
                console.log('Data from API:' + data);
                deferred.resolve(data);
            })
            .error(function () {
                deferred.reject();
            });
            return deferred.promise;
        }

    };

}]);

This service is used in my controller to get addresses of a specific store:

$scope.loadAddresses = function (store_id) {
    var load = storesService.addresses(store_id);
    load.then(function (data) {
        console.log('Deferred data:' + data);
        $scope.addresses = data.addresses;
    });
};

In my view I have the ng-init="loadAddresses(store_id)", store_id is a right value.

I'm also using angular-xeditable (select-local) to manage my store selection.

I added onaftersave='storeChanged(store.id)' in my view to get the store id selected by the user and it return correctly the new id.

my storeChanged function is very easy, it basically run a new request to the API:

$scope.storeChanged = function (store_id) {
    $scope.loadAddresses(store_id);
};

What happen:

At the beginning, with ng-init I see correctly the console.log, first the one from the service and then the one from the controller.

Once I select another store from my select I first see the console.log from the controller and then the one from the service.

Basically the data in the controller is not updated and I can not understand why it happen...

Thanks guys, I'm sure all your answers are correct but the one from @Reto is the one closest to my code. Thanks again. — Ayeye Brazo, Jul 24 '15 at 16:15
While it certainly works, keep in mind that one of the nice things about angular is that it works with promises out of the box. Built in async services return promises, and things like `resolve` expect them. `$q` is only really needed if you're dealing with grouping promises, `$q.all`, or your interfacing with async services that have not yet implemented promises. — Dylan Watt, Jul 24 '15 at 16:36
@AyeyeBrazo As Dylan said, your code and the chosen answer have an issue. I shared a link within my answer that details that issue, which is that you're creating another promise rather than using the one returned by `$http`. Your code "works", but it's unnecessary, less understandable, and harms performance. — m59, Jul 25 '15 at 15:27

score 1 · Answer 1 · edited May 23 '17 at 12:14

You're trying to re-resolve a promise, which you can't do. You only create one deferred that every request uses. That should be inside of the addresses function so that a new one is created for each request, but you don't need it anyway because $http creates and returns a promise already. You need to return the promise from $http rather than creating a new one. See this post for a better understanding: What is the explicit promise construction antipattern and how do I avoid it?

addresses: function (store_id) {
  return $http.get(endpoint.store.addresses, {
    params: {
      id: store_id
    }
  }).then(function(resp) {
    console.log('Data from API:' + resp.data);
    return resp.data;
  });
}

score 1 · Accepted Answer · answered Jul 24 '15 at 16:06

You defined your deferred globally in the service, so there is only one global promise. Because a promise can only be resolved or rejected once, it will stay resolved/rejected forever after your first http call. To fix simply move the line var deferred = $q.defer(); into your service function:

App.factory('storesService', ['$http', '$q', 'endpoint', function ($http, $q, endpoint) {

return {

    addresses: function (store_id) {

        var deferred = $q.defer();

        $http.get(endpoint.store.addresses, {
            params: {
                id: store_id
            }
        })
        .success(function (data) {
            console.log('Data from API:' + data);
            deferred.resolve(data);
        })
        .error(function () {
            deferred.reject();
        });
        return deferred.promise;
    }

  };

}]);

This answer isn't complete. This code still has the promise construction anti-pattern and should not be used. — m59, Jul 25 '15 at 15:25
this comment is correct, this answer is not the best solution for wrapping a $http call in a service. But it shows explicitly where the mistake in the OPs code was. In general directly returning the $http promise is certainly better. — Reto, Jul 25 '15 at 16:11

score 1 · Answer 3 · answered Jul 24 '15 at 16:07

You've created one defer for potentially many requests. The first time you make a request it will work, but after than it will instantly return, as the one promise you've set up has already resolved. Patterns smilar to this can be very useful for caching, actually.

$http already returns a promise. You don't need to go out of your way to use $q

App.factory('storesService', ['$http', 'endpoint', function ($http, endpoint) {

    return {

        addresses: function (store_id) {
           return $http.get(endpoint.store.addresses, {
                params: {
                    id: store_id
                }
            }).then(function(response){
               //Chain an extra promise here to clean up the response to just return the data.
               return response.data;
            })
        }

    };

}]);

Angular $q execute "then" before http request

3 Answers3