Returning null in firestore function trigger after async code?-CodePudding

I'm having trouble understanding how to properly end a firestore trigger. From what I read from this and this, it seems you should only return null to end a function if there's no async code such as to quickly end if a condition isn't met. When I return null in my scenario below, it seems to work fine. Is there a better practice for what I'm doing that I'm missing?

I need to log my own custom error message which why I need the catch. I know I could return Promise.all here instead of null in the try block, but this is just sudo code for my scenario.

export const delacc = functions.auth.user().onDelete(async (user) => {

  const userUID = user.uid;
  try{
      await admin.firestore().collection("users").doc(userUID).delete();
      await admin.firestore().collection("spam").doc(userUID).delete();
      await admin.firestore().collection("photos").doc(userUID).delete();
      return null;
  }catch(error){
      functions.logger.error(error)
      return null
  }  

});

CodePudding user response：

There's no hard requirement to return null. In fact, async functions always return a promise, no matter what you do inside the function. The promise it returns is based on the completion of any other promises that you await during processing. Even if you explicitly return null, the function is still actually just returning a promise that is immediately fulfilled with the value null. It has no effect on the final outcome, since Cloud Functions onDelete triggers don't use the fulfilled value in any way. The important thing is that the function indeed returns a promise that indicates when all the work is complete (and, as I stated, async functions always do that if you use await correctly inside the function on all async work).

When you're not using async/await, I advise programmers to always return a promise, or null if there is no async work. The null there is an explicit way to tell the reader of your code that you do not intend for Cloud Functions to wait for any async work before fully terminating the function. It helps also helps satisfy eslint or TypeScript warnings, which will suggest to you that you should return something. The value itself isn't really important - it's what you're communicating to others about the termination of your function. Code readability is important if you work with others.

CodePudding user response：

What I'd do is make the delete operations atomic—delete all of the documents or none of them—and return the promise returned by the batch since there isn't any other task performed in this function (which makes me think returning null isn't a necessary abstraction). If the batch throws an error, throw the client a new HTTPS error (along with the batch error) which (a) automatically terminates the cloud function and (b) gives the client the batch error to evaluate, which, depending on the reason, could warrant a retry or an error message to the end user.

export const delacc = functions.auth.user().onDelete(async (user) => {
    const userUID = user.uid;
    const db = admin.firestore();
    const batch = db.batch();

    try {
        batch.delete(db.collection("users").doc(userUID));
        batch.delete(db.collection("spam").doc(userUID));
        batch.delete(db.collection("photos").doc(userUID));
        const writeResult = await batch.commit();
        return writeResult;
    } catch(error) {
        functions.logger.error(error);
        throw new functions.https.HttpsError("unknown", "batch-error", error);
    }
});