0

I have following schema

const schema = new Schema(
  {
    ImageId: { type: String, ref: models.IMAGES },
    reportedBy:[
      userId:{type:String, ref: models.USER}
    ]
  }
);

class ImageReport {}

schema.loadClass(ImageReport);

module.exports = model(models.IMAGEREPORT, schema);

And following code

reportImage: async (req, res, next) => {
    try {
      const { ImageId } = req.params;
      const image = await Image.findOne({ ImageId }); // there is an image schema with fields: ImageId and url
      if(!image) 
        return res.status(400).send({
           statusCode: 400,
           error:"CANNOT FIND THE IMAGE",
           data: {},
        });

      const userId = "some userId";
      try{
        let imageReports = await ImageReport.findOne({ ImageId:ImageId})
        if(imageReports){
          imageReports.reportedBy.forEach(rep=>{
            if(rep.userId===userId)
         // this error should be returned immediately. Instead it goes to ImageReport.updateOne() call
              return res.status(400).send({
                statusCode: 400,
                error: "YOU HAVE ALREADY REPORTED THIS IMAGE",
                data: {},
               });
          });
        }  
      } catch(err){
        return res.status(400).send({
             statusCode: 400,
             error: error,
             data: {},
            });;
      }
       
   // this is called even if the error is found in the above try catch block
      await ImageReport.updateOne(
        {ImageId:ImageId},
        {
          $push:{
            reportedBy:userId
          }
        },
        {upsert:true},
        function (error) {
          if (error) {
            return res.status(400).send({
             statusCode: 400,
             error: error,
             data: {},
            });
          }
        }
      )
      
      return res.status(200).send({
        statusCode: 200,
        error: 'THE IMAGE WAS REPORTED SUCCESSFULLY',
        data: {},
      });

    } catch (e) {
      return res.status(400).send({
        statusCode: 400,
        error: error,
        data: {},
      });
    }
  },

When i run this query and suppose the user already reported the image (i.e., their userId is in the 'reportedBy' array) the API call doesn't return error asap. instead it goes to the ImageReport.updateOne() call and inserts the document and then returns the error saying("YOU HAVE ALREADY REPORTED THIS IMAGE")

Can anyone tell me why isn't async await working properly? I am getting following error too

Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on 
unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:16920) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
  • The problem isn't with `async`/`await`, it's that you cannot `return` from a `forEach` "loop". – Bergi Mar 30 '21 at 20:27

2 Answers2

1

Use for of instead of forEach method because your return just finishes a callback of a certain iteration inside forEach:

for (const rep of imageReports.reportedBy) {
  if(rep.userId===userId)
  // this error should be returned immediately. Instead it goes to ImageReport.updateOne() call
  return res.status(400).send({
    statusCode: 400,
    error: "YOU HAVE ALREADY REPORTED THIS IMAGE",
    data: {},
  });
}
Anatoly
  • 20,799
  • 3
  • 28
  • 42
0

I think it is a case of refactoring. The error 400 is returned, of course, but it stops only the scope the error is inserted. Note that this error is sent at second catch, but the scope of the first one (update) remains.

As JS can be tricky with scopes, I would make some changes to try avoid this behvior:

  1. Split the functions - insert and update logic in different functions that are called (using await). Ex:
findProcessedImageReport: async (params) => {
   // ...
   if(imageReports){
          imageReports.reportedBy.forEach(rep=>{
            if(rep.userId===userId)
                throw new Error("YOU HAVE ALREADY REPORTED THIS IMAGE");
          });
        }

// ... not error 
   return imageReportData;
}

updateReportImage: params => { // ... logic }

  1. Handle the errors once, at the call of this functions. In the function itself, I would throw the error instead respond it directly to express. So, I would get the throwed error at the place of express response.
reportImage: async () => {
   try {
       const processedImageReport = await findProcessedImageReport(params)
       // The current flow would be stopped if got error "error: "YOU HAVE ALREADY REPORTED THIS IMAGE"

       await processUpdateImageReport(params)
       return res.status(200).json({success: true, ...data})
    } catch (err){
       return res.status(400).json(err)
    }
}

  • 2
    Really greattt..... Thank you so much... It works perfectly fine without any errors or warnings.... Great! :) –  Mar 31 '21 at 11:18