AngularJS http promise firing 4 times

Question

I am using AngularJS to call an http service that returns some opening times in an object. I don't understand why, in my controller, the console.log is printed 4 times, instead of one time. Can anyone explain this to me?

Here is my service/factory code:

myApp.factory('BookingFactory', ['$http', '$q', function($http, $q) {
    var deferredTime = $q.defer();

    return {
        GetDealerLocationTimeList: function(websiteId) {
            return $http.get('/d/GetDealerLocationTimes?website_id=' + websiteId)
                .then(function(response) {
                    deferredTime.resolve(response.data);
                    dealerLocationTimeList.push(response.data);
                    return deferredTime.promise;
                }, function(error) {
                    deferredTime.reject(response);
                    return deferredTime.promise;
                });
        }
    }
}]);

Here is my controller code that is calling the service:

var promise = BookingFactory.GetDealerLocationTimeList(website_id);

promise.then(
    function(da) {
        $scope.dealerLocationTimeList = da;
        console.log($scope.dealerLocationTimeList);
    },
    function(error) {
        $log.error('failure loading dealer associates', error);
    }
);

Apart from 4 logs through one promise being hardly believable, what is this `deferredTime` thing supposed to do? Looks close to the [deferred antipattern](http://stackoverflow.com/q/23803743/1048572) to me, and in the error case there isn't even a `response` in scope. — Bergi, Oct 26 '15 at 02:14
Are you sure this is the only `console.log` statement in your code? — Bergi, Oct 26 '15 at 02:14
Bergi, I got this way of doing promises from an example somewhere. If I'm not correctly doing an async call, can you please tell me the correct way? I'm pretty new to Angular. — CaptainMorgan, Oct 26 '15 at 02:48

Icycool · Accepted Answer · 2015-10-26T09:55:09.793

There are many mistakes in this code ><

If you want to use deferred, then this should be the code:

myApp.factory('BookingFactory', ['$http', '$q', function($http, $q) {
    return {
        GetDealerLocationTimeList: function(websiteId) {
            var deferredTime = $q.defer(); // deferred should be created each time when a function is called. It can only be consumed (resolved/rejected) once.

        /* return - don't need to return when you already creating a new deferred*/ 
            $http.get('/d/GetDealerLocationTimes?website_id=' + websiteId)
                .then(function(response) {
                    deferredTime.resolve(response.data);
                    // dealerLocationTimeList.push(response.data);
                }, function(error) {
                    deferredTime.reject(error); // it should be 'error' here because your function argument name says so...
                });

           return deferredTime.promise; // promise is returned as soon as after you call the function, not when the function returns
        }
    }
}]);

But it is a better practice to return the promise if your inner function is a promise itself (like $http.get)

myApp.factory('BookingFactory', ['$http', '$q', function($http, $q) {
    return {
        GetDealerLocationTimeList: function(websiteId) {
            // no need to create new deferred anymore because we are returning the promise in $http.get
            return $http.get('/d/GetDealerLocationTimes?website_id=' + websiteId)
                .then(function(response) {
                    // dealerLocationTimeList.push(response.data);
                    return response.data; // return the data for the resolve part will make it available when the outer promise resolve
                }/* this whole part should be omitted if we are not doing any processing to error before returning it (thanks @Bergi)
                , function(error) {
                    return $q.reject(error); // use $q.reject to make this available in the reject handler of outer promise
                }*/);

            // no need to return promise here anymore
        }
    }
}]);

You can see I've also commented your dealerLocationTimeList.push(response.data). In this case you should push the data into your scope variable on the outer layer (in the promise.then), because dealerLocationTimeList is not available in the factory.

promise.then(
    function(da) {
        // you might want to do an isArray check here, or make sure it is an array all the time
        $scope.dealerLocationTimeList.push(da);
        console.log($scope.dealerLocationTimeList);
    },
    ...
);

Thanks for the tips. I've realised my controller is being called multiple times, which is why I got the multiple outputs from console.log. I've removed all references to my BookingController, except the one. But it's still being called twice. Not sure if it's a routing issue caused by angular-ui-router. — CaptainMorgan, Oct 26 '15 at 03:58
You can use break points in developer's tool to check whether a specific part of code is called multiple times. — Icycool, Oct 26 '15 at 04:00
Notice you can (and should) simply omit the `function(error) { $q.reject(error); }` callback to `then`, so that the error is propagated automatically. Or if not, add the missing `return` to it. — Bergi, Oct 26 '15 at 09:39
@Bergi yes in this case the reject part should be omitted. Updated answer to reflect it. — Icycool, Oct 26 '15 at 09:53

AngularJS http promise firing 4 times

1 Answers1