1

Evening stack! Tonight I decided to play around with some NodeJS and I'm having a little trouble understanding the appropriate way for me to handle errors in this situation.

I have a table that simply stores playerName and the name must be unique. So rather than just try to insert and get an error, I want to first make a select and if I get a result, return a 400 to the user and let them know the name already exists. If not, continue on as normal, insert the name, and return a 203.

What I've got clearly isn't working. I've attempted a try/catch and that didn't work. And I clearly can't return an error with the methods I'm using. So what's a good way to go about this?

router.post('/addPlayer' , function(req, res, next){
  var playerName = req.body.name;
  if(playerName === undefined || playerName.length === 0)
  {
    return res.status(400).send('No name provided');
  }

  var query = 'SELECT name FROM players WHERE name LIKE ?';
  var inserts = [playerName];
  query = connection.format(query , inserts);
  connection.query(query, function(err, results){
    if(err) return res.status(500).send('Error connecting to database.');

    if(results.length !== 0) return res.status(400).send('This name has already been used.');
  });

  query = 'INSERT INTO players (name) VALUES(?)';
  inserts = [playerName];
  query = connection.format(query , inserts);
  connection.query(query, function(err){
    if(err) return res.status(500).send('Error connecting to database.');
  });

  res.status(201).send("Added player: " + playerName);
});

In this current version my obvious problem is Node crashes complaining about not being able to set the headers after they've already been sent. I know what I need to do. Which is end the execution of the route and return the error to the browser, but I'm just not clear on how to best go about that.

I'm using the Express framework and mysql.

Thanks.

Squeegy
  • 869
  • 9
  • 20
  • As a side note, you should probably still handle an error from the database because someone could insert an identical key after you've checked for uniqueness but before you insert. – Millie Smith May 15 '15 at 01:14
  • @MillieSmith Yeah, I plan on doing that too. – Squeegy May 15 '15 at 01:16
  • @MillieSmith Yeah, I've tried that one already. It doesn't end the execution of the rest of the code. – Squeegy May 15 '15 at 01:23
  • You might have to just chain your callbacks. Are the query calls async? – Millie Smith May 15 '15 at 01:32
  • possible duplicate of [Why is my variable unaltered after I modify it inside of a function? - Asynchronous code reference](http://stackoverflow.com/questions/23667086/why-is-my-variable-unaltered-after-i-modify-it-inside-of-a-function-asynchron) – Aaron Dufour May 15 '15 at 01:32
  • @AaronDufour I think that might be. I always forget javascript works like that. Let me give this a shot. – Squeegy May 15 '15 at 01:36

2 Answers2

1

The problem is you're running both queries in parallel. So the INSERT is executed before the response of the SELECT is received. Which means that there is a race condition. Both queries tries to send res.status() but one will happen after the other which causes the error.

To fix this, wait until the SELECT is received then do your INSERT:

var query = 'SELECT name FROM players WHERE name LIKE ?';
var inserts = [playerName];
query = connection.format(query , inserts);
connection.query(query, function(err, results){
    if(err) return res.status(500).send('Error connecting to database.');

    if(results.length !== 0) return res.status(400).send('This name has already been used.');

    query = 'INSERT INTO players (name) VALUES(?)';
    inserts = [playerName];
    query = connection.format(query , inserts);
    connection.query(query, function(err){
        if(err) return res.status(500).send('Error connecting to database.');

        res.status(201).send("Added player: " + playerName);
    });
});
slebetman
  • 109,858
  • 19
  • 140
  • 171
  • Yup, Aaron pointed this same thing out in the comments above. It makes sense too now. Thanks! I'll give this a shot. – Squeegy May 15 '15 at 01:37
0

Is there anything wrong with sending the error(err) itself? For example:

if (err)
    res.send(err);
Bidhan
  • 10,607
  • 3
  • 39
  • 50