Is this a good way of breaking "THEN chain" when using promises?

Question

I have a simple "then chain" that runs some functinality steps. If some condition is met I need to cancel the chain and exit (mainly if an error occurrs). I'm using this in a firebase cloud function, but I think it is a basic concept applicable to any node/express eviroment.

This is the code I have:

let p1 = db.collection('products').doc(productId).get();
let p2 = db.collection('user_data').doc(userUid).get();
let promises= [p1,p2];

return Promise.all(promises)
    .then(values => 
    {
        let proudctDoc = values[0];
        let userDataDoc = values[1];

        if(!proudctDoc.exists)
        {
            console.log("The product does not exist");
            response.status(404).json({error: ErrorCodes.UNKNOWN_PRODUCT, msg: "The products does not exist"});
            throw("CANCEL");
        }

        if(!userDataDoc.exists)
        {
            console.log("User data block not found!");
            response.status(404).json({error: ErrorCodes.UNKNOWN_USER_DATA, msg: "User data block not found!"});   
            throw("CANCEL");
        }

        variantCountryRef = db.doc('products/'+productId+'/variants/'+variantCountry);
        return variantCountryRef.get();
    })
.then(variantCountryDoc =>
{
    ....
})
.catch(err =>
{
    if(err !== "CANCEL")
    {
        //Deal with real error
    }
}

As you see, I just run 2 promises and wait for them to finish. After this I check some returned values and notify the client if an error occurs. At this time I must finish the "Then chain".

Is this a common pattern? Anything I could improve?

If any of `Promise.all` rejects, result Promise will reject also. And - *Anything I could improve?* is not a valid question. Post this in Code Review. — Julius Dzidzevičius, Jun 05 '19 at 17:57
what doesn't feel right for you? reliability? try to use async/await instead, you can wrap it into a try catch and it will be elegant and readable, but you mean more like performance, there is not a difference — andresmijares, Jun 05 '19 at 18:06
Better use the power of `if`/`else` and nesting promises if you want to [break the chain](https://stackoverflow.com/a/29500221/1048572). In your particular case, throwing an error might work as well, but then you should throw the `404` error object with the error code and message, and handle it by sending the response. Not "cancelling" the rest of the chain. — Bergi, Jun 05 '19 at 20:09

Renaud Tarnec · Accepted Answer · 2019-06-05T21:01:16.457

You could do as follows:

  const p1 = db.collection('products').doc(productId).get();
  const p2 = db.collection('user_data').doc(userUid).get();
  const promises = [p1, p2];

  return Promise.all(promises)
    .then(values => {
      let proudctDoc = values[0];
      let userDataDoc = values[1];

      if (!proudctDoc.exists) {
        console.log('The product does not exist');
        throw new Error(ErrorCodes.UNKNOWN_PRODUCT);
      }

      if (!userDataDoc.exists) {
        console.log('User data block not found!');
        throw new Error(ErrorCodes.UNKNOWN_USER_DATA);
      }

      variantCountryRef = db.doc(
        'products/' + productId + '/variants/' + variantCountry
      );
      return variantCountryRef.get();
    })
    .then(variantCountryDoc => {
         //Don't forget to send a response back to the client, see https://www.youtube.com/watch?v=7IkUgCLr5oA&
         //...
         //response.send(...);
    })
    .catch(err => {
      if (err.message === ErrorCodes.UNKNOWN_PRODUCT) {
        response.status(404).json({
          error: ErrorCodes.UNKNOWN_PRODUCT,
          msg: 'The products does not exist'
        });
      } else if (err.message === ErrorCodes.UNKNOWN_USER_DATA) {
        response
          .status(404)
          .json({
            error: ErrorCodes.UNKNOWN_USER_DATA,
            msg: 'User data block not found!'
          });
      } else {
        //Deal with other error types
      }
    });

Yep, this is something I was gonna to add later. Just wanted to know if this method was "approved" by more node.js seassoned devs. Thanks for the confirmation. I will do that. I think It is a nice method because all exits are done in just one place and not nested promises are needed :). — Notbad, Jun 05 '19 at 19:58

Is this a good way of breaking "THEN chain" when using promises?

1 Answers1