Node.js / Sails.js - Function in forEach loop

Question

I'm trying to get the following simplified code to finish and then move on synchronously:

factors.forEach(function(factor, index){
  Factor.findOrCreate({function_name: factor}).exec(function(){
    sails.log('success');
  })
});

How do I do it?

So far I've tried:

Putting lines 2-4 in a separate function and calling that from inside the loop
Using the async.js async.series() call

But while it will execute the calls (print 'success'), it'll find or create a Factor in the DB very infrequently, so it has something to do with the asynchronous nature of the code.

Using promises to wait until all findOrCreate() calls return, then continuing

var promises = [];
factors.forEach(function(factor, index){
  promises.push(Factor.findOrCreate({function_name: factor}));
});
Q.all(promises).then(function(){
  sails.log('success');
});

I've tried a few versions of this one, but I still can't get it to print 'success' - This seems like the right way to do it, but I'm not quite sure what's wrong.

Asking the Rubber Ducky

Do you think the "infrequent working" is related to the fact that the queries are processed in parallel, instead of sequentially? — Bergi, Jul 22 '15 at 00:16
That is my assumption, which is why I thought to try async.series() and promises. All tests I've done seem to enforce that notion. — smileham, Jul 22 '15 at 00:59

score 2 · Answer 1 · answered Jul 22 '15 at 00:12

2

There is a simple way of doing this using the async.js library forEachOf method.

Ex :

var async = require('async');
var promises = [];
async.forEachOf(factors,function(factor, key, callback){
  Factor.findOrCreate({function_name: factor}).exec(function(result,err){
    if (err) {
      callback(err);
    } else {
      promises.push(result);
      callback();
    }
  });
}, function(err){
  if (err) {
    console.log("Error: "+err);
  } else {
    console.log(promises);
  }
});

answered Jul 22 '15 at 00:12

John

338
2
7

I ran this five times with the following results: 2/5: Doesn't print anything or alter the database. 3/5: Error (E_UNKNOWN) :: Encountered an unexpected error : Could not connect to MySQL: Error: Pool is closed. at afterwards (.../node_modules/sails-mysql/lib/connections/spawn.js:72:13) at .../node_modules/sails-mysql/lib/connections/spawn.js:40:7 at process._tickDomainCallback (node.js:463:13) Details: Error: Could not connect to MySQL: Error: Pool is closed. – smileham Jul 22 '15 at 01:00

galactocalypse · Accepted Answer · 2015-07-22T01:24:25.893

2

A simple method of doing this without using async or promises would be to manually keep track of how many records have been processed:

var ctr = 0;
factors.forEach(function(factor, index){
    Factor.findOrCreate({function_name: factor}).exec(function(err, factorObj){
        if (err) errorHandler(err);
        ctr ++;
        if (ctr === factors.length) {
            successCallback();
        }
    });
});

successCallback() will be called only after all Factor objects have been created.

Update

Alternatively, you could try this to avoid looping altogether:

Factor.findOrCreate(factors).exec(function(err, factorObjArray){
    callback(err, factorObjArray);
});

edited Jul 22 '15 at 01:24

answered Jul 22 '15 at 00:57

galactocalypse

1,905
1
14
29

This also seems correct, but when I ran it, I got the same results as the other answer: 2/5: Doesn't print anything or alter the database. 3/5: Error (E_UNKNOWN) :: Encountered an unexpected error : Could not connect to MySQL: Error: Pool is closed. at afterwards (.../node_modules/sails-mysql/lib/connections/spawn.js:72:13) at .../node_modules/sails-mysql/lib/connections/spawn.js:40:7 at process._tickDomainCallback (node.js:463:13) Details: Error: Could not connect to MySQL: Error: Pool is closed. – smileham Jul 22 '15 at 01:09
The errors you mentioned point to a database connection error. Try restarting your database or closing all open connections. – galactocalypse Jul 22 '15 at 01:12
It doesn't have to do with Sails. Either your pool size is too small or some other application is hogging the connections. Did you try again after restarting your database? – galactocalypse Jul 22 '15 at 01:13
I did both but it's not behaving any differently :/ – smileham Jul 22 '15 at 01:14
Are the other parts of your code dealing with the database working fine? – galactocalypse Jul 22 '15 at 01:15
Yeah, I just placed a regular DB insertion `Factor.findOrCreate({function_name: 'test'}).exec(function(){sails.log('success')});` and it worked. However, it only printed 'success' some of the time. – smileham Jul 22 '15 at 01:17
Interesting. I've made an update to the answer. See if that works or not. It's really weird that you're getting database errors randomly. – galactocalypse Jul 22 '15 at 01:25
After that last clue, I figured out that the test I was running concluded, lowering sails and closing the DB connection before the callback could be processed; thus the DB error. I put in a delay before closing and each code snippet (including my original) worked fine. So I just need to structure my code such that it does not conclude before the DB functions return. Does that seem correct? – smileham Jul 22 '15 at 01:25
That sounds reasonable. As long as the connection is open, you should be good to go. However, even if it is working, your code can possibly execute the callback before all objects are created. You might want to resort to one of the answers mentioned here for some consistency. – galactocalypse Jul 22 '15 at 01:32

score 1 · Answer 3 · answered Jul 22 '15 at 01:37

You need to use something like Q, promises. But in a proper asyncronous way.

var cb = function(factor) {
  var deferred = Q.defer();
  Factor.findOrCreate({function_name: factor})
    .exec(function(err, factorObj) {
      if(err) deferred.reject(err);
      deferred.resolve(factorObj);
    });
  return deferred.promise;
};

var promises = [];
Factor.forEach(function(f) {
  promises.push(cb(f));
});

Q.all(promises)
  .then(function() { // You can get the params too.
    console.log('WE ARE THE CHAMPIONS!!');
  })
  .fail(function() { // You can get the error params too.
    console.log('WE LOOSE');
  });

Hope it helps! Its only an example, Its been a few months since I stop using NodeJS and SailsJS (unfortunately).

Notice that sails does support promises, and you should avoid the [deferred antipattern](http://stackoverflow.com/q/23803743/1048572) then. — Bergi, Jul 22 '15 at 01:47

Node.js / Sails.js - Function in forEach loop

3 Answers3

Linked