0

looking for some help on an issue that makes me losing my hair ! :)

I have to send a series of call to an API I consume. so I have created a factory with a function like

addItem : function(){
  var deferred=$q.defer();
  //call to the API
    .then(function(response){
      deferred.resolve(process(response.data));
    }, function(response){
      deferred.reject(errorManagement(response.status));
      });
    }
  return deferred.promise;
}

Then I have built an array with code:

for(var i=0; i<nbOfElements; i++) {
  arrayOfPromises[i]=Factory.addItem();
}
$q.all(arrayOfPromises).then(..)

My expectation is that the $q.all will resolve only when all calls to the API have been completed. Unfortunately, it appears that this is not the case, and then I display a partial result, not satisfactory

after some debugging, it appears that the promises returned by the factory all have $$state.status = 1, which seems to be "resolved" state, explaining why the $q.all resolved before I would like. (link to the values of $$state.status)

Still I find this weird as I have used this $q.defer() a lot, but without $q.all and it always worked fine

Any explaination on this issue and how to solve it would be much welcome :)

Community
  • 1
  • 1
Nicolas
  • 75
  • 9
  • What does `process(response.data)` and `errorManagement(response.status)` do? Can you please provide the code for them as well. – Brendan Green Sep 01 '15 at 03:29
  • If you are seeing only partial results, then you must be suspicious that at least one of the `arrayOfPromises` settled with a rejection - ie an error occurred. To see what is going on, try logging errors in errorManagement()` and/or in the chained `.then()` after `$a.all(...)`. – Roamer-1888 Sep 01 '15 at 12:39
  • In fact I checked in the console and it looks like all promises are resolved ($$state===1) before resolution... Weird ... Definitely weird ! – Nicolas Sep 01 '15 at 15:33
  • The reference you link to says you need to inspect `promise.$$state.status` not `promise.$$state`. – Roamer-1888 Sep 01 '15 at 19:27
  • yes that's an error, I meant $$state.status, I'm going to edit my comment on this – Nicolas Sep 01 '15 at 22:05

2 Answers2

1

Maybe I can't solve your problem, just giving suggestions:

You can save a lot of code by returning the promise of your API call instead of creating new promises (this is one of the promise anti-patterns)

addItem: function() {
    //call to the API
    return $http(something).then(function(response){
      return process(response.data);
      // if you happen need to reject here, use $q.reject(reason/data);
    }, function(response){
      return $q.reject(errorManagement(response.status));
    });
}

I'm more comfortable on assigning into an array using push

var arrayOfPromises = [];
for(var i=0; i<nbOfElements; i++) {
  arrayOfPromises.push(Factory.addItem());
}
$q.all(arrayOfPromises).then(..)

You are not missing the part before .then in the addItem function in your real code, are you?

Icycool
  • 7,099
  • 1
  • 25
  • 33
  • 1
    Careful with the error handler. You probably want `return $q.reject(errorManagement(response.status))` (assuming that `errorManagement()` doesn't itself return a rejected promise) – Phil Sep 01 '15 at 03:53
  • @Phil In my experience both process and errorManagement doesn't have to be a promise - they can be normal functions. – Icycool Sep 01 '15 at 04:02
  • 1
    Yes, but OP appears to want to still return a rejected promise in the case of an HTTP error. If you don't use `$q.reject`, `addItem` will always return a successful promise, even if an error occurs. – Phil Sep 01 '15 at 04:15
  • @Phil Thanks, I've been misunderstanding how the return work in the second function of then. Edited answer for the correct way. – Icycool Sep 01 '15 at 05:14
  • I will try this way tonight, thanks a lot. Will accept your answer then if it works ! Thanks again for the help. – Nicolas Sep 01 '15 at 15:35
1

Simply return the $http.get() promise will actually work: JSFiddle.

If you want to do some pre-processing in the factory before returning the data (like your process and errorManagement), check demo: JSFiddle.

The $q.all callback function not invoked only after all promises are resolved. It is not because of $q.all.

Your array code has problem: arrayOfPromises[i]=Factory.addItem();. You'd better use push to expand the array.

Joy
  • 9,430
  • 11
  • 44
  • 95
  • thank you for your answer, I am looking at the second fiddle which works exactly as I would like, but I struggle to identify the reason why ! I'm going to dive into this to understand . thanks again for your help – Nicolas Sep 01 '15 at 22:04
  • Oh I've just understood what I had done wrong, some stupid mistake ! in my actual code, I was creating the array within another then() and the $q.all was outside of this a then, so was no asynchronous, and therefore was evaluating at the time it was executed based on an empty array... which was obviously resolved :D – Nicolas Sep 01 '15 at 22:48
  • In any case, thanks again for you help, I'm going to select your answer as it is still a good use case for anybody discovering the $q.all() function ! – Nicolas Sep 01 '15 at 22:49
  • The second fiddle falls into one of the promise anti-patterns, the forgotten promise. Not that it won't work, just that there is many redundant code and there is a cleaner way of doing it. [Reference here](http://taoofcode.net/promise-anti-patterns/#the_forgotten_promise) – Icycool Sep 02 '15 at 03:24