Cameron
Cameron

Reputation: 57

res.send() not working for two different callbacks

I am unable to get res.send() to work for all the callbacks, an idea I had was to move the return res.send({ error, success }); to a callback placed above but then it doesn't do the potential error/success messages for below.

I tried doing it a different way where I used a function like createUser() that used a callback to return an error/success message but also wasn't able to get it to work. Is there anything that can point me to how I can make this work properly?

A friend suggested using await and async callbacks, but when searching it I wasn't too familiar to understand how it properly works.

app.post('/create', function(req, res) {

    // -- Default Variables
    let error       = "";
    let success     = "";
    let formData    = req.body.formData;

    if (formData.userName, formData.userPass, formData.userEmail) {
        console.log(formData);
        conn.query("SELECT COUNT(userId) AS rowCount FROM users WHERE userName = ?", [formData.userName], function(error, results, fields) {

            if (results[0].rowCount == 0) {
                conn.query("INSERT INTO users ( userName, userEmail, userPass ) VALUES ( ?, ?, ? )", [ formData.userName, formData.userEmail, formData.userPass ], function(error, results, fields) {

                    if (results.affectedRows >= 1)
                        success = "Your account has successfully been created!";
                    else
                        error = "Unexpected error occured, please try again!";
                });
            } else { error = "You already have an account!"; }
        });
    } else { 
        error = "Please make sure all fields are entered correctly!";
    }
    // -- Return
    return res.send({ error, success });
});

Upvotes: 0

Views: 58

Answers (2)

Algef Almocera
Algef Almocera

Reputation: 819

I can see multiple issues with your code:

  1. It's actually working, BUT it's logically incorrect, returning your empty error and success variables immediately, without waiting for your queries to finish.
  2. You have some conflict with your error variable, consider renaming to something that doesn't overlap with your outside declaration. While it does work, it gives you room for confusion.
  3. Your if has incorrect conditions and logical operators.

Here's a quick fix, but not considering that this can be much much cleaner. Also some quick tips:

  1. Return/Terminate early if possible.
  2. Avoid very long lines to make your code much easier to read
app.post('/create', function(req, res) {

    // -- Default Variables
    let error       = "";
    let success     = "";
    const formData    = req.body.formData;

    const { 
        username,
        userPass,
        userEmail
    } = formData;


    // Avoid too much nested code, terminate/Return early if possible.
    if (!userName || !userPass || !userEmail) {
        error = "Please make sure all fields are entered correctly!";
        return res.send({ error, success });
    }

    conn.query("SELECT COUNT(userId) AS rowCount FROM users WHERE userName = ?", [userName], function(error, results, fields) {

        if (results[0].rowCount == 0) {
            const parameters = [ userName, userEmail, userPass ];
            // Avoid very long lines. It becomes harder to read (TIP: Consider using lint)
            conn.query("INSERT INTO users ( userName, userEmail, userPass ) VALUES ( ?, ?, ? )", parameters , function(error, results, fields) {

                if (results.affectedRows >= 1) {
                    success = "Your account has successfully been created!";
                } else {
                    error = "Unexpected error occurred, please try again!";
                }

                res.send({ error, success }); return;
            });
        } else { 
            error = "You already have an account!"; 
            res.send({ error, success }); return;
        }
    });

});

Upvotes: 1

px1mp
px1mp

Reputation: 5372

conn.query calls an async callback and return res.send({ error, success }); returns immediately. The values changed in the asynchonous callback are in a closure, that executes only after return res.send({ error, success }); already returned the value. So the only error that might show up is the only one in else branch.

Try rewriting it with promises:

app.post('/create', function x(req, res) {
    return new Promise(function(resolve, reject){
        let error       = "";
        let success     = "";
        let formData    = req.body.formData;
        if (formData.userName && formData.userPass && formData.userEmail) {
            console.log(formData);
            conn.query("SELECT COUNT(userId) AS rowCount FROM users WHERE userName = ?", [formData.userName], function(error, results, fields) {
                if (results[0].rowCount == 0) {
                    conn.query("INSERT INTO users ( userName, userEmail, userPass ) VALUES ( ?, ?, ? )", [ formData.userName, formData.userEmail, formData.userPass ], function(error, results, fields) {
                        if (results.affectedRows >= 1)
                            success = "Your account has successfully been created!";
                        else
                            error = "Unexpected error occured, please try again!";
                        resolve({ error, success });
                    });
                } else { 
                    error = "You already have an account!"; 
                    resolve({ error, success }); }
            });
        }
        else{
            error = "Please make sure all fields are entered correctly!";
            resolve({ error, success });
        }
    });
});

Even though I know that always resolving is not a clean way to use Promises. You should consider changing your API return proper errors with appropriate HTTP error codes to client.

Upvotes: 1

Related Questions