Search code examples
javascriptpromisebluebird

Promise.map.map chain fails when there's concurrency but passes unit tests


I naively assumed I can just chain a .map() off a Promise.map() but it appears I'm wrong. I get the annoying issue of passing unit tests but failing real world tests when concurrency is high. I think this makes this a gotcha worth noting on Stack Overflow.

I changed the code to use the usual .then() pattern and it appears to work every time now.

Is the issue here that unlike arrays in JS I cannot chain a .map() off a Promise.map() or was I doing something else wrong? I note I cleaned up a few lint issues in the working version, but that doesn't seem to have made a difference in behavior.

Below is code that fails randomly (when concurrency is high) and one that appears to work all the time in low concurrency unit tests. Note all functions called return promises.

// this has concurrency issues. occasionally the function
// returns [{ key: undefined: val: correct }]
db.assocThreeFewCountsGet = function(manyid, atype) {
  counts = [];
  return db.assocThreeFewDistinctGet(manyid, atype)
  .then(r => {
    console.log('Distinct', r);   // shows there are valid IDs
    counts = r;
    return Promise.map(r, fewid => {
      return db.assocCount(manyid, fewid);
    }).map((el, idx) => {
      return { key: counts[idx], val: el };
    });
  });
};

// this appears to work correctly.  key: and val: are correct
db.assocThreeFewCountsGet = function(manyid, atype) {
  var ids;
  return db.assocThreeFewDistinctGet(manyid, atype)
  .then(r => {
    ids = r;
    console.log('Distinct IDs:', ids);  // shows there are valid IDs
    return Promise.map(ids, fewid => {
      return db.assocCount(manyid, fewid);
    });
  }).then(counters => {
    return counters.map((el, idx) => {
      return { key: ids[idx], val: el };
    });
  });
};

Solution

  • That counts = []; assignment is suspicious and seems to create a global variable. That would cause all sorts of issues, including counts referring to the latest array in concurrent executions of the asynchronous method. Also in your working code, you have var ids for the same purpose, which is local to each call.

    If you don't need counts anywhere else, fix your code by using

    db.assocThreeFewCountsGet = function(manyid, atype) {
      return db.assocThreeFewDistinctGet(manyid, atype)
      .then(counts => {
        console.log('Distinct', counts);   // shows there are valid IDs
        return Promise.map(counts, fewid => {
          return db.assocCount(manyid, fewid);
        }).map((el, idx) => {
          return { key: counts[idx], val: el };
        });
      });
    };