Search code examples
javascriptnode.jspromisebluebird

Async promises inside loop


I know this is a topic where there is a lot of info, but I still can't solve this.

I am working with NodeJs that connects to a mysql server, I have an array, I need to iterate the array and do some stuff with the data, the thing is I need to do a query to the mysql server but I need to the for loop wait until I got the results of the query, I have tried a lot of things, now I am trying with .each method of bluebird but still is not working properly, this is my code.

the initial function is Notification.getByUser

thanks in advance

'use strict';
var Promise = require("bluebird");

module.exports = function(Notifications) {
  Notifications.execute = function(sql, itemId) {
    return new Promise((resolve, reject) => {
      this.dataSource.connector.execute(sql, (e, result) => {
        console.log('-----RESULT----', itemId);
        console.log(result);
        console.log('-----ERROR-----', itemId);
        console.log(e);
        if (result.length === 0) {
          resolve(false);
        } else {
          resolve(true);
        }
      });
    });
  };
  Notifications.isMatching = function(item, user, type) {
    return new Promise((resolve, reject) => {
      console.log(type, item, user);
      if (item !== null) {
        if (item !== user) {
          resolve(false);
        }
      }

      resolve(true);
    });
  };
  Notifications.getByUser = function(userId, isZolver, countryId, cityId, userState, cb) {
    var where = { status: 1 };
    var plainText = '';
    var items = Notifications.find({ where });
    Promise.each(items, item => {
      return Notifications.isMatching(item.isZolver, isZolver, 'isZolver')
        .then(() => {
          Notifications.isMatching(item.cityId, cityId, 'cityId');
        })
          .then(() => {
            if(item.extraCondition !== null && item.extraCondition !== '') {
              var sql = item.extraCondition.replace(/:user_id/g, userId);
              // console.log(sql);
              Notifications.execute(sql, item.id)
                .then(render => console.log('extraCondition', render));
            } else {
              console.log('extraCondition', true);
            }
          });
    }).then(res => {
      // console.log('res del loop', res);
    });
    cb(null, 'ok');
  };
};


Solution

  • There are several issues with your code:

    • In order to chain promises, you must make sure to return promises that you create within a then callback

    • You should not need to create a promise for results that are immediately available (isMatching)

    • The then callback is always executed when a promise fulfils. It does not matter whether you do resolve(false): even if the promised value is false this will still make the promise fulfilled and trigger the then callback(s).

    There are some unknowns in your code, like Notifications.find, and what kind of SQL you are executing: does it return a result set, if so, would you not be interested to get that result instead of just resolving to a boolean?

    Anyway, here are some applied corrections:

    'use strict';
    var Promise = require("bluebird");
    
    module.exports = function(Notifications) {
        Notifications.execute = function(sql, itemId) {
            return new Promise((resolve, reject) => {
                this.dataSource.connector.execute(sql, (e, result) => {
                    console.log('-----RESULT----', itemId);
                    console.log(result);
                    console.log('-----ERROR-----', itemId);
                    console.log(e);
                    resolve (result.length !== 0); // The `if ... else` is overkill
                });
            });
        };
    
        //You don't need isMatching to return a promise
        Notifications.isMatching = function(a, b) {
            return a === null || a === b;
        };
    
        Notifications.getByUser = function(userId, isZolver, countryId, cityId, userState) {
            var where = { status: 1 };
            var items = Notifications.find({ where })
                // use filter() to limit the items to those you want to execute the SQL for
                .filter(item => {
                    return Notifications.isMatching(item.isZolver, isZolver)
                        && Notifications.isMatching(item.cityId, cityId)
                        && item.extraCondition !== ''
                });
            // Return the promise! (don't go back to the old callback system!)
            return Promise.each(items, item => {
                var sql = item.extraCondition.replace(/:user_id/g, userId);
                // Return the promise!
                return Notifications.execute(sql, item.id);
            }).then(res => {
                console.log('res del loop', res);
            });
        };
    };
    

    Note that the signature of Notifications.getByUser does not have the final callback parameter: you should keep using promises once you start with them, so when you call this function, call the result's then method like you would do for any promise:

     Notifications.getByUser( .......arguments....).then( function () {
         // do something...
     });