0

I have a Firebase collection of devices that have a field called state.

I want my code to loop through each document, find any one document with a state of 0, and update that state to 2. Finally, I want to grab the name of the device whose state was changed.

After running my code, it does find and update a document in the collection properly, however I am unable to return the Promise.resolve(..) that stores the document name.

It seemingly only outputs "Transaction failure: No devices with state 0 found", meaning return Promise.reject('No devices with state 0 found'); is reached when it shouldn't be.

Here is my code:

  let deviceRef = db.collection("devices");

  let transaction = db.runTransaction(t => {
    return t.get(deviceRef)
      .then(snapshot => {
        snapshot.forEach(doc => {
          console.log(doc.id, '=> state:', doc.data().state);
          if (doc.data().state == 0) {
            doc.ref.update({ state: 2 });
            return Promise.resolve(doc.id + ' has been changed to state 2');
          }
        });
        return Promise.reject('No devices with state 0 found');
      });
  }).then(result => {
    console.log('Transaction success', result);
  }).catch(err => {
    console.log('Transaction failure:', err);
  });

The code does reach doc.ref.update({state: 2}); because I can see the Firebase collection being properly modified, however return Promise.resolve(doc.id + ' has been changed to state 2'); does not seem to be "captured".

What am I doing wrong?

I have a suspicion that I am using promises incorrectly. Some research leads me to believe that I need to use Promise.all or some form of map to deal with the forEach.

H. Pauwelyn
  • 13,575
  • 26
  • 81
  • 144
Brian
  • 3
  • 2

1 Answers1

1

Your issue lies in your return of Promise.resolve. It's only going to return into the forEach and the return.reject is always going to execute. I highly suggest adding in some console logs so you can see this happening. Also you have multiple documents that may be updated so I'm not sure exactly how you saw the individual Promise.resolve working, maybe you meant to do something like the following where it keeps an array of promises?

.then(snapshot => {
    var promises = [];
    snapshot.forEach(doc => {
        console.log(doc.id, '=> state:', doc.data().state);
        if (doc.data().state == 0) {
            doc.ref.update({
                state: 2
            });
            //this is only returning into the forEach
            //return Promise.resolve(doc.id + ' has been changed to state 2');
            promises.push(Promise.resolve(doc.id + ' has been changed to state 2'));
        }
    });
    if(promises.length){
        return Promise.all(promises);
    }
    return Promise.reject('No devices with state 0 found');
});
Mathew Berg
  • 28,625
  • 11
  • 69
  • 90
  • I see it now, I will give this a shot. Does the promises array ever become larger than one? Thanks – Brian Dec 20 '19 at 17:59
  • You're doing a `snapshot.forEach` right? And you have multiple documents? So yes? – Mathew Berg Dec 20 '19 at 18:01
  • Yeah sorry, I would have seen the answer to that question as soon as I tried it. I only need to update one device in this transaction. To do this I would need to break out of the forEach as soon as a `Promise.resolve` is pushed to the promise array right? I saw [this post](https://stackoverflow.com/questions/2641347/short-circuit-array-foreach-like-calling-break) on breaking out of forEach, and I was wondering if it's necessarily "bad" to wrap it all in a try-catch and use exceptions to "break". I tried using for-of, but I dont think the snapshot is iterable. Thanks – Brian Dec 20 '19 at 19:02
  • You could just use a for loop on your own instead of the one built onto snapshot. It'd be easy to break out then. Or keep track of a variable so as soon as it finds one the forEach returns immediately and no longer executes. – Mathew Berg Dec 20 '19 at 19:18