Error Handling mongoose with forEach loop

Question

I have a mongodb with express and I'm getting Cannot set headers after they are sent to the client. Unhandled promise rejections are deprecated error when doing the following.

exports.deleteBooking = (req, res, next) => {
  req.body.courts.forEach(element => {
    Booking.deleteOne({$and: [
      { cid: element.cid },
      { year: element.day.year }
    ]})
    .then(result => {
      res.status(201).json({
        message: result
      });
    })
    .catch(() => {
      res.status(500).json({
        error: 'error'
      })
    });
  })
};

I am sending an array of objects to my server and want to perform one deletion per object.

Because of the forEach it might run the .catch after the headers are sent to the client. What is the correct way of handling .then and .catch with a forEach loop?

Thank you!

EDIT: I forgot to add that if I would delete the .then and .catch the query would still run, and without errors. But I would like to keep the error handling in this case.

Should the response, whether it is error or message be send after the entire forEach loop is done or for each item in the loop? — Jeevan
– Jeevan, Commented Nov 22, 2018 at 14:57
So I tried this but then it gives the error Cannot read property 'then' of undefined — Devchris
– Devchris, Commented Nov 22, 2018 at 15:17

chridam · Accepted Answer · 2018-11-22 15:33:14Z

res.status(201).json is within the forEach loop hence why you are getting the above error: you can only send data once.

To fix this, you basically need to send the deleteOne operations once and use bulkWrite() as it allows you to send multiple deleteOne operations to the MongoDB server in one command. It takes in input in form of an array of objects like the following

Booking.bulkWrite([
    { deleteOne: { filter: { cid: 1, year: 2007 } } },
    { deleteOne: { filter: { cid: 2, year: 2007 } } },
    { deleteOne: { filter: { cid: 3, year: 2007 } } },
    { deleteOne: { filter: { cid: 4, year: 2007 } } }, 
])

So in your case you can map req.body.courts array to the above deleteOne operations as

exports.deleteBooking = (req, res, next) => {
    Booking.bulkWrite(
        req.body.courts.map(({ cid, day }) => ({
            deleteOne: { filter: { cid, year: day.year } }
        }))
    ).then(message => {
        res.status(201).json({ message })
    }).catch(error => {
        res.status(500).json({ error })
    })
}

Med Lazhari · Accepted Answer · 2018-11-22 15:19:19Z

0

You can use the Promise.all instead of using forEach

exports.deleteBooking = (req, res, next) => {
  const promises = req.body.courts.map(element => 
    Booking.deleteOne({$and: [
      { cid: element.cid },
      { year: element.day.year }
    ]})
  );

  Promise.all(promises).then((results) => {
    // Handle the result response
  })
  .catch((error) => {
    // Handle the error response
  })
};

answered Nov 22, 2018 at 15:19

Med Lazhari

6185 silver badges9 bronze badges

1 Comment

Devchris Over a year ago

Thanks a lot! This fixed the problem!

Collectives™ on Stack Overflow

Error Handling mongoose with forEach loop

2 Answers 2

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related