Firestore + Firebase Functions Working Slow or Returning too quick

Question

A bit new to this so apologies if it's simple.

I have this firebase function below:

Some Context Code:

const functions = require("firebase-functions");
const cors = require("cors")({
  origin: true,
});
const admin = require("firebase-admin");
admin.initializeApp();
const db = admin.firestore();

Actual Function:

exports.testFunction = functions.https.onRequest((request, response) => {
  cors(request, response, ()=>{
    functions.logger.info(request.body, {structuredData: true});
    const rbody = JSON.parse(request.body);
    const egConst = rbody.messageDets;
    egConst.sent = new Date().toUTCString();
    const setM = db.collection("egColl")
        .doc(rbody.egID)
        .collection("messages")
        .doc()
        .set({
          sent: egConst.sent,
          user: egConst.user,
          text: egConst.text,
        })
        .catch((error) => {
          response.status(400).send({code: 400, body: "Error\n" + error});
        });
    setM.then(
        response.status(200).send({code: 200, body: "Success"})
    );
  });
});

Now this kind of works as planned, but extremely slow. Firstly, I get a 200 Response almost immediately, with speeds that I would expect, but the document is not created for another 40 seconds-1 minute.

So two parts...

1. Why do I get a 200 response before "setM" is done.
2. More importantly, why does it take so long to create the document? If I run a very similar version of this code on the client it is extremely quick, but I run into time sync issues.

Thank you in advanced for any help.

P.S. I'm planning to move it back to the client, but I would still like answers to further my understanding.

Answer 1

The problem is caused by these lines:

setM.then(
  response.status(200).send({code: 200, body: "Success"})
);

Here, instead of passing a callback to the then() function, you are passing the result of the send() function. Because it's not a callback, you are sending back the 200 response code early. In addition, once a function has sent back a response, it is considered 'stale' and performance will drop significantly while it awaits to be shutdown by Cloud Functions. Any work after sending a response may not be completed. So make sure to do any work (like writing to databases, etc) before sending back the response. If you expect to do a long running task, write the task to a database (where it triggers another function) and send back a response to the client telling it that you've accepted the job and where to look for updates on its progress.

It should be:

setM.then(() => {
    response.status(200).send({code: 200, body: "Success"})
});

or just:

setM.then(() =>
    response.status(200).send({code: 200, body: "Success"})
);

However, in addition to this problem, you have chained these actions in the wrong order. By placing setM.then() where it is, you have effectively done this:

const setM = db.collection("egColl")
  .doc(rbody.egID)
  .collection("messages")
  .doc()
  .set({
    sent: egConst.sent,
    user: egConst.user,
    text: egConst.text,
  })
  .catch((error) => {
    response.status(400).send({code: 400, body: "Error\n" + error});
  })
  .then(() => {
    response.status(200).send({code: 200, body: "Success"})
  });

In the above code, if the set() call were to throw an error, the handler you passed to catch() would run as you expect and send back a 400 response, but in addition to that, it would also try to send the 200 response causing another error because you can't send back two different responses.

Next, in your current code you also parse the request's body manually, if you call this endpoint with a Content-Type of application/json (as you should be doing), the body will already be parsed for you.

This is roughly how I would write your function:

exports.testFunction = functions.https.onRequest((request, response) => {
  cors(request, response, () => {
    functions.logger.info(request.body, {structuredData: true});
    const rbody = typeof request.body === "string" ? JSON.parse(request.body) : request.body;

    const { messageDets, egID } = rbody;
    const datetimeString = new Date().toUTCString();

    const messageRef = db.collection("egColl")
      .doc(egID)
      .collection("messages")
      .doc();

    messageRef
      .set({
        sent: datetimeString,
        user: messageDets.user,
        text: messageDets.text,
      })
      .then(
        () => {
          functions.logger.info(`Created new message: egColl/${egID}/messages/${messageRef.id}`);
          response.status(200).json({
            code: 200,
            message: 'Success',
            resourceId: messageRef.id
          })
        },
        (error) => {
          functions.logger.error("Error whilst setting data", error);
          response.status(400).send({
            code: 400,
            message: 'Error',
            // don't leak stack traces!
            // return Firebase error code instead of message if available
            error: error.code || error.message
          })
        }
      )
      .catch((error) => {
        // if everything is working as it should, this will never get called
        
        // you could send this to it's own logger
        functions.logger.error("Unexpected error while responding", error);
      });
  });
});

Answer 2

I would suggest refreshing the Firebase console but that most likely isn't the issue. I would recommend restructuring your code like this and check if the issue still persists:

exports.testFunction = functions.https.onRequest((request, response) => {
    cors(request, response, () => {
        functions.logger.info(request.body, { structuredData: true });
        const rbody = JSON.parse(request.body);
        const egConst = rbody.messageDets;
        egConst.sent = new Date().toUTCString();
        return db.collection("egColl")
            .doc(rbody.egID)
            .collection("messages")
            .doc()
            .set({
                sent: egConst.sent,
                user: egConst.user,
                text: egConst.text,
            }).then(() => {
                return response.status(200).send({ code: 200, body: "Success" })
            })
            .catch((error) => {
                return response.status(400).send({ code: 400, body: "Error\n" + error });
            });
    });
});

or you would have to use async-await. The docRef.set() returns a promise so you would have to await it. But your existing code won't wait for it because you don't have await and hence will end up returning 200 even before the request is processed and terminate the cloud function.