Promise won't resolve

Question

I'm new to node and I'm having an issue with resolving an async Promise. My promise isn't resolving and I'm not sure what I did wrong. I'm still having troubles understanding promises and callbacks so any feedback is helpful.

  var filterFiles = function(){
    return new Promise(function(resolve, reject){
      fs.readdir(rootDir, function(err, files){
        if(err) return console.log(err);

        var task = function(file){
          return new Promise(function(resolve, reject){
            if(! /^\..*/.test(file)){
              fs.stat(rootDir + '/' + file, function(err, stats){
                if(stats.isDirectory()){
                  dirArray.push(file);
                  console.log(dirArray.length);
                  resolve(file);
                }
                if(stats.isFile()){
                  fileArray.push(file);
                  console.log(fileArray.length);
                  resolve(file);
                }
              })
            }

          })
        };

        var actions = files.map(task);

        return Promise.all(actions).then(function(resolve, reject){
          resolve({dirArray: dirArray, fileArray: fileArray});
        });
      })
    })
  }

  filterFiles().then(function(data){
    console.log(data);
    var obj = {
      fileArray: fileArray,
      dirArray: dirArray
    };
    res.send(obj);
  })

Answer 1

It see at least three errors:

1. When you hit this if statement if(! /^\..*/.test(file)){ and it does not execute the if block, then the parent promise is never settled.

2. There is no error handling on fs.stat() so if you get an error on that call, you are ignoring that and will be attempting to use a bad value.

3. The error handling on your call to fs.readdir() is incomplete and will leave you with a promise that is never settled (when it should be rejected).

---

For a robust solution, you really don't want to be mixing promises and callbacks in the same code. It leads to the opportunity for lots of mistakes, particularly with error handling (as you can see you had at least three errors - two of which were in error handling).

If you're going to use Promises, then promisify the async operations you are using at the lowest level and use only promises to control your async code flow. The simplest way I know of to promisify the relevant fs operations is to use the Bluebird promise library with its Promise.promisifyAll(). You don't have to use that library. You could instead manually write promise wrappers for the async operations you're using.

Here's a version of your code using the Bluebird promise library:

const Promise = require('bluebird');
const fs = Promise.promisifyAll(require('fs'));

function filterFiles() {
    return fs.readdirAsync(rootDir).then(function(files) {
        let fileArray = [];
        let dirArray = [];

        // filter out entries that start with .
        files = files.filter(function(f) {
            return !f.startsWith(".");
        });
        return Promise.map(files, function(f) {
            return fs.statAsync(f).then(function(stats) {
                if (stats.isDirectory()) {
                    dirArray.push(f);
                } else {
                    fileArray.push(f);
                }
            });
        }).then(function() {
            // make the resolved value be an object with two properties containing the arrays
            return {dirArray, fileArray};
        });
    });
}



filterFiles().then(function(data) {
    res.json(data);
}).catch(function(err) {
    // put whatever is appropriate here
    res.status(500).end();
});

This was rewritten/restructured with these changes:

1. Use promises for all async operations
2. Fix all error handling to reject the returned promise
3. Filter out files starting with a . synchronously before processing any files (simplifies async processing).
4. Use Promise.map() to process an array of values in parallel.
5. In the filterFiles().then() handler, handle errors
6. You can't res.send() a Javascript object so I used res.json(data) instead (though I'm not sure what exactly you really want to send).
7. Replace regex comparison with more efficient and simpler to understand .startsWith().

---

If you don't want to use the Bluebird promise library, you can make your own promise wrappers for the fs methods you use like this:

fs.readdirAsync = function(dir) {
    return new Promise(function(resolve, reject) {
        fs.readdir(dir, function(err, data) {
            if (err) {
                reject(err);
            } else {
                resolve(data);
            }
        });
    });
}

fs.statAsync = function(f) {
    return new Promise(function(resolve, reject) {
        fs.stat(f, function(err, data) {
            if (err) {
                reject(err);
            } else {
                resolve(data);
            }
        });
    });
}

function filterFiles() {
    return fs.readdirAsync(rootDir).then(function(files) {
        let fileArray = [];
        let dirArray = [];

        // filter out entries that start with .
        files = files.filter(function(f) {
            return !f.startsWith(".");
        });
        return Promise.all(files.map(function(f) {
            return fs.statAsync(f).then(function(stats) {
                if (stats.isDirectory()) {
                    dirArray.push(f);
                } else {
                    fileArray.push(f);
                }
            });
        })).then(function() {
            // make the resolved value be an object with two properties containing the arrays
            return {dirArray, fileArray};
        });
    });
}


filterFiles().then(function(data) {
    res.json(data);
}).catch(function(err) {
    res.status(500).end();
});

Answer 2

The main issue you are having is that outer-most Promise is not resolved or rejected. You can fix this by resolving your Promise.all instead of returning it.

    resolve(
      Promise.all(actions)
        .then(function(resolvedTasks){
          // ... next potential issue is here
          return {dirArray: dirArray, fileArray: fileArray}
        })
    );

(I know, kind of awkward-looking right?)

Next, your return value after the Promise.all resolves is a little weird. In the task function, you're pushing items onto dirArray and fileArray, but they are not declared or assigned in your snippet. I will assume that they are in-scope for this code. In this case, you just need to return your desired object.

Additionally, to make your async code more readable, here are some tips:

• try not to mix callbacks with Promises
• use a Promise library to promisify any code limited to callbacks. Example: bluebird's promisifyAll
• avoid nesting callbacks/promises when possible