0

I would like to know if this promise is ok or not. Because I am using await inside of them.

Example 1:

new Promise(async (resolve, reject) => {
    await fetch(url, postReq)
      .then((data) => {
        return data.json();
      })
      .then((res) => {
        console.log(res);
        resolve("Message sent successfully");
      })
      .catch((error) => {
        console.log(error);
        reject(error);
      });
  });

Example 2:

  const sendEmail = async (reciever, data) => {
      new Promise(async (resolve, reject) => {
        console.log("Sending Email");
        try {
          // create reusable transporter object using the default SMTP transport
          let transporter = nodemailer.createTransport({
            service: "gmail",
            auth: {
              user: process.env.EMAIL_ADDRESS,
              pass: process.env.EMAIL_PASSWORD,
            },
          });

      // send mail with defined transport object
      let info = await  transporter.sendMail({
        from: '', // sender address
        to: `${reciever}`, // list of receivers
        subject: "", // Subject line
        text: ``, // plain text body
        html: ``, // html body
      });

      console.log("Message sent: %s", info.messageId);
      resolve("Email Sent")
    } catch (err) {
      console.log(err);
      reject(err);
    }
  });
};

If I don't use await, I am not able to read info.messageId because it's undefined.

Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
Fran ETH
  • 23
  • 1
  • 6
  • 5
    Why are you wrapping the functionality in `new Promise()` in the first place? It seems like you're over-complicating things. – David Feb 21 '23 at 22:45
  • 4
    `Example 1:` No no no no no. You __already__ got a promise, why wrap it in _another_ promise? `Example 2:` Same thing, async functions __already__ return a promise. – tkausl Feb 21 '23 at 22:46
  • 3
    Protip: _don't_ mix `async`/`await`, `new Promise`, and `.then()` all in the same function. – Dai Feb 21 '23 at 22:46
  • [Promise constructor antipattern](https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it) is the biggest crime in your code - mixing async/await with Promise then/catch is the second :p – Jaromanda X Feb 22 '23 at 00:04

1 Answers1

5

Neither one of these examples is the proper way to do things:

  • Don't wrap an existing promise in a manually created one. Doing so is named the promise constructor anti-pattern.
  • Don't mix .then() and await. Doing so complicates the control flow and readability and proper error handling. You should generally pick one control flow model or the other for a given function.
  • You do not need to catch and rethrow your own errors unless you're going to do something in the catch (like change the error or run some logic or log the error). Promise rejections can just propagate back to the caller in many cases. It will be the caller's responsibility to catch any rejection and handle it appropriately.
  • In a new Promise executor callback, don't make it async and don't use await in it. If you are tempted to do that, then something else is wrong and you probably don't need the new Promise() at all which is the case here (since you apparently already have promises that shouldn't be rewrapped).

Example 1 can be like this:

const response = await fetch(url, postReq);
const data = await response.json();
return data;

or even just this:

const response = await fetch(url, postReq);
return response.json();

Example 2 can be like this:

const sendEmail = async (reciever, data) => {
  console.log("Sending Email");
  // create reusable transporter object using the default SMTP transport
  let transporter = nodemailer.createTransport({
    service: "gmail",
    auth: {
      user: process.env.EMAIL_ADDRESS,
      pass: process.env.EMAIL_PASSWORD,
    },
  });

  // send mail with defined transport object
  const info = await transporter.sendMail({
    from: '', // sender address
    to: `${reciever}`, // list of receivers
    subject: "", // Subject line
    text: ``, // plain text body
    html: ``, // html body
  });

  console.log("Message sent: %s", info.messageId);
  return info;
};
jfriend00
  • 683,504
  • 96
  • 985
  • 979