1

I am having an issue with the order in which promises/functions get called. Basically I fetch an array of ID and then, for each id, want to fetch order detail and save. Then on to the next ID. As of now it will not save until every single order detail has been fetched.

The code:

// Convenience function
var fetchOrders = function() {
  return self.fetchOrderList()
    .then(function(orders) {
      return P.map(orders, function(r) {
        return self.fetchOrderById(r.id).then(fn);
      });
    });
};

// Fetch initial order list
var fetchOrderList = function() {
  return new P(function(resolve, reject) {
    debug('Fetching initial list');
    resolve([{
      id: 123
    }, {
      id: 134
    }, {
      id: 1333
    }]);
  });
};


var fetchOrderById = function(id) {
  return new P(function(resolve, reject) {
    debug('Fetching by ID');
    resolve({
      foo: 'bar'
    });
  }).then(function(o) {
    debug('Saving');
    return o;
  })
};

fetchOrders();

Expected result:

Fetching initial list
Fetching by ID 
Saving
Fetching by ID 
Saving
Fetching by ID 
Saving

Actual results

Fetching initial list
Fetching by ID 
Fetching by ID 
Fetching by ID 
Saving
Saving
Saving
cyberwombat
  • 38,105
  • 35
  • 175
  • 251
  • Please describe what the desired order of execution is. You first get a list of orderIDs, then you have to get detail on each and save that detail to a DB. What operations do you want run in parallel and which in sequence? And, you mention throttling - what do you want throttled (which usually means slowed down)? And, why do you reassign `queue` inside of `doCall()`, but never used the reassigned variable - what is the point of that? And, what is the point of the delay? Rather than describing your code (which seems overly complicated to me), please describe a simple spec for what you want. – jfriend00 Sep 24 '15 at 23:25
  • The queue works fine - each call to `doCall` should be running through the queue. Expectations is as listed above. 1) fetch array of ids (queued). 2) for each id in turn do queued call to get detail and save detail to db (dummy call). That is not what is happening. its not doing what would be the db call until each and every entry in initial array of orders has got the details fetched. – cyberwombat Sep 24 '15 at 23:30
  • Sorry, it may be easy for you to see what you want and it sounds like a fascinating problem to tackle, but I honestly don't understand what you want. I'd like to see a spec description of the exact outcome you want without referring to your existing code. – jfriend00 Sep 24 '15 at 23:32
  • @jfriend00 - cleaned it all up - the queue turned out to be irrelevant to issue. – cyberwombat Sep 25 '15 at 00:01

1 Answers1

1

OK, I understand why it is doing what it is doing. First, I'll explain why, then discuss possible changes.

In your test code and with no options set for P.map(), it will synchronously call all the fetchOrderById() operations one after another so they would all be in flight at the same time (if they were real async operations).

So, you've launched all the fetch by ID operations for all the orders. Then, as each one of those resolves, they will then run the save. In the test code, they all resolve immediately. But, by Promise specification all .then() handlers are called asynchronously AFTER all synchronous code has unwound and finished (technically, I think it uses something like nextTick() to schedule each .then() handler).

So, you see all the fetching by ID happen, they all get resolved synchronously, which queues all the save operations to run at nextTick. Thus you get the actual results you see.

If the fetchByOrderId() was actually asynchronous (it resolved a bit later), you will still see all the fetches start right away - all those requests would be launched and then saving would happen as each fetch by id arrived.

If you really want to change it to Fetch by ID/save, Fetch by ID/save, Fetch by ID/save, I can think of a good way to structure that, but that will necessarily sequence all the operations and take longer to complete (no parallel network operations).

FYI, I put your code into a running jsFiddle here for testing purposes: http://jsfiddle.net/jfriend00/k49qbd3z/. I just had to remove one .then(fn) because there was no such fn in the code you provided.


You can get the requested output by adding {concurrency: 1} to P.map() to force it to only do one operation at a time.

// Convenience function
var fetchOrders = function() {
  return fetchOrderList().then(function(orders) {
      return P.map(orders, function(r) {
        return fetchOrderById(r.id);
      }, {concurrency: 1});
    });
};

See working demo here: http://jsfiddle.net/jfriend00/wshLu0g5/ with this output:

Fetching initial list
Fetching by ID
Saving
Fetching by ID
Saving
Fetching by ID
Saving

Keep in mind that this will likely seriously slow down the end result because you're serializing all fetching by id and saving to only one operation at a time. Fetch data for one id, then save it, then fetch data for next id, then save it, then fetch data for next id, then save it and so on.

jfriend00
  • 683,504
  • 96
  • 985
  • 979