0

I have a cloud function in Firebase that, among a chain of promise invocations, ends with a call to this function:

function sendEmail() {
  return new Promise((accept) => {
    const Email = require('email-templates');
    const email = new Email({...});
    email.send({...}).then(() => {
      console.log('Email sent');
    }).catch((e) => {
      console.error(e);
    });
    accept();
  });
}

I am well aware of the fact that email.send() returns a promise. There's a problem however with this approach, that is, if I were to change the function to be:

function sendEmail() {
    const Email = require('email-templates');
    const email = new Email({...});
    return email.send({...});
}

It usually results in the UI hanging for a significant amount of time (10+ seconds) because the time it takes from the promise to resolve equals the amount of time it takes for the email to send.

That's why I figured the first approach would be better. Just call email.send() asynchronously, it'll send the email eventually, and return a response to the client whether the email has finished its round trip or not.

The first approach is giving me problems. The cloud function finishes execution must faster, and thus ends up being a better experience for the user, however, the email doesn't send for another 15+ minutes.

I am considering another approach where we have a separate cloud function hook that handles the email sending, but I wanted to ask StackOverflow first.

MackProgramsAlot
  • 583
  • 1
  • 3
  • 16
  • 3
    Yes, avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it). If you want [just fire off the async function and not wait for it](https://stackoverflow.com/q/32384449/1048572), your function should not return a promise at all though. – Bergi Oct 10 '20 at 23:03
  • 1
    "*the email doesn't send for another 15+ minutes.*" - that doesn't really make sense to me. I doubt it is caused by the change in this code. – Bergi Oct 10 '20 at 23:05
  • Bergi, hence my question, when I return email.send(), the email sends 10 seconds later, and all is good. When I go with the Promise constructor antipattern, the result is an email that doesn't send for a long period of time... I am not lying I swear! ;) – MackProgramsAlot Oct 10 '20 at 23:07
  • 2
    I noticed this is a similar question: https://stackoverflow.com/questions/63581753/firebase-functions-how-best-to-await-unblocking-promises-after-response-is-sent – MackProgramsAlot Oct 10 '20 at 23:16
  • Please edit the question to show the complete, minimal function that exhibits a problem. You shouldn't ever ignore promises in Cloud Functions, and we need to see how you are using this function in context. – Doug Stevenson Oct 10 '20 at 23:26
  • The two versions are not equivalent. In the first version, the `accept()` (more often resolve()) call should be in the `.then()` callback. Otherwise `accept()` is called immediately, hence sendEmail's caller will be returned a promise that is already resolved - without delay. There should also be a `reject()` call in the `catch()` callback. The second version makes far more sense. The timing you observe should be a true reflection of what actually happens. – Roamer-1888 Oct 11 '20 at 02:14

1 Answers1

0

I think there are two aspects being mixed here.

One side of the question deals with promises in the context of Cloud Functions. Promises in Cloud Functions need to be resolved before you call res.send() because right after this call the function will be shutdown and there's no guarantee that unresolved promises will complete before the function instance is terminated, see this question. You might as well never call res.send() and instead return the result of a promise as shown in the Firebase documentation, the key here would be to ensure the promise is resolved properly for example using an idiom like return myPromise().then(console.log); which will force the promise resolution.

Separately, as Bergi pointed out in the comments the first snippet uses an anti-pattern with promises and the second one is way more concise and clear. If you're experiencing a delay in the UI it's likely that the execution gets freezed waiting for the Function response and you might consider whether this could be avoided in your particular use case.

All that said, your last idea of creating a separate function to deal with the email send process would also likely reduce the response time and could even make more sense from a separation of concerns point of view. To go this route I would suggest to send a PubSub message from the main function so that a second one sends the email. Moreover, PubSub triggered function allows to configure retry policies which may be useful to ensure the mail will be sent in the context of eventual errors. This approach is also suggested in the question linked above.

Happy-Monad
  • 1,962
  • 1
  • 6
  • 13