Firebase functions trigger onDelete works sometimes

I have a Firestore Database that looks like this:

vehicle collection

orgs collection

users collection

One collection for vehicles, one for organisations and one for users. In the organisations collection every document has a field called vehicles that contains the vehicles from the vehicle collection that this company owns. Every document in the users collection has a field named vehicles which contains all the vehicles that this user have access to. In my app I can delete the whole organisation (delete the document in the orgs collection). Then I have cloud functions that takes care of the rest. Or atleast should.

exports.deleteVehiclesInOrg = functions.firestore.document("/orgs/{orgId}").onDelete((snap) => {
 const deletedOrgVehicles = snap.data().vehicles;
 return deleteVehiclesInOrg(deletedOrgVehicles);
});

const deleteVehiclesInOrg = async(deletedVehicles: string[]) => {
 for (const vehicle of deletedVehicles) {
  await admin.firestore().doc(vehicles/${vehicle}).delete();
 }
  return null;
};

This trigger function above deletes all the vehicles from this organisation which trigger this function below when a document from the vehicle collection is deleted:

const getIndexOfVehicleInUser = (vehicle: string,user: FirebaseFirestore.DocumentData) => {
 for (let i = 0; i < user.vehicles.length; i++) {
  if (user.vehicles[i].vehicleId === vehicle) {
   return I;
  }
 }return null;
};

const deleteVehiclefromUsers = async (uids: [{ userId: string }],vehicleId: string) => {
for (const user of uids) {
 const userSnap = await admin.firestore().doc(`users/${user.userId}`).get();
 const userDoc = userSnap.data();
 if (userDoc) {
  const index = getIndexOfVehicleInUser(vehicleId,userDoc);
  userDoc.vehicles.splice(index, 1);
  await admin.firestore().doc(`users/${user.userId}`).update({ vehicles: userDoc.vehicles });
 }
}
return null;
};

exports.deleteVehicleFromUsers = functions.firestore.document("/vehicles/{vehicleId}").onDelete((snap, context) => {
 const deletedVehicleId = context.params.vehicleId;
 const deletedVehicleUsers = snap.data().users;
 return deleteVehiclefromUsers(deletedVehicleUsers, deletedVehicleId);});

The deleteVehiclesInOrg function trigger as it should, firebase function always deletes all the vehicles that was in the orgs document. This should trigger the deleteVehicleFromUsers function which deletes the vehicles from the user document. My problem is that sometimes it does and sometimes it doesn't. Most of the times if I have around 10 vehicles it only deletes about 6-8. But every time all the vehicles are being removed as they should.

Is there a promise that I don't handle correctly or isn't it possible to rely on background trigger functions like this, when it was another function (deleteVehiclesInOrg) that deleted the document that should trigger the function deleteVehicleFromUsers?

Here's my bet: (just guessing...)

This line in deleteVehiclefromUsers :

await admin.firestore().doc(`users/${user.userId}`).update({ vehicles: userDoc.vehicles });

is being executed simultaneously by different triggers and, if they're all working with the same user document, they're overwriting each other's vehicles array. Remember triggers are async so they may be executed concurrently without waiting for the other triggers to finish first.

Example:
vehicles = [A, B, C, D]

• Trigger 1 reads user and removes C => vehicles = [A, B, D]
• Trigger 2 reads user and removes D=> vehicles = [A, B, C]
• Trigger 1 writes user and stores => vehicles = [A, B, D]
• Trigger 2 writes user and stores => vehicles = [A, B, C]

Final vehicles is [A, B, C] instead of [A, B].

Prove this is actually the case:
Add some logs to the beginning/end of your trigger, just to make sure they were actually triggered, and the user document id they're updating. If you're deleting 10 vehicles and your trigger doesn't trigger (at least) 10 times, your problem is somewhere else.
(yes, a trigger very very occasionally may trigger more than once).

How to solve it:
Use a firestore transaction. This way, you will get() the user document and update() it atomically, meaning you'll be writing the vehicles array to the same array you read (and not to the array that was already written by another trigger).

That would be something like this: (not tested)

const userRef = admin.firestore().doc(`users/${user.userId}`);
await db.runTransaction(async (t) => {
  const userSnap = await t.get(userRef);
  if (userSnap.exists) {
    const userDoc = userSnap.data();
    const index = getIndexOfVehicleInUser(vehicleId,userDoc);
    userDoc.vehicles.splice(index, 1);
    t.update(userRef, { vehicles: userDoc.vehicles });
 }  
});

I personally recommend reading thoughtfully about transactions, they're an important concept to have in mind. Also notice the transaction may run more than once in case of collisions and may permanently fail if many many transactions are being run on the same document (like deleting at once 100 vehicles that belong to the same user, for example).

---

EXTRA:

In response to your comment, what I do is estimate a very large amount of possible documents to delete, and delete in bulk with a "sleep" in between. I know it sounds awful but it works and it gives time enough for the triggers not to collide.

You just need to make sure the original trigger (onDelete orgs) has time enough to complete, and that transactions on user are spread enough not to collide that much.

The "numbers" depend on your use case. Let's say for example:

• You estimate a huge organization will have 1000 vehicles, make your calculations based on 2000 vehicles, let's say.
• Functions have a timeout of at most 9 minutes. Try to make all this fit in... 5 minutes. You can use runWith() to adjust the timeout. For example: functions.firestore.runWith({ memory: '1GB', timeoutSeconds: 540 }).document("/orgs/{orgId}").onDelete(...);
• Consider the worst case: a single user in an organization has access to ALL of the vehicles.

You could delete vehicles in bulks of 20, with a sleep of 1 second in between: 1000 vehicles / 20 vehicles by bulk = 50 iterations x (1 sec sleep + ~0.2 sec firestore) => ~60 seconds, VERY roughly.

I'd change this function: (again, not tested at all)

const deleteVehiclesInOrg = async(deletedVehicles: string[]) => {
  const db = admin.firestore();
  const bulkWriter = db.bulkWriter();
  let i = 0;
  for (const vehicle of deletedVehicles) {
    const docRef = db.doc(vehicles/${vehicle});
    bulkWriter.delete( docRef );
    i++;
    if ( i % 20 == 0 ) { // bulk size
      bulkWriter.flush();
      await new Promise(r => setTimeout(r, 1000)); // sleep for a sec
    }
  }
  await bulkWriter.close(); // flush and wait the remaining are committed
  return null;
};

Better yet, to distribute them even more you could use a bulk size of 10 and sleep for 500 ms. (or bulk 5 and sleep 250 ms, you get the idea...)

Also, you should have a try-catch around your transaction in users, so you can at least error log in case a transaction definitively fails because of reaching the max collisions.

PS: notice the use of bulkWriter which is much more efficient than doing individual deletes. Find this info in the same link above (firestore transactions and bulk writes).