NodeJS less visible callbacks and more structure

Question

One of the advantages of NodeJS is its async and non-blocking I/O, which in my case is great on the one hand, but breaks my neck every day on the other hand.

I consider myself a NodeJS / Async novice and I often end up having such code:

function(req, res) {
            req.assert("name", "Lobbyname is required").notEmpty();
            req.assert("name", "Lobbyname length should be between 4 and 64 characters").len(4, 64);
            req.assert("game", "Game not found").isInt();

            req.sanitize("game").toInt();
            var userId = req.user.id;

            var errors = req.validationErrors();
            var pg_errors = [];
            var games = null;
            if (errors) {
                console.log(errors);
                client.query("SELECT * FROM games", function(err, result) {
                    if (!err) {
                        games = result.rows;
                        res.render("lobby/create", {
                            title: "Create a new lobby",
                            games: games,
                            errors: errors.toString()
                        });
                    }
                    else {
                        res.send("error");
                    }
                });
            }
            else {
                errors = null;
                client.query("SELECT COUNT(*) as in_lobbies FROM users u RIGHT JOIN lobby_userlist ul ON ul.user_id = u.id WHERE u.id = $1", [userId], function(err, result) {
                    if (!err) {
                        console.log(result.rows[0]);
                        if (result.rows[0].in_lobbies < 1) {
                            client.query("SELECT COUNT(*) as hosting_lobbies FROM lobbies WHERE owner = $1", [userId], function(err, result) {
                                if (!err) {
                                    if (result.rows[0].hosting_lobbies < 1) {
                                        client.query("INSERT INTO lobbies(name, game, owner) VALUES($1, $2, $3)", [req.param("name"), req.param("game"), userId], function(err, result) {
                                            if (!err) {
                                                res.redirect("/lobby");
                                            }
                                            else {
                                                pg_errors.push(err);
                                                console.log(err);
                                            }
                                        });
                                    }
                                    else {
                                        errors = "You can only host one lobby at a time";
                                    }
                                }
                                else {
                                    pg_errors.push(err);
                                    client.query("SELECT * FROM games", function(err, result) {
                                        if (!err) {
                                            games = result.rows;

                                            res.render("lobby/create", {
                                                title: "Create a new lobby",
                                                games: games,
                                                errors: errors
                                            });
                                        }
                                        else {
                                            pg_errors.push(err);
                                        }
                                    });
                                }
                            });
                        }
                        else {
                            pg_errors.push(err);
                        }
                    }
                });

                console.log("pg_errors");
                console.log(pg_errors);
                console.log("pg_errors _end");

                if (pg_errors.length < 1) {
                    console.log("no errors");
                }
                else {
                    console.log(pg_errors);
                    res.send("error service operation failed");
                }
            }
        }

This an example I have written using the following npm packages:

• pg (native)
• express
• express-validator (middleware of node-validator)
• passport (auth middleware)

Checking whether the input given by the user is valid or not is the least problem, I have this checks where I assert the variables and give back a rendered version of the page printing out the errors to the user.

BUT if we pass the validation errors in the first place we assume the "lobby" is ready to be inserted into the database, before I want to ensure that the user has no other lobby open and is not member of another lobby. Well now I end up putting one query into another and theoretically I would have to put my view render function (res.render()) into every query callback if the query encounters an error or returns a result which inidicates that the user is not allowed to create a lobby. I don't want that and it doesn't seem very practicable.

I tried removing the render logic and every other logic from the query callbacks and instead let the query callbacks set error arrays or variables which would indicate a success or a failure and below my query code I would check if(errors) renderPageWithErrors.

This lead to strange errors due to the async behaviour of nodejs in which case res.redirect() was called after res.render() and stuff like that. I had to move my res.render back into the query callbacks.

Is there a proper way of doing this?

Answer 1

You might want to check for https://github.com/0ctave/node-sync library as well. It's a syntax sugar for nodejs Fibers, a way to write asynchronous code in a traditional way without breaking nodejs event loop model. There are a lot of discussions about pros and cons of using Fibers, but I prefer code readability and ease of development over potential small resource usage increase.

I don't know all of your code logic, but function above can look something like this:

function(req, res) {
    Sync(function() {
        req.assert("name", "Lobbyname is required").notEmpty();
        req.assert("name", "Lobbyname length should be between 4 and 64 characters").len(4, 64);
        req.assert("game", "Game not found").isInt();

        req.sanitize("game").toInt();
        var userId = req.user.id;

        var errors = req.validationErrors();
        var pg_errors = [];
        var games = null;
        if (errors) {
            console.log(errors);
            var games = client.query.sync(client, "SELECT * FROM games").rows;
            games = result;
            res.render("lobby/create", {
                title: "Create a new lobby",
                games: games,
                errors: errors.toString()
            });
        }
        else {
            errors = null;
            var result = client.query.sync(client, "SELECT COUNT(*) as in_lobbies FROM users u RIGHT JOIN lobby_userlist ul ON ul.user_id = u.id WHERE u.id = $1", [userId]);

            console.log(result.rows[0]);
            if (result.rows[0].in_lobbies < 1) {
                var result = client.query.sync(client, "SELECT COUNT(*) as hosting_lobbies FROM lobbies WHERE owner = $1", [userId]);

                if (result.rows[0].hosting_lobbies < 1) {
                    var res = client.query.sync(clien, "INSERT INTO lobbies(name, game, owner) VALUES($1, $2, $3)", [req.param("name"), req.param("game"), userId]);
                    res.redirect("/lobby");
                }
                else {
                    errors = "You can only host one lobby at a time";
                }
            }
            else {
                var games = client.query.sync(client, "SELECT * FROM games").rows;

                res.render("lobby/create", {
                    title: "Create a new lobby",
                    games: games,
                    errors: errors
                });
            };
        }
    }, function(err) {
        if(err) {
            // do your error handling here
        }
    });
}

Answer 2

You might want to look into an async library such as https://github.com/caolan/async. It helps structure async code so that it doesn't turn into a mess like this. There are different methods depending on your requirements from simple series and parallel execution to things like waterfall to auto which does dependency tracking.

async.auto({
    get_data: function(callback){
        // async code to get some data
    },
    make_folder: function(callback){
        // async code to create a directory to store a file in
        // this is run at the same time as getting the data
    },
    write_file: ['get_data', 'make_folder', function(callback){
        // once there is some data and the directory exists,
        // write the data to a file in the directory
        callback(null, filename);
    }],
    email_link: ['write_file', function(callback, results){
        // once the file is written let's email a link to it...
        // results.write_file contains the filename returned by write_file.
    }]
}, function(err) { 
    // everything is done or an error occurred 
});

The other nice thing it does is consolidate all errors into a single callback. That way you only have to handle errors in one place instead of them sprinkled throughout your code.