Nodejs Promise.all() resolving always

Question

I am new to promises. I am trying to ping some machines to check whether they are active. I am using native NodeJS promises. My ping function :

function ping(addr) {
    return new Promise(function(resolve, reject) {
        var args = ['-n', '1', '-w', '5000'];
        args.push(addr);
        var ls = cp.spawn('ping.exe', args);

        ls.on('error', function (e) {
            reject(Error('There was an error while executing the ping'));

        });

        ls.on('exit', function (code) {
            if(code === 0) {
                resolve({host: addr});
            } 
            else {
                reject(Error(addr + " is  down!"));
            }

        });
    });

}

Now, I have details of machines in an array read from a JSON:

gulp.task('pingNodes', ['readConfigJSON'], function () {

    var batches = ConfigJSON.NodeDetails.Batch1.concat(ConfigJSON.NodeDetails.Batch2);
    var pingPromises = batches.map(function (host) {
        return ping(host.Name)
            .then(function (res) {
                console.log(res.host + " is  up!");
            }).catch(function (err) {
                console.log(err);
            });
    });
    return Promise.all(pingPromises).then(function(){console.log("All nodes are up!")});
});

Now it doesn't reject even if some node is down:

[16:58:46] Starting 'init'...
Starting Deployment
[16:58:46] Finished 'init' after 135 µs
[16:58:46] Starting 'readConfigJSON'...
[16:58:46] Finished 'readConfigJSON' after 204 µs
[16:58:46] Starting 'pingNodes'...
machine1 is  up!
machine2 is  up!
machine3 is  up!
[Error: machine4 is  down!]
All nodes are up!
[16:58:49] Finished 'pingNodes' after 2.54 s

Answer 1

Solution

To fix this problem, throw the error again in the catch handler, like this

}).catch(function (err) {
    console.log(err);
    throw err;
});

or remove the catch handler. Basically, you should let the rejection flow down the chain, so that Promise.all gets a rejected promise if a machine is down.

---

Basic understandings

1. All the then and catch handlers create a new Promise object and return them, so that we can chain them.

2. When a Promise is rejected, a rejection handler will handle it, but if the rejection handler doesn't reject the promise, then the subsequent handler in the chain, will not treat the promise as rejected.

In your case, when the machine is down, you are rejecting it and the rejection is handled in,

}).catch(function (err) {
    console.log(err);
});

But, the catch handler returns a promise which is is not rejected. So, Promise.all actually gets a Promise object which is not rejected. That is why it is not stopping as soon as it finds out that one of the machines is down.

You can confirm this understanding with the following program

var a = Promise.resolve(1)
    .then(function (e) {
        throw e;
    })
    .catch(function (e) {
        // This is what you are doing if the machine is down
        console.log(e);

        // No rejection here, so `then` will be called.
        // Uncomment the throw to see 
        // throw e;
    });

a.then(function (e) {
    console.log("Inside then")
});

a.catch(function (e) {
    console.log("Inside catch")
});

---