1

I am trying to move database calls out of my controller to clean things up and make them testable. When they are in the controller, everything moves smoothly. I moved them out of the controller and added an async to ensure that we wait. Otherwise, I would call the res.render() function within the .exec() of Users.findOne(). Now, once I am using async/await, the function in my controller thinks that there is no user because it is not waiting.

There are several questions on SO regarding async await, but I did not find one that resolves my issue. I did verify that my User is returned, and I added console logs to show the path.

  • node mongoose async await seemed promising, but their actual problem was failure to return the item, and mine returns fine
  • async not waiting for await is an issue with kotlin
  • async await not waiting Seems very applicable, but I do not understand fully the answer about nested await/async - the problem the asker had was more complicated than my query because they are dealing with loops and forEach
  • Javascript async await is spot on, so I checked that the mongoose function returns a promise. Mongoose docs show that it is ready for async because calls return a promise.

Assume we are resolving the route /users

routes/index.js

// requires & other routes not shown 
router.get('/users', controller.testUserShow);

controllers/index.js

// requires & other routes not shown 
   exports.testUserShow = async (req, res, next) => {
      if (req.user) { // if code to get user is right here, with no async/await, the user is found and the code continues
        try {
          found = await services.fetchUser(req.user._id)
          console.log("I am not waiting for at testusershow")
          console.log(found); //undefined
          // go on to do something with found
        } catch(e) {
          throw new Error(e.message)
        }
      }
    }

services/index.js

const db = require('../db')
exports.fetchUser = async (id) => {
  try {
    console.log("fetchUser is asking for user")
    return await db.returnUser(id)
  } catch(e) {
    throw new Error(e.message)
  }
}

db/index.js

const User = require('../models/user');
exports.returnUser = async (id) => {
  User.findById(id)
      .exec(function(err, foundUser) {
          if (err || !foundUser) {
              return err;
          } else {
              // if this was in the controller 
              // we could res.render() right here
              console.log("returnUser has a user");
              console.log(foundUser); // this is a User
              return foundUser;
          }
      });
}

The console log goes

fetchUser is asking for user
I am not waiting for at testusershow
undefined
returnUser has a user
// not printed... valid user 

I would expect the initial call to be undefined if I was calling something that did not return a promise, but User.findOne() should.

What am I missing here?

jessi
  • 1,438
  • 1
  • 23
  • 36
  • 3
    Your function `returnUser ` isn't actually returning anything. – Mark Dec 09 '18 at 02:50
  • 1
    With the `exec()` call, you can pass a callback function, like `exec(cb)` or use the promise `.exec().then()`, sounds like you need to use the latter. – Alexander Mills Dec 09 '18 at 02:53

2 Answers2

2

This is the simplest way:

const User = require('../models/user');
exports.returnUser =  (id) => {
   return User.findById(id).exec().then(foundUser => { 
          console.log(foundUser); // this is a User
          return foundUser;
  });
}

if you want to use async/await then you can do:

    exports.returnUser = async id => {
       const foundUser = await User.findById(id).exec();
       console.log({foundUser});
       return foundUser;
      });
    }

and if you wanted to use callbacks, it would look like:

exports.returnUser =  (id, cb) => {
   return User.findById(id).exec(cb);
}

the cool think about Mongoose is that if you don't pass a callback it will return a promise from the exec function/method.

Alexander Mills
  • 90,741
  • 139
  • 482
  • 817
  • Wow. I can't believe I did not realize that about exec. That's what I get for taking my code out of one place and not really rewriting it for new location. – jessi Dec 09 '18 at 13:35
1

Your db/index should be like this:

exports.returnUser = (id) => {
  return User.findById(id);
}

When you don't call exec, it will return a promise. And as your services/index.js already uses await to get the response, db/index doesn't need to be an async function.

iagowp
  • 2,428
  • 1
  • 21
  • 32
  • This is a nice simple approach. I like how clean it is. Since I am using .populate() in my actual function, I'm going to have to use the .exec().then(), but it would be beautiful if I did not need it. – jessi Dec 09 '18 at 13:36
  • 1
    @jessi You can use `User.findById(id).populate(populate)` without exec or then. Just add populate after find and it will work the same – iagowp Dec 09 '18 at 15:10
  • Wow - I had no idea @iagowp - I always thought I had to keep going to get it. – jessi Dec 10 '18 at 01:42