Asynchronicity issue: why does the second part of my function run before a loop event finishes?

Question

I have a route on an Express server that updates a User profile. The User profile is updated before I have finished to parse the data to update. How so?

I want to update two const: newProfilePicture & newOtherPictures. They are correctly updated, but after the user has been updated, so it's useless. How to fix this asynchronicity issue?

Here is the function:

router.post("/upload-images", upload.array("image"), async (req, res) => {
  const { userId } = req.body;
  try {
    if (req.files) {
      let newProfilePicture = null;
      let newOtherPictures = [];
      req.files.forEach(({ path, originalname }) => {
        cloudinary.uploader.upload(
          path,
          {
            resource_type: "image",
            public_id: `myapp/users/${userId}/${originalname}`,
            crop: "scale",
            quality: "auto",
          },
          (err, res) => {
            if (err) {
              return fs.unlinkSync("./" + path);
            }
            fs.unlinkSync("./" + path);
            if (originalname === "main") {
              return (newProfilePicture = res.secure_url);
            }
            return newOtherPictures.push({
              id: originalname,
              url: res.secure_url,
            });
          }
        );
      });
// THIS PART IS COMPLETE BEFORE THE req.files.forEach IS DONE
      const user = await User.findById(userId);
      const { otherPictures, profilePicture } = updatePictures(
        newProfilePicture,
        newOtherPictures,
        user
      );
      User.findByIdAndUpdate(
        userId,
        { profilePicture, otherPictures },
        { new: true }
      );
      res.send("upload images success");
    }
  } catch (err) {
    console.log("err", err);
    return res.status(500).send("upload images failed");
  }
});

Answer 1

It happens because cloudinary.uploader.upload() runs asynchronously. Since you mentioned it doesn't have promise interface, you can convert the callback to promise using NodeJS's util.promise function as it's error first callback.

const { promisify } = require("util");
const fs = require("fs");
const cloudinaryUpload = promisify(cloudinary.uploader.upload.bind(cloudinary.uploader))

router.post("/upload-images", upload.array("image"), async (req, res) => {
  try {
    if (!req.files) {
      return res.send("no images in the request body");
    }
    let newProfilePicture = null;
    let newOtherPictures = [];
    for (const { path, originalName } of req.files) {
      try {
        const response = await cloudinaryUpload(path, {
          resource_type: "image",
          public_id: `myapp/users/${userId}/${originalName}`,
          crop: "scale",
          quality: "auto",
        });
        await fs.promises.unlink("./" + path);
        if (originalname === "main") {
          newProfilePicture = response.secure_url;
          continue;
        }
        newOtherPictures.push({
          id: originalName,
          url: response.secure_url,
        });
      } catch (error) {
        //do what you want if there is an error
        //throw error if you want
        await fs.promises.unlink("./" + path);
      }
    }
    const user = await User.findById(userId);
    const { otherPictures, profilePicture } = updatePictures(
      newProfilePicture,
      newOtherPictures,
      user
    );
    //use User.updateOne() as you don't need the doc back
    await User.findByIdAndUpdate(
      userId,
      { profilePicture, otherPictures },
      { new: true }
    );
    return res.send("upload images success");
  } catch (error) {
    console.log("err", err);
    return res.status(500).send("upload images failed");
  }
});