38

I'm trying to get the hang of using Mongoose promises with the async/await functionality of Node.js. When my function printEmployees is called I want to save the list of employees which are queried by the orderEmployees function. While, the console.log statement inside orderEmployees returns the expected query, the console.log inside of printEmployees returns undefined, suggesting that I'm not returning the promise correctly.

I'm new to promises so entirely possible that I'm not correctly understanding the paradigm... any help is much appreciated.

  printEmployees: async(company) => {
    var employees = await self.orderEmployees(company);
    // SECOND CONSOLE.LOG
    console.log(employees);
  },

  orderEmployees: (companyID) => {
    User.find({company:companyID})
    .exec()
    .then((employees) => {
      // FIRST CONSOLE.LOG
      console.log(employees);
      return employees;
    })
    .catch((err) => {
      return 'error occured';
    });
  },
Patrick Connors
  • 5,677
  • 6
  • 27
  • 54

5 Answers5

49

In order to make orderEmployees behave like async functions, you have to return the resulting promise. There are two rules to follow when using promises without async/await keywords:

  1. A function is asynchronous if it returns a Promise
  2. If you have a promise (for example returned by an async function) you must either call .then on it or return it.

When you are using async/await then you must await on promises you obtain.

This said you will notice that you do not return the promise generated inside orderEmployees. Easy to fix, but its also easy to rewrite that function to async too.

orderEmployees: (companyID) => {
  return User.find({company:companyID}) // Notice the return here
  .exec()
  .then((employees) => {
    // FIRST CONSOLE.LOG
    console.log(employees);
    return employees;
  })
  .catch((err) => {
    return 'error occured';
  });
},

or

orderEmployees: async(companyID) => {
  try {
    const employees = await User.find({company:companyID}).exec();
    console.log(employees);
    return employees;
  } catch (err) {
    return 'error occured';
  }
},

PS: the error handling is somewhat flawed here. We usually do not handle errors by returning an error string from a function. It is better to have the error propagate in this case, and handle it from some top-level, UI code.

Tamas Hegedus
  • 28,755
  • 12
  • 63
  • 97
  • Hi can you do async(companyID) => { await User.find({company:companyID}).then(data =>{ return data }).catch(err => { return err.message }) } – O'Dane Brissett Sep 14 '19 at 04:31
  • @ODaneBrissett What's the question exactly? The code you wrote has many problems, it wouldn't work. Technically you can mix await and then, but I would advise against it as it is easy to get it wrong. Just use async await. `try { return await User.find(); } catch(e) { showerror(e.message); // dont return the error message, you never want to do that }` – Tamas Hegedus Sep 14 '19 at 08:09
  • I was just seeking clarification to see if my approach would work. But thanks for your input. Can you say why it wouldn't work – O'Dane Brissett Sep 14 '19 at 16:29
  • @O'DaneBrissett I noticed 4 things, 1: that async function does not return anything 2: `.then(data =>{ return data })` doesn't do anything, it just evaluates to a similar promise with the same resolution value, 3: `.catch(err => { return err.message })` will hide the exception, makes handling errors impossible, 4: a red flag is that there is no await in an async function – Tamas Hegedus Sep 15 '19 at 17:26
  • Hmm interesting thanks for that, however, this is my full implementation below and it works `exports.getLoan = async (req, res) => { await Loans.findById(req.params.id) .then(data => { if (!data) return res.status(404).send('Sponsored product not found') res.send(data) }) .catch(err => { return res.status(500).send({ message: err.message }) }) }` Just one of the methods in a controller. I tested and it catches the error plus returns the response. I just want to know if there's any faults in this approach – O'Dane Brissett Sep 15 '19 at 22:32
  • That should work fine although it has an unnecessary return statement in catch, and unnecessary async and await. Remove those and replace await with return and the code will work the same. But that means you take a step back instead of forward, the state of the art way would be to eliminate all the thens and using use async and await – Tamas Hegedus Sep 15 '19 at 23:40
  • So when I remove the then, i should just wrap it in try catch and return the results in a const like so: `router.get('/', async (req, res) => { try{ const customers = await Customer.find().sort('name') res.send(customers) }catch{ //What ever code } })` And based on what you're saying is the same thing? – O'Dane Brissett Sep 16 '19 at 13:46
  • Ohhh Okay. My original thought is that the previous code does the same thing but in a more elegant way. Also, is the .exec() really necessary at in my implementation above? given that there's no additional callback? – O'Dane Brissett Sep 16 '19 at 14:31
  • I don't know, as the question is rather about promises and not really about Mongoose I did not look up the documentation. But you should! – Tamas Hegedus Sep 16 '19 at 14:54
  • Yeaaa. Thanks for the input tho. much appreciated – O'Dane Brissett Sep 16 '19 at 15:54
  • `.exec()` was my missing part. thanks, – DragonKnight Jun 25 '22 at 03:32
30

You need to return your Promise.

  • Currently, you are awaiting on a function that returns undefined.
  • await only actually "waits" for the value if it's used with a Promise.
  • Always keep in mind that you can only await Promises or async functions, which implicitly return a Promise1.

orderEmployees: (companyID) => {
  return User.find({ company:companyID }).exec()
}

Also really important, you should throw instead of return in your .catch handler. Returning from within a .catch handler will cause the promise chain to trigger it's .then instead of it's .catch thus breaking the error handling chain.

Better yet, don't include .catch at all and let the the actual error bubble up the promise chain, instead of overriding it with your own non-descriptive 'error occured' message.

Error conditions should throw the error, not return it.


1 You can also await non-Promises, but only for values that are evaluated synchronously.

nicholaswmin
  • 21,686
  • 15
  • 91
  • 167
4

You are not returning a Promise from orderEmployees.

printEmployees: async(company) => {
  var employees = await self.orderEmployees(company);
  // SECOND CONSOLE.LOG
  console.log(employees);
},

orderEmployees: (companyID) => {
  return User.find({company:companyID})
 .exec()
 .then((employees) => {
   // FIRST CONSOLE.LOG
   console.log(employees);
   return employees;
 })
 .catch((err) => {
   return 'error occured';
 });
},
barnski
  • 1,642
  • 14
  • 18
3

You need to return a Promise from orderEmployees

orderEmployees: companyId => User.find({ companyId }).exec()

If you want to do some error handling or pre-processing before you return then you can keep your code as is but just remember to return the result (promises are chainable).

James
  • 80,725
  • 18
  • 167
  • 237
  • I dont see how the [explicit promise creation antipattern](https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it) would help with error handling. What is the benefit? You could just `return employees` and `throw e` from a simple async function. – Tamas Hegedus Sep 27 '17 at 21:11
  • @TamasHegedus yeah good point, old habit from times where Node didn't handle throwing exceptions well from inside promises! – James Sep 27 '17 at 21:17
3

if you're going to use async/await then it works like this.

  • await in front of the function that returns a promise.
  • async in front of the wrapping function.
  • wrap the function body inside try/catch block.

Please have a look on this function, it is a middleware before i execute a specific route in express.

const validateUserInDB = async (req, res, next) => {
    try {
        const user = await UserModel.findById(req.user._id);
        if (!user) return res.status(401).json({ message: "Unauthorized." });
        req.user = user;
        return next();
    } catch (error) {
        return res.status(500).json({ message: "Internal server error." })
    }
}
  • The code after await is waiting the promise to be resolved.
  • Catch block catches any error happened inside the try block even if the error that is triggered by catch method comes from awaiting promise.
Mohammed Ramadan
  • 655
  • 7
  • 10