0

I've spent a couple of hours trying to get the below Async request working correctly, and after learning a whole bunch about Async requests, mongoose, and node.js, it turns out my issue was more or less a syntax problem (as I can understand it).

Can anyone tell me why this code works, but my original code doesn't?

Working Code:

router.get('/usercheck', function(req, res) {
  var usr = Person.findOne({username: req.query.u}, function(err, user){
    if(user != null){
      return user
    }else return null
  })
   var eml = Person.findOne({email: req.query.e}, function(err, user){
     if(user != null) {
       return user
     }else return null
   });
   var resolve = function(e, u){
     if(e === null && u === null) res.sendStatus(200) //Both available
     else if (e === null && u != null) res.sendStatus(409) //"Email free, user taken"
     else if (e != null && u === null) res.sendStatus(403) //"Email taken, user free"
     else if (e!=null && u!=null) res.sendStatus(418) //"Email and user taken"
     else res.sendStatus(500) // Something broke, no idea what
   }
   async function handler() {
       const user = await usr;
       const email = await eml;
       return resolve(email, user);
   }
   handler();
 });

Original Code:

router.get('/usercheck', function(req, res) {
  function usr() {
    Person.findOne({username: req.query.u}, function(err, user){
      if(user != null){
        return user
      }else return null
    })
  }
  function eml(){
    Person.findOne({email: req.query.e}, function(err, user){
      if(user != null) {
        return user
      }else return null
    });
  }
  var resolve = function(e, u){
    if(e === null && u === null) res.sendStatus(200) //Both available
    else if (e === null && u != null) res.sendStatus(409) //"Email free, user taken"
    else if (e != null && u === null) res.sendStatus(403) //"Email taken, user free"
    else if (e!=null && u!=null) res.sendStatus(418) //"Email and user taken"
    else res.sendStatus(500) // Something broke, no idea what
  }
  async function handler() {
      const user = await usr();
      const email = await eml();
      return resolve(email, user);
  }
  handler();
});

I'm sure there is a logical reason for this, but I don't understand it atm. In so far as my understanding goes, the return statement in either version of the code should get passed to resolve(), which should evaluate it.

However, with the second, original implementation, nothing gets passed to resolve() at all, the if else loop fails, and fall back error 500 is sent; Can anyone help me understand why?

3 Answers3

1

First you either stick to Promises or callbacks.

var usr = Person.findOne({username: req.query.u}).exec() // do not need a nodeback

Then you functions return undefined. Should return a promise to interact w/ await correctly

function usr() {
  // return!
  return Person.findOne({username: req.query.u}).exec()
}
Yury Tarabanko
  • 39,619
  • 8
  • 73
  • 90
  • Thank you! Noticing that they were returning undefined was what made me realise how to fix it! Can you clarify why the return statement in the cb I was using, wasn't being returned? – user1429671 Dec 30 '18 at 16:30
  • Should the "return user" not have been returned? – user1429671 Dec 30 '18 at 16:30
  • Returning in callback usually makes no sense. Check this question https://stackoverflow.com/questions/14220321/how-do-i-return-the-response-from-an-asynchronous-call – Yury Tarabanko Dec 30 '18 at 16:42
0

So many issues...for starters, async functions wrap returned values as promises so what you are actually returning in handler() is an unresolved promise.

so convert function(req, res) to async version and await handler() or call

handler().then(cb)

You should also stick to either callbacks or promises in your mongoose models. If you omit the callback then the returned result of calling Model.find/delete/insert() is usually a promise.

I would suggest refactoring code to something like.

router.get('/usercheck', async function(req, res) {
  const { u:username, e:email } = req.query;
  try {
    const { username:u, email:e } = await Person.findOne({ username, email });
    if(e && u) {
       res.status(200) //Both available
    }
    else if (e && !u) {
       res.status(409) //"Email free, user taken"
    }
    else if (!e && u) {
       res.status(403) //"Email taken, user free"
    }
    else if (e!=null && u!=null) {
       res.status(418)
    }
  } catch(e) {
    res.status(500).json(e);
  }
});
shanks
  • 852
  • 9
  • 23
0

There are lots of things I am unhappy regarding style and clarity about the code you've written, but the reason you aren't getting results in the second case is simply that you aren't returning from your usr() and eml() functions. It should be solved by prefixing return immediately before the Person.findOne(... lines.

jhanschoo
  • 817
  • 10
  • 15