asynchronously iterate over a group of files using promises/defer

Question

My goal is to iterate over a directory of files, run some operations on each file, and return a json object containing a subset of the directory.

I got it working using the synchronous version of calls in Node's fs library, but I'd like to figure out the best async solution. My failed attempt at an async version is below using the Q library to defer. No matter what I do, I cannot get the final step to defer until the iterations have completed.

The iterations complete successfully, but not before sendResponse() is called.

Can anyone help me understand what I'm doing wrong?

router.get('/mediaTree', function(req, res){
  var mediaTree = { "identifier" : "id", "label" : "name", "items" : []}; 
  var idCounter = 1;

  var fs_readdir = q.denodeify(fs.readdir);

  fs_readdir(MEDIAPATH)
    .then(function(files) {
        files.forEach(function(dir) { 
          fs.stat(MEDIAPATH + dir, function(err, stats) {
            if(err) { console.log(err); return; }

            var thisFile = {};
            if(stats.isDirectory()) {
             thisFile.id = idCounter++;
             thisFile.type = "branch";  
             thisFile.name = dir;
             thisFile.path = MEDIAPATH + dir;
             mediaTree.items.push(thisFile);  
            } 
          });  
        }); 
    })
   .then(sendResponse);


  function sendResponse() {  
    res.json(mediaTree);
  }
});

Answer 1

To make your above code work properly, you have to use promises to their full extend. Please see MDN or similar sources on how promises work in detail.

Given this, you should wrap fs.stat in a promise as well. That way the promise manages waiting for the results for you and gives you an option to run most of your code in a more sync matter.

var q = require('q'); // implied from your code sample
var path = require('path'); // joining paths with "+" might fail

router.get('/mediaTree', function(req, res){
    var qreaddir = q.denodeify(fs.readdir);
    var qstat = q.denodeify(fs.stat); 

    qreaddir(MEDIAPATH)
    .then(function(files) {
        // from inside out
        // - a promise for the fs.stat of a single file,
        // EDIT: Containing an object ob both the result and the dir
        // - an array of these promises created via Array.prototype.map from the files
        // - a promise from this array
        return q.all(files.map(function(dir) {
            return qstat(path.join(MEDIAPATH, dir))
                .then(function(stat) { // EDIT: extending the original answer
                    return {
                        dir: dir,   // the dir from the outer outer scope
                        stat: stat, // the stats result from the qstat promise
                    };
                });
        }));
    })
    .then(function(results) {
        // Promises should have no side effects, declare vars in function scope
        var mediaTree = {
            identifier: "id", 
            label: "name", 
            items: []
        };
        var idCounter = 1;

        // since we now have a sync array, we can continue as needed.
        results.forEach(function(result) {
            // EDIT: The original answer had the stats as immediate result
            // Now we get an object with both the dir and it's stat result.

            var stats = result.stats;
            var dir = result.dir; // Use this as needed.

            if (stats.isDirectory()) {
                var thisFile = {};

                thisFile.id = idCounter++;
                thisFile.type = "branch";  
                thisFile.name = dir;
                thisFile.path = path.join(MEDIAPATH, dir);

                mediaTree.items.push(thisFile);  
            }
        });

        return res.json(mediaTree);
    })
    .catch(function(err) {
        // Failsafe. Will log errors from all promises above. 
        console.log(err);
    });
});