Tomasz Golinski
Tomasz Golinski

Reputation: 728

Promise all not firing

I'm trying to get promises work for the first time. However, my Promise.all is never firing.

I'm using node.js & express

const promises = [
    new Promise( () => {
        var query = `...`;  

        mssql.query(query, function(obj){
            finalRes['key1'] = obj.recordset;
            return true;
            //this works
        });
    }),
    new Promise( () => {
        var query = `...`;  

        mssql.query(query, function(obj){
            finalRes['key2'] = obj.recordset;
            return true;
            //this works
        });
    }),
    ...
]

Promise.all(promises).then(() => {
    res.send(finalRes);
    // this is never firing
});

I have been googling stuff and I can't find a solution. I would appreciate someone point out what I do wrong here.

Cheers

Upvotes: 0

Views: 267

Answers (1)

raina77ow
raina77ow

Reputation: 106375

Your promise creation code is wrong, as they never resolve. What you actually should do is fire resolve functions inside your callback-based code. I'd go a bit further - and make all those promises resolve with their results instead of modifying some external value. Like this:

const promises = [
  new Promise( (resolve, reject) => {
    var query = `...`;  
    mssql.query(query, function(obj){
      resolve({key1:obj.recordset});
    });
  }),
  new Promise( (resolve, reject) => {
    var query = `...`;  
    mssql.query(query, function(obj){
      resolve({key2:obj.recordset});
    });
  }) // ...
];

Promise.all(promises).then(results => {
  res.send(Object.assign({}, ...results));
});

Depending on how queries are built, you might go even further - and write a generic query-generator function, which takes query and key as params, and returns a promise. Again, this function should be easy to test.

As a sidenote, your code is overly optimistic, it should also provide error callbacks with reject() invoke to each query.

Upvotes: 1

Related Questions