javascript - How can I wait for asyncronous loop in node.js

Question

I am making a private messaging system with Node.js and socket.io. I need to send the latest messages between 2 users. Separately both get_messages_between_users function and get_message_user functions works fine. But If I use them like below, messages array is staying empty when socket emits the data. It is normal actually due to asynchronous structure of Javascript but how can I solve this problem? Here is the code.

function get_message_user(message, callback) {
    var sql = "SELECT * from users WHERE id = " + message.from_user
    var query = dbh.query(sql, function(err, result) {
        callback(message, result[0]);
    });
};

function get_messages_between_users(user1, user2, callback) {
    var sql = "SELECT id, from_user, to_user, content, msg_date, status FROM messages WHERE ((to_user = "+user1+" AND from_user = "+user2+") OR (to_user = "+user2+" AND from_user = "+user1+")) ORDER BY id ASC LIMIT 20;"
    var query = dbh.query(sql, function(err, result) {
        callback(result);
    });
};

io.on('connection',function (socket) {
    //Other stuff for handling clients
    socket.on("page messages", function (data) {
        var messages = [];
        get_messages_between_users(socket.session.user_id, data.to_user, function(result){
            for(var i = 0; i < result.length; i++){
                get_message_user(result[i], function(message, user){
                    var obj = {id: message.id, user_id: user.id, username: user.username, avatar:user.avatar_url, content: message.content, msg_date:message.msg_date};
                    messages.push(obj);
                });
            }
        });

        //The problem is messages array is empty here.
        socket.emit('message history', JSON.stringify(messages));
    });
});

I am using message parameter in get_message_user just for bypassing variable to result.

FYI, you're wide open to SQL injection attacks and related flaws. Never concatenate arbitrary data into a query. Bind parameters instead! — Brad
– Brad, Commented Dec 16, 2019 at 2:24
I am aware of that. I just wanted to make everything work then I will make necessary improvements. — kenarsuleyman
– kenarsuleyman, Commented Dec 16, 2019 at 2:26
That's really unfortunate, that rather than building this correctly, you're actually taking extra time to make a less reliable and more insecure application. A for...of loop is what you want, with await. — Brad
– Brad, Commented Dec 16, 2019 at 2:27
I don't know what DB library you're using, but normally you'd do something like const query = db.bind('SELECT * FROM whatever WHERE field=?;'). And then, something like query.execute(['value for the field']);. As for the async loop: developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/… — Brad
– Brad, Commented Dec 16, 2019 at 2:33
Also, take care of validating/escaping the users' input before placing them as data to your DB queries, as correctly @Brad has pointed out. Do not start by overlooking these issues, as it will become a (bad) habit. — Kostas Minaidis
– Kostas Minaidis, Commented Dec 16, 2019 at 2:50

Kostas Minaidis · Accepted Answer · 2019-12-16 11:08:19Z

You need to do a couple of things in order to refactor your code to handle all these asynchronous operations which depend on each other:

1) Convert the get_message_user and get_messages_between_users functions into Promise-based functions.

(Note: as suggested in the comments below, the optimal solution would be to use the promise-based functions supported by the current database library or switch to one that uses Promises for these operations. Alternatively a utility can be used to easily turn the callback functions into Promise-based functions. One such utility is Promisify

2) Convert the socket.on("page message") callback into an async function

3) Inside the socket callback, call the get_messages_between_users function and await for the async result

4) Once the result list is returned, loop over it and create an array of calls to the get_message_user async function and await for the result of all these async requests using the Promise.all() function.

5) Once the results are returned, map (or loop) over them, and add the data to the messages array and emit the data via the socket.emit function

function get_message_user(message) {
    // Return a Promise that will be handled via a then() method
    return new Promise((resolve,reject)=>{
        let sql = "SELECT * from users WHERE id = " + message.from_user;
        dbh.query(sql, function(err, result) {
            if ( err ){ reject(err); } // Handle Error via a catch() method
            resolve([message,result[0]]);
        });
    })
};

function get_messages_between_users(user1, user2) {
    return new Promise((resolve,reject)=>{
        let sql = "SELECT id, from_user, to_user, content, msg_date, status FROM messages WHERE ((to_user = "+user1+" AND from_user = "+user2+") OR (to_user = "+user2+" AND from_user = "+user1+")) ORDER BY id ASC LIMIT 20;"
        dbh.query(sql, function(err, result) {
            if ( err ){ reject(err); }
            resolve(result);
        });
    })
};

io.on('connection',function (socket) {

    // We need an async function here, in order to use await on async operations
    socket.on("page messages", async function (data) {

        let messages = [];
        // Wait for the asynchronous operation to give back a result using await
        const result = await get_messages_between_users(socket.session.user_id, data.to_user);
        const requests = []; // This will hold a list of async requests for each result
        for ( let i = 0; i < result.length; i++){
            requests.push( get_message_user(result[i]) );
        }
        const results = await Promise.all(requests);
        results.map((result)=>{
            let message = result[0];
            let user = result[1];
            let obj = {
              id: message.id,
              user_id: user.id,
              username: user.username,
              avatar:user.avatar_url,
              content: message.content,
              msg_date:message.msg_date
            };
            messages.push(obj);
        });

        socket.emit('message history', JSON.stringify(messages));

    });

});

References:

Note: no need to pay for a Udemy course. You can check these excellent courses on asynchronous programming in Udacity:

Warning: As correctly pointed out by the comments above, the code that handles the SQL queries posted in the question needs some heavy refactoring in order for the code to be secure. The code in this answer is provided only to demonstrate how one might handle multiple asynchronous requests. The rest of the code remains insecure and must be refactored.

FYI, it would be best for the OP to just USE the promise interface built into their database rather than create their own. If this is mysql, then that can be found in msyql2. Lots and lots and lots of good reasons to be moving entirely to promises for asynchronous work anyway.
You are right @jfriend00. I was thinking about that, but since I didn't know which library was used I decided to post the example above. I will update the answer with your suggestion. Thanks.

Collectives™ on Stack Overflow

javascript - How can I wait for asyncronous loop in node.js

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related