NodeJS MySQL query in loop returning before intented

Question

I am trying to create a function that would generate 4 unique id (hexadecimal) to insert into my database. I put the query inside a do while loop to check for collision, if the code is already present, I regenerate the code, if not, I return the value.

The problem is that the loop exits before intended. For example if the code 'a' is generated but already presented in the database, the code gets regenerated but the loop exits and a new query never gets made. The new code does not get returned - instead the first code that was generated is returned.

Here's my code:

const FIND_EXISITNG_COURT =
  "SELECT access_code,team1,team2 FROM courts WHERE access_code= ?";

function generateAccessCode() {

  var random = Math.floor(Math.random() * (+30 - +0)) + +0;
  var code = random.toString(16);

  var hasDupe = false;

  do {
      connection.query(FIND_EXISITNG_COURT, [code], (err, results) => {

          if (err) {
              throw err;
          } else if (results.length > 0) {
              random = Math.floor(Math.random() * (+30 - +0)) + +0;
              code = random.toString(16);

              hasDupe = true;
          } else {
              hasDupe = false;
          }
      });
  } while (hasDupe);

  return code;
}

I am new to NodeJS so I don't know if it's bad practice to do this. Any help would be much appreciated!

You have a sync problem. Your loop will run and not wait for the inner-function call to return. The most easy solution is to make the loop recursive inside the connection.query function — split
– split, Commented Nov 20, 2018 at 5:30

Milad Aghamohammadi · Accepted Answer · 2018-11-20 05:57:32Z

1

In your code, Callback function of query will be run later when the data has been ready, so hasDupe will be false at first time and generated code will be return.

You can use Promise and async function to solve your problem

const FIND_EXISITNG_COURT =
"SELECT access_code,team1,team2 FROM courts WHERE access_code= ?";

function selectByCode(code) {  
    return new Promise((resolve, reject) => {
        connection.query(FIND_EXISITNG_COURT, [code], (err, results) => {
        if (err) {
            reject(err);
        } else {
            resolve(results.length)
        }
    });
}

async function generateAccessCode() {

    var random = Math.floor(Math.random() * (+30 - +0)) + +0;
    var code = random.toString(16);

    var hasDupe = false;

    let count =0;
    try{
        do {

            count = await selectByCode(code);
            if (count > 0) {
                random = Math.floor(Math.random() * (+30 - +0)) + +0;
                code = random.toString(16);

                hasDupe = true;
            } else {
                hasDupe = false;
            }


        } while (hasDupe);

        return code;
    }
    catch(e){
        throw e;

    }
}

answered Nov 20, 2018 at 5:57

Milad Aghamohammadi

1,9961 gold badge21 silver badges31 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Lalit Fauzdar Over a year ago

I was doing such a connection with 1500 requests in seconds, it showed nothing, never entered into the connection block, no error, no log, nothing but it worked for like 10 items. Adding promise did the job.

axanpi · Accepted Answer · 2018-11-20 06:07:27Z

Your call to connection.query is asynchronous, meaning the callback you've defined does not run right away. Your code just registers that callback and then keeps on executing until the end of generateAccessCode. The callback doesn't get called until much later (when the db returns something) so hasDupe gets set only long after the original function has already exited.

You basically have 3 options to handle this: callbacks, promises, or async/await (which is really just syntactic sugar on top of promises).

An example using async/await but trying to keep as close to your original structure as I can (run it more than once to see it working):

// this is a mock for the db call. This "database" already holds records with IDs 0, 2 and 3.
// IRL you will need to wrap your db call in a function that returns a promise
// if you want to do it this way
const connectionQuery = function (code) {
  return new Promise((resolve) => {
    setTimeout(() => resolve(code === 1 ? [] : [true]), 1000);
  });
}

async function generateAccessCode() {
  // simplified the code generation for this example
  let code = Math.floor(Math.random() * 4); // Code can be 0, 1, 2, or 3

  let hasDupe = false;
  let results;
  do {
      results = await connectionQuery(code); // await here is the key
      if (results.length > 0) {
        console.log(`Code ${code} already exists in the database. Generating new code...`);
        code = Math.floor(Math.random() * 4);           
        hasDupe = true;
      } else {
        hasDupe = false;
      }      
  } while (hasDupe);

  return code;
}

generateAccessCode()
  .then((code) => {
    console.log(`Final result: ${code}`);
  })
  .catch((err) => {
    console.log(err);
  });

Using callbacks instead:

// this is a mock for the db call. This "database" already holds records with IDs 0, 2 and 3.
const connectionQuery = function (code, callback) {
  setTimeout(() => {    
    callback(null, code === 1 ? [] : [true]);
  }, 1000);
}

function generateAccessCode(callback) {
  // simplified the code generation for this example
  let code = Math.floor(Math.random() * 4); // Code can be 0, 1, 2, or 3

  let hasDupe = false;
  let results;
  
  connectionQuery(code, (err, results) => {
    if (err) {
      return callback(err);
    }
  
    if (results.length) {
      console.log(`Code ${code} already exists in the DB`);
      return generateAccessCode(callback);
    }
    
    callback(null, code);
  });
}

generateAccessCode((err, code) => {
  console.log(`Final result: ${code}`);
});

Thank you for all the code examples! Definitely learned more about asynchronous functions and how to handle them.

Collectives™ on Stack Overflow

NodeJS MySQL query in loop returning before intented

2 Answers 2

1 Comment

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related