I have this chunk of code
User.find({}, function(err, users) {
for (var i = 0; i < users.length; i++) {
pseudocode
Friend.find({
'user': curUser._id
}, function(err, friends) * * ANOTHER CALLBACK * * {
for (var i = 0; i < friends.length; i++) {
pseudocode
}
console.log("HERE I'm CHECKING " + curUser);
if (curUser.websiteaccount != "None") {
request.post({
url: 'blah',
formData: blah
}, function(err, httpResponse, body) { * * ANOTHER CALLBACK * *
pseudocode
sendMail(friendResults, curUser);
});
} else {
pseudocode
sendMail(friendResults, curUser);
}
});
console.log("finished friend");
console.log(friendResults);
sleep.sleep(15);
console.log("finished waiting");
console.log(friendResults);
}
});
There's a couple asynchronous things happening here. For each user, I want to find their relevant friends and concat them to a variable. I then want to check if that user has a website account, and if so, make a post request and grab some information there. Only thing is, that everything is happening out of order since the code isn't waiting for the callbacks to finish. I've been using a sleep but that doesn't solve the problem either since it's still jumbled.
I've looked into async, but these functions are intertwined and not really separate, so I wasn't sure how it'd work with async either.
Any suggestions to get this code to run sequentially?
Thanks!
I have this chunk of code
User.find({}, function(err, users) {
for (var i = 0; i < users.length; i++) {
pseudocode
Friend.find({
'user': curUser._id
}, function(err, friends) * * ANOTHER CALLBACK * * {
for (var i = 0; i < friends.length; i++) {
pseudocode
}
console.log("HERE I'm CHECKING " + curUser);
if (curUser.websiteaccount != "None") {
request.post({
url: 'blah',
formData: blah
}, function(err, httpResponse, body) { * * ANOTHER CALLBACK * *
pseudocode
sendMail(friendResults, curUser);
});
} else {
pseudocode
sendMail(friendResults, curUser);
}
});
console.log("finished friend");
console.log(friendResults);
sleep.sleep(15);
console.log("finished waiting");
console.log(friendResults);
}
});
There's a couple asynchronous things happening here. For each user, I want to find their relevant friends and concat them to a variable. I then want to check if that user has a website account, and if so, make a post request and grab some information there. Only thing is, that everything is happening out of order since the code isn't waiting for the callbacks to finish. I've been using a sleep but that doesn't solve the problem either since it's still jumbled.
I've looked into async, but these functions are intertwined and not really separate, so I wasn't sure how it'd work with async either.
Any suggestions to get this code to run sequentially?
Thanks!
Share Improve this question edited Dec 31, 2014 at 17:34 Nick Tomlin 29.2k12 gold badges63 silver badges94 bronze badges asked Dec 31, 2014 at 17:27 user3340037user3340037 7312 gold badges8 silver badges23 bronze badges 2- Here is a similar question that may help: stackoverflow.com/questions/9644197/… – tier1 Commented Dec 31, 2014 at 17:30
- 1 The point of callbacks is that the code doesn't wait. If the code waited, there'd be no need for the callbacks. While you could use a "Promise" solution, it's better to first understand the fundamental problem and how to manage it without abstractions. Then use abstractions if desired. – user1106925 Commented Dec 31, 2014 at 17:38
6 Answers
Reset to default 6I prefer the promise module to q https://www.npmjs.com/package/promise because of its simplicity
var Promises = require('promise');
var promise = new Promises(function (resolve, reject) {
// do some async stuff
if (success) {
resolve(data);
} else {
reject(reason);
}
});
promise.then(function (data) {
// function called when first promise returned
return new Promises(function (resolve, reject) {
// second async stuff
if (success) {
resolve(data);
} else {
reject(reason);
}
});
}, function (reason) {
// error handler
}).then(function (data) {
// second success handler
}, function (reason) {
// second error handler
}).then(function (data) {
// third success handler
}, function (reason) {
// third error handler
});
As you can see, you can continue like this forever. You can also return simple values instead of promises from the async handlers and then these will simply be passed to the then
callback.
I rewrote your code so it was a bit easier to read. You have a few choices of what to do if you want to guarantee synchronous execution:
Use the async library. It provides some helper functions that run your code in series, particularly, this: https://github.com/caolan/async#seriestasks-callback
Use promises to avoid making callbacks, and simplify your code APIs. Promises are a new feature in Javascript, although, in my opinion, you might not want to do this right now. There is still poor library support for promises, and it's not possible to use them with a lot of popular libraries :(
Now -- in regards to your program -- there's actually nothing wrong with your code at all right now (assuming you don't have async code in the pseucode blocks). Your code right now will work just fine, and will execute as expected.
I'd recommend using async for your sequential needs at the moment, as it works both server and client side, is essentially guaranteed to work with all popular libraries, and is well used / tested.
Cleaned up code below
User.find({}, function(err, users) {
for (var i = 0; i < users.length; i++) {
Friend.find({'user':curUser._id}, function(err, friends) {
for (var i = 0; i < friends.length; i++) {
// pseudocode
}
console.log("HERE I'm CHECKING " + curUser);
if (curUser.websiteaccount != "None") {
request.post({ url: 'blah', formData: 'blah' }, function(err, httpResponse, body) {
// pseudocode
sendMail(friendResults, curUser);
});
} else {
// pseudocode
sendMail(friendResults, curUser);
}
});
console.log("finished friend");
console.log(friendResults);
sleep.sleep(15);
console.log("finished waiting");
console.log(friendResults);
}
});
First lets go a bit more functional
var users = User.find({});
users.forEach(function (user) {
var friends = Friend.find({
user: user._id
});
friends.forEach(function (friend) {
if (user.websiteaccount !== 'None') {
post(friend, user);
}
sendMail(friend, user);
});
});
Then lets async that
async.waterfall([
async.apply(Users.find, {}),
function (users, cb) {
async.each(users, function (user, cb) {
async.waterfall([
async.apply(Friends.find, { user, user.id}),
function (friends, cb) {
if (user.websiteAccount !== 'None') {
post(friend, user, function (err, data) {
if (err) {
cb(err);
} else {
sendMail(friend, user, cb);
}
});
} else {
sendMail(friend, user, cb);
}
}
], cb);
});
}
], function (err) {
if (err) {
// all the errors in one spot
throw err;
}
console.log('all done');
});
Also, this is you doing a join, SQL is really good at those.
You'll want to look into something called promises. They'll allow you to chain events and run them in order. Here's a nice tutorial on what they are and how to use them http://strongloop.com/strongblog/promises-in-node-js-with-q-an-alternative-to-callbacks/
You can also take a look at the Async JavaScript library: Async It provides utility functions for ordering the execution of asynchronous functions in JavaScript.
Note: I think the number of queries you are doing within a handler is a code smell. This problem is probably better solved at the query level. That said, let's proceed!
It's hard to know exactly what you want, because your psuedocode could use a cleanup IMHO, but I'm going to what you want to do is this:
- Get all users, and for each user
a. get all the user's friends and for each friend:
- send a post request if the user has a website account
- send an email
- Do something after the process has finished
You can do this many different ways. Vanilla callbacks or async work great; I'm going to advocate for promises because they are the future, and library support is quite good. I'll use rsvp, because it is light, but any Promise/A+ compliant library will do the trick.
// helpers to simulate async calls
var User = {}, Friend = {}, request = {};
var asyncTask = User.find = Friend.find = request.post = function (cb) {
setTimeout(function () {
var result = [1, 2, 3];
cb(null, result);
}, 10);
};
User.find(function (err, usersResults) {
// we reduce over the results, creating a "chain" of promises
// that we can .then off of
var userTask = usersResults.reduce(function (outerChain, outerResult) {
return outerChain.then(function (outerValue) {
// since we do not care about the return value or order
// of the asynchronous calls here, we just nest them
// and resolve our promise when they are done
return new RSVP.Promise(function (resolveFriend, reject){
Friend.find(function (err, friendResults) {
friendResults.forEach(function (result) {
request.post(function(err, finalResult) {
resolveFriend(outerValue + '\n finished user' + outerResult);
}, true);
});
});
});
});
}, RSVP.Promise.resolve(''));
// handle success
userTask.then(function (res) {
document.body.textContent = res;
});
// handle errors
userTask.catch(function (err) {
console.log(error);
});
});
jsbin