Problems with optional promise that has nested promises

Question

I have a server GET request where I am trying to create a promise from a function but it's not going too well. I need this promise to be optional, if that's possible, so that if the result is empty it should be ignored (I've marked where it shouldn't carry on with the function with the words IGNORE FUNCTION).

The idea is that it loops through the docs inside the snapshot, and adds them to an array calls jamDocs. Then if that array isn't empty it should loop through each jam and call an async function for each of them. This then sets the jam's hostName. When this is completed, the original promise should return the jamDocs.

If the jamDocs are empty, the whole promise should be ignored. Finally, after other promises are completed (there is one called profilePromise that returns a profile object), the jams should be assigned to the profile object, and the object sent back to the client with a 200 status code.

My code:

var jamsQuery = firebase.db.collection('users')
                  .where("hostId", "==", id)
                  .orderBy("createdAt")
                  .limit(3)
var jamsPromise = jamsQuery.get().then(snapshot => {
  var jamDocs = []
  snapshot.forEach(doc => {
    var jam = doc.data()
    jam.id = doc.id
    jamDocs.push(jam)
  })
  if (!snapshot.length) { // Error: Each then() should return a value or throw
    return // IGNORE FUNCTION
  } else {
    var jamNamePromises = jamDocs.map(function(jam) {
      var jamRef = firebase.db.collection('users').doc(jam.hostId)
      return jamRef.get().then(doc => {
        if (doc.exists) {
          jam.hostName = doc.data().firstName
          return jam
        } else {
          throw new Error("yeah")
        }
      })
    })
    Promise.all(jamNamePromises)
      .then(function() {
        return jamDocs
      })
      .catch(...)
  }
})

// Later on
Promise.all(profilePromise, jamsPromise)
.then(objectArray => {
  var profile = objectArray[0]

  myProfile.jams = jamDocs
  return res.status(200).send(myProfile);
})
.catch(
  res.status(500).send("Could not retrieve profile.")
)

I get errors like: Each then() should return a value or throw and Avoid nesting promises. How do I fix this? Thanks!

I don't know if it's the cause of your linter error, but your last `.catch` should be `.catch(() => res...)`, not `.catch(res...)`. — Jordan Running, Feb 12 '18 at 17:35
Thanks, that was one issue, but the main error is the `Each then() should return a value or throw` at the point where it says `if (!snapshot.length) {`. — Tometoyou, Feb 12 '18 at 17:37
It seems to me like you ought to have a `return` before the first `Promise.all`. — Jordan Running, Feb 12 '18 at 17:41
@JordanRunning I can put `return Promise.all` and that seems to compile but I'm not sure if what I'm doing is right — Tometoyou, Feb 12 '18 at 17:42
Well, you need to return something in that code path, right? Without a `return` there the promise will be resolved immediately, instead of after `jamNamePromises` are all resolved. In your last `then`, don't you want `myProfile.jams = objectArray[1]`, since `jamsPromise` should resolve to `jamDocs`? — Jordan Running, Feb 12 '18 at 17:46

score 0 · Accepted Answer · answered Feb 12 '18 at 17:48

Your then on this line doesn't return anything and also has nested promises in it. var jamsPromise = jamsQuery.get().then(snapshot => {

You should refactor it, to move the nested promises outside of this then:

var jamsPromise = jamsQuery.get()
    .then(snapshot => {
        var jamDocs = []
        snapshot.forEach(doc => {
            var jam = doc.data()
            jam.id = doc.id
            jamDocs.push(jam)
        })
        if (!snapshot.length) {
            return // IGNORE FUNCTION
        } else {
        var jamNamePromises = getJamDocsPromises();

        return Promise.all(jamNamePromises);
    })
    .catch(...)
});

function getJamDocsPromises(jamDocs) {
    return jamDocs.map(function(jam) {
        var jamRef = firebase.db.collection('users').doc(jam.hostId)
        return jamRef.get().then(doc => {
           if (doc.exists) {
               jam.hostName = doc.data().firstName
               return jam
           } else {
               throw new Error("yeah")
           }
    });
}

score 0 · Answer 2 · answered Feb 12 '18 at 18:52

If the result is empty it should be ignored (shouldn't carry on with the function). If that array isn't empty it should loop through each jam

Don't make it more complicated than it needs to be. Looping over an empty collection does nothing anyway, so you don't need a special case for that. Just continue normally with the empty array.

var jamsPromise = jamsQuery.get().then(snapshot => {
  var jamDocs = []
  snapshot.forEach(doc => {
    jamDocs.push(Object.assign(doc.data(), {id: doc.id}))
  })
  return Promise.all(jamDocs.map(function(jam) {
    return firebase.db.collection('users').doc(jam.hostId).get().then(doc => {
      if (doc.exists) {
        jam.hostName = doc.data().firstName
        return jam
      } else {
        throw new Error("yeah")
      }
    })
  }))
})

I get errors like: Each then() should return a value or throw

You had forgotten a return in front of the Promise.all.

and Avoid nesting promises. How do I fix this?

There's nothing wrong with nesting promises, but in your case the .then(() => jamDocs) was not necessary since the Promise.all already resolves with an array of all the jams that the inner promises fulfill with.

Problems with optional promise that has nested promises

2 Answers2