Promise inside the loop

Question

In the following code, I have an infinite loop that I do not know why this is happening. My best guess is that inside the async function, the loop does not wait for this, and therefore the loop never stops. What is the best way to solve this problem?

  var generateToken = function(userId) { return new Promise(function(resolve, reject) { User.findOne({userId: userId}, function(err, user) { if (user !== null) { var loop = true; while (loop) { var token = Common.randomGenerator(20); (function(e) { User.find({tokens: e}, function(err, result) { if (err) { loop = false; reject('Error querying the database'); } else { if (result.length === 0) { if (user.tokens === undefined) { user.tokens = []; } user.tokens.push(e); loop = false; resolve(); } } }); })(token); } } else { return reject('UserNotFound'); } }); }); }; 

This function receives userId ( User.findOne() used to search for the user, and if there is no user with this identifier, reject the promise) and creates a unique random token ( randomGenerator ) for this user, add it to that stored in MongoDB and return it to the caller .

( NOTE There were several voices saying that this question is the same as this one . It's not like I already had a closure in my code, and it still doesn't work. This question was more about how to bind a loop variable to close)

Answer 1

You are right that you cannot go in cycles as you try to make.

Javascript is single threaded. Since as long as the main thread is looped in your while(loop) statement, NOTHING else gets a chance to start. Everything would be fine if your main thread changed the loop variable, but this is not quite what is happening. Instead, you are trying to change the loop variable in the async response, but since your main thread is circular, the async response is NEVER processed, so your loop variable can never be changed and therefore an endless loop is unproductive.

To fix this, you will have to switch to a different loop construct. A common design pattern is to create a local function with the code you want to repeat. Then run the async operation, and if in the async result handler you decide you want to repeat the operation, you simply call the local function again from the inside. Since the result is asynchronous, the stack is unwound, and this is not technically recursion with stack accumulation. It just starts another iteration of the function.

I'm a bit confused by the logic in your code (there are zero comments there to explain this), so I'm not entirely sure I got it right, but here's the general idea:

 var generateToken = function(userId) { return new Promise(function(resolve, reject) { User.findOne({userId: userId}, function(err, user) { function find() { var token = Common.randomGenerator(20); User.find({tokens: e}, function(err, result) { if (err) { reject('Error querying the database'); } else { if (result.length === 0) { if (user.tokens === undefined) { user.tokens = []; } user.tokens.push(e); resolve(); } else { // do another find until we don't find a token find(); } } }); } if (user !== null) { find(); } else { reject('UserNotFound'); } }); }); }; 

I should notice that you have incomplete error handling in the User.findOne() operation.

FYI, all the logic of a continuous request, until you get result.length === 0 , just seems weird. The logic seems weird and it smells like “polling a database in a narrow loop”, which is usually a very bad job. I suspect that there are much more effective ways to solve this problem if we understand the general problem at a higher level.

Answer 2

As for learning how to solve problems like this, you can look in the async library ( https://github.com/caolan/async ). It provides fairly intuitive ways to handle asynchronous situations like this in a way that can be understood by most people familiar with synchronous iteration, and the basics of javascript for almost any style of asynchronous iteration you can imagine, and is widely used and very well documented .