Each then() should return a value or throw when using Promises

Question

I have a few async methods that I need to wait for completion before I return from the request. I'm using Promises, but I keep getting the error:

Each then() should return a value or throw // promise/always-return

Why is this happpening? This is my code:

router.get('/account', function(req, res) {
  var id = req.user.uid
  var myProfile = {}
  var profilePromise = new Promise(function(resolve, reject) {
    var userRef = firebase.db.collection('users').doc(id)
    userRef.get()
      .then(doc => { // Error occurs on this line
        if (doc.exists) {
          var profile = doc.data()
          profile.id = doc.id
          myProfile = profile
          resolve()
        } else {
          reject(Error("Profile doesn't exist"))
        }
      })
      .catch(error => {
        reject(error)
      })
  })
  // More promises further on, which I wait for
})

Answer 1

why is this happening ? Each then() should return a value or throw So I am adding this since the why was never explained.

For anyone else wondering the why. this is not a normal JS Error. This is a direct result of using the ES-Lint promise package:

• https://github.com/xjamundx/eslint-plugin-promise
• https://github.com/xjamundx/eslint-plugin-promise/blob/development/rules/always-return.js

This package has a ruleset to error when it does not find a return from a promise. This can be helpful to identify promise flow errors. You could fix this by simply adding a return as thats what the linter is triggered by or editing the es-lint rule set (not recommended). Thats why I assume it works when it is not being linted causing your confusion.

 router.get('/account', function(req, res) {
  var id = req.user.uid
  var myProfile = {}
  var profilePromise = new Promise(function(resolve, reject) {
    var userRef = firebase.db.collection('users').doc(id)
    userRef.get()
      .then(doc => { // Error occurs on this line
        if (doc.exists) {
          var profile = doc.data()
          profile.id = doc.id
          myProfile = profile
          return null
        } else {
          reject(Error("Profile doesn't exist"))
        }
      })
      .catch(error => {
        reject(error)
      })
  })
  // More promises further on, which I wait for
})

Here is a list of the default rulesets used by that package. Hope it helps anyone else trying to get background on why this return was needed.

{
  "rules": {
    "promise/always-return": "error",
    "promise/no-return-wrap": "error",
    "promise/param-names": "error",
    "promise/catch-or-return": "error",
    "promise/no-native": "off",
    "promise/no-nesting": "warn",
    "promise/no-promise-in-callback": "warn",
    "promise/no-callback-in-promise": "warn",
    "promise/avoid-new": "warn",
    "promise/no-new-statics": "error",
    "promise/no-return-in-finally": "warn",
    "promise/valid-params": "warn"
  }

Answer 2

If you can't fix this issue but still want to run your code...

open   : eslintrc.json file (search from project root directory)
search : 'promise/always-return'
change : Case 1: if (existing value is 2) => change to 1
         Case 2: else if(existing value is "error" => change to "warn")

It will make this error into warning, but be careful with it... Also use eslint plungin in your editor to remind of good practice. Otherwise you won't get any promise/always-return related warnings.

Also make sure you find the right eslintrc.json if more than 1 appears on your search

Answer 3

Add at the end of the then()

return null

That's it.

Each then() should return a value or throw Firebase cloud functions

Answer 4

In your case firebase.db.collection('users').doc(id) returning promise itself, please check firebase snippet to here for node-js.

If you have multiple promises and you need to call them one by one then use Promises chaining.

Please check this article this will help you.

Use following code in your case,

          router.get('/account', function(req, res) {

            var id = req.user.uid;
            var myProfile = {};

            var userRef = firebase.db.collection('users').doc(id)

            userRef.get()
            .then(doc =>  {

                if (!doc || !doc.exists) {
                   throw new Error("Profile doesn't exist")
                }

                var profile = doc.data();
                profile.id = doc.id;
                myProfile = profile;

               return myProfile;

            })
            .catch(error => {
              console.log('error', error);
            })

          })

And use Promise.all if you have multiple promises and you want's to execute them in once.

The Promise.all(iterable) method returns a single Promise that resolves when all of the promises in the iterable argument have resolved or when the iterable argument contains no promises. It rejects with the reason of the first promise that rejects.

For example:

  var promise1 =  new Promise((resolve, reject) => {
    setTimeout(resolve, 100, 'foo1');
  });
  var promise2 =  new Promise((resolve, reject) => {
    setTimeout(resolve, 100, 'foo2');
  });
  var promise3 =  new Promise((resolve, reject) => {
    setTimeout(resolve, 100, 'foo3');
  });

  Promise.all([promise1, promise2, promise3])
  .then(result =>  console.log(result))
  //result [foo1, foo2, foo3] 

Hopes this will help you !!

Answer 5

Just avoid the Promise constructor antipattern! If you don't call resolve but return a value, you will have something to return. The then method should be used for chaining, not just subscribing:

outer.get('/account', function(req, res) {
  var id = req.user.uid
  var userRef = firebase.db.collection('users').doc(id)
  var profilePromise = userRef.get().then(doc => {
    if (doc.exists) {
      var profile = doc.data()
      profile.id = doc.id
      return profile // I assume you don't want to return undefined
//    ^^^^^^
    } else {
      throw new Error("Profile doesn't exist")
//    ^^^^^
    }
  })
  // More promises further on, which I wait for:
  // profilePromise.then(myProfile => { … });
})