Promise from an array of promises in NodeJS Deferred?

Question

I want to make a module that outputs a set of metrics about the health of my application, such as background queue lengths, response time to service dependencies etc. This is Node JS using Deferred:

var metrics = {
    queueLength: function(def) {
        // .. Do some stuff to resolve the queue length ..
        def.resolve(45); // Example
    }
    // ... more metrics
}
for (i in metrics) {
    def = deferred();
    metrics[i](def);
    promiselist.push(def.promise);
    def.promise(function(result) {
        metrics[i] = result;
    }
}
return deferred(promiselist)(function(result) {
    console.log('All metrics loaded', result, metrics);
});

This produces the output

Metrics loaded [ [Function] ]  { queueLength: [Function] }

When I would have expected:

Metrics loaded [ 45 ]  { queueLength: 45 }

I think I'm doing two things wrong but don't know how to correct them 'properly':

The return deferred([array of promises])(group promise) idea doesn't seem to work
I've just realised def is getting reused on each iteration so if I had multiple metrics it would probably only track the last one.

Mariusz Nowak · Answer 1 · 2013-10-13T12:49:45.963

1

I agree with most of things Bergi pointed. You shouldn't destroy metrics methods and you should create and return promises internally within them.

There's some other improvements you can do:

There's deferred.map (named as counterpart to [].map) which is dedicated for lists and arrays, you can use it directly to resolve array of promises:

deferred.map(promiselist).then(/* ... */)

Additionally you can improve composition if you use deferred.map in its full and also replace for..in loop:

var result = {}; 
deferred.map(Object.keys(metrics), function (name) {
  return metrics[name]().aside(function (value) { result[name] = value; }); 
}).done(function (resultArr) {
  console.log('All metrics loaded', resultArr, result);
});

edited Oct 13 '13 at 12:49

answered Oct 13 '13 at 10:51

Mariusz Nowak

32,050
5
35
37

I was intrigued to see in the code that all your promise objects are `then` functions. Do you recommend to use one syntax (`.then()` vs. direct invocation) over the other? – Bergi Oct 13 '13 at 11:51
I leave that decision to developer. If you'd like your code to look as if written with common promise implementation use `promise.then()`. If you feel using `promise()` may be confusing to reader of code, and would like to visually distinguish `then` calls, then also use `promise.then()`. I personally rather use `promise()` as it leaves me with even cleaner code and algorithm logic is more transparent. It's also nice feature that allows you to write generic algorithms that can be backed with either sync or async functions. – Mariusz Nowak Oct 13 '13 at 12:55

score 0 · Accepted Answer · edited May 23 '17 at 12:10

metrics[i] = result;

That's a bad idea. You shouldn't destroy the metrics object, it's properties are supposed to be and to stay functions. If you want an object with the results, build a new one.

The return deferred([array of promises])(group promise) idea doesn't seem to work

From the docs and the code you can see that the library does not support arrays as arguments. You will need to use apply:

deferred.apply(null, promiselist)

I've just realised def is getting reused on each iteration so if I had multiple metrics it would probably only track the last one.

No, it's not getting reused, you're creating a new deferred object each loop turn. You did however forget the var keyword to make the variable local.

There also is a problem with the i variable - it is not in the closure scope of each promise callback, which means that when the callbacks are resolved then the variable will already have the value from the last loop turn. Check JavaScript closure inside loops – simple practical example.

A small design flaw is that the metrics methods do take a deferred as an argument which they will resolve. It would be better if they did take no argument, and return a promise for their result which they create (and manage) on their own.

var metrics = {
    queueLength: function() {
        var def = deferred();
            // .. Do some stuff to resolve the queue length ..
            def.resolve(45); // Example
        return def.promise;
    }
    // ... more metrics
};

var results = {},
    promiselist = [];
for (var p in metrics) {
    promiselist.push( metrics[p]().aside( function(_p) {
        return function(result) {
            results[_p] = result;
        };
    }(p) ) );
}
return deferred.apply(null, promiselist).then(function(resultArr) {
    console.log('All metrics loaded', resultArr, results);
    return results;
});

Promise from an array of promises in NodeJS Deferred?

2 Answers2

Linked