Loop inside a transaction to fetch multiple documents does not work as expected

Question

I am trying to fetch a bunch of documents within a transaction and also update all of them in the same transaction (because they reference each other, this has to be atomic). I know that all reads have to happen before all writes, but still I get The referenced transaction has expired or is no longer valid error in my console.

I see all the ids printed by console.log(Transaction id: ${transactionId}) but never a single output of console.log(Transaction ${transactionId} fetched and pushed to array).

What is wrong with my transaction?

try {
        return admin.firestore().runTransaction(async t => {
            const bundleRef = admin.firestore().doc(`bundles/${bundleId}`)
            const bundle = await t.get(bundleRef)
            const bundleData = bundle.data()
  
            const transactions:FirebaseFirestore.DocumentSnapshot[] = []

            await bundleData!.transactionIds.forEach(async (transactionId: string) => {
                console.log(`Transaction id: ${transactionId}`)
                const transactionRef = admin.firestore().doc(`transactions/${transactionId}`)
                const transaction = await t.get(transactionRef)
                transactions.push(transaction)
                console.log(`Transaction ${transactionId} fetched and pushed to array`)
            })

            console.log(`All transactions fetched`)
            transactions.forEach(async transaction => {
                console.log(`Updating transaction ${transaction.id}`)
                const transactionData = transaction.data()
                transactionData!.timestamp = admin.firestore.FieldValue.serverTimestamp()
                t.update(transaction.ref, transactionData!)
                console.log(`Transaction ${transaction.id} updated`)
            })
            console.log(`Update bundle ${bundle.id}`)
            bundleData!.timestamp = admin.firestore.FieldValue.serverTimestamp()
            t.update(bundle.ref, bundleData!)
            console.log(`Finished`)
        })
    } catch (err) {
        return Promise.reject(new Error("Transaction failed."));
    }

forEach doesn't return a promise that you can await. The work for each item is going to happen after forEach returns. You'll have to rewrite this so the code actually waits on the work completing for each item in each loop. — Doug Stevenson, Aug 13 '20 at 14:38
That makes sense. But still inside the loop i have an async function which awaits the get and after i retrieved the document i am pushing it to an array. Should this not work? Should the loop not complete before the code moves on? In other words: I don't understand why the second log message of my first loop never prints — Avinta, Aug 13 '20 at 14:51
The way you are using await inside an async lambda won't work the way you're expecting. The work is completing asynchronously, after forEach returns. https://stackoverflow.com/questions/37576685/using-async-await-with-a-foreach-loop — Doug Stevenson, Aug 13 '20 at 15:08
This explains it. Okay, i can work with that. Thank you for explanation — Avinta, Aug 13 '20 at 15:10

Renaud Tarnec · Accepted Answer · 2020-08-13T15:21:23.367

As explained by Doug in his comment you cannot use forEach() and async/await this way. To fetch all the transaction documents the best is to use the getAll() method as follows:

    try {

        return admin.firestore().runTransaction(async t => {

            const bundleRef = admin.firestore().doc(`bundles/${bundleId}`)
            const bundle = await t.get(bundleRef)
            const bundleData = bundle.data()

            const transactionRefs = []

            bundleData.transactionIds.forEach((transactionId) => {
                const transactionRef = admin.firestore().doc(`transactions/${transactionId}`)
                transactionRefs.push(transactionRef);
            })

            const transactionSnapshots = await t.getAll(...transactionRefs);

            transactionSnapshots.forEach(transactionSnap => {
                t = t.update(transactionSnap.ref, { timestamp: admin.firestore.FieldValue.serverTimestamp() });
            })

            t.update(bundle.ref, { timestamp: admin.firestore.FieldValue.serverTimestamp() })
            console.log(`Finished`)
        })
    } catch (err) {
        console.log(err)
        return null;
    }

Note that:

You don't need to do transactionData!.timestamp = admin.firestore.FieldValue.serverTimestamp() because you are actually overwriting the existing document data with the same values! Just update with the timestamp, since it is the only field which changes (if I have correctly understood)
(I have removed all the object types, because the only project I have in the machine I'm using at this moment is configured for JavaScript).

Thanks for the t.getAll tip. I was already trying to use a for of loop here. I really did not know that forEach works this way. I learned a lot today. Also for the note: I stripped away most of the updates that are happening, to provide a minimal example. That's why i overwrite with the object iteself — Avinta, Aug 13 '20 at 15:22

Loop inside a transaction to fetch multiple documents does not work as expected

1 Answers1