Async function works when called once but not multiple occasions

Question

I have used async.waterfall to flatten out a nested function as per the below.

function convertPortfolio(trademarks, fn){
    async.waterfall([function(callback){
        groupByCountry(trademarks, callback)
    },
    function(TMsGroupedByCountry, callback){
        addEUTrademarks(TMsGroupedByCountry['EU'], TMsGroupedByCountry, callback)
    },
    function(revisedGroupByCountry, callback){
        groupTrademarksByStatus(revisedGroupByCountry, callback)

    }], function(err, splitByStatus){
        fn(null, splitByStatus);
    })
}

Nested alternative

function convertPortfolio(trademarks, fn){
    groupByCountry(trademarks, function(err, TMsGroupedByCountry){
        addEUTrademarks(TMsGroupedByCountry['EU'], TMsGroupedByCountry,   function(err, revisedGroupByCountry){
            groupTrademarksByStatus(revisedGroupByCountry, function(err, splitByStatus){
                 fn(null, splitByStatus)
            });
        });
    });
 }

Subsequently when I call this function once as part of another function, it works perfectly. However when I attempt to call the function multiple times using a separate value from a forEach call on an array, it fails to work, when the nested version works fine. I am at a loss to explain why this would be the case and in all honesty, I'm not sure where to start. My understanding is that the forEach call should simply mean that each separate value is processed accordingly and closed over so this shouldn't be the issue.

Async function works correctly and returns value in this instance

exports.convertPortfolioAndAddToGeoJSON = function(geojson, trademarks, fn){
   convertPortfolio(trademarks, function(err, revisedTMs){
    addToGeoJson(geojson, revisedTMs, function(err, gj){
        fn(null, gj)
      });
   });
}

But in this instance the end target object is not populated:

exports.convertYearPortfolioAndAddToGeoJSON = function(geojson, trademarks, fn){
  var target = {};
  Object.keys(trademarks).forEach(function(year){
    convertPortfolio(trademarks[year], function(err, revisedTMs){
        addToGeoJson(geojson, revisedTMs, function(err, revisedGeoJSON){
            target[year] = revisedGeoJSON;
            });
        });
    });
    fn(null, target);
  }

Using console.log at certain points shows that in the nested example, the return values are logged prior to the target object being logged, whereas with the async.waterfall example, the target object is logged prior to the returned data being available (so I suppose it is not surprising that logging the target results in an empty object).

I thought in each case that the presence of callbacks, would mean that the logging of target would only take place once all previous processed had been completed, but although this appears to be the case with the nested alternative, this is not so with the async version at least when it is called multiple times.

Any guidance would be appreciated.

UPDATE Out of interest here's the revised code using async.forEach:

exports.convertYearPortfolioAndAddToGeoJSON = function(geojson, trademarks, fn){
  var target = {};
  async.forEach(Object.keys(trademarks), function(year, callback){
    async.waterfall([
        async.apply(convertPortfolio, trademarks[year]),
        function(revisedTMs, callback){
            addToGeoJson(geojson, revisedTMs, callback)
        }], 
        function(err, revisedGeoJSON){
            target[year] = revisedGeoJSON;
            callback()
        })
   }, function(err){
       fn(null, target);
   });
}

Answer 1

Object.keys(trademarks).forEach is synchronous and doesn't properly track closures/scope. You need to use async.forEach(Object.keys(trademarks), function(year, callback).... there, and adjust the async control flow accordingly.

Also just FYI when you have this pattern with a tiny wrapper function:

function convertPortfolio(trademarks, fn){
  async.waterfall([function(callback){
    groupByCountry(trademarks, callback)
  },

You can use async.apply for that boilerplate:

function convertPortfolio(trademarks, fn){
  async.waterfall([async.apply(groupByCountry, trademarks),
     ...