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
})

You appear to be using ESLint (or similar). Did you refer to its documentation of this test? — T.J. Crowder, Feb 12 '18 at 16:20
@DanielA.White Why is that? I use `Promise.all(profilePromise...).then().catch()` later on — Tometoyou, Feb 12 '18 at 16:20
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. — Santosh Shinde, Feb 12 '18 at 16:34

score 35 · Answer 1 · edited Apr 02 '19 at 05:53

35

Add at the end of the then()

return null

That's it.

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

edited Apr 02 '19 at 05:53

4b0

21,981
30
95
142

answered Apr 02 '19 at 03:17

Marlhex

1,870
19
27

Bergi · Accepted Answer · 2018-02-12T16:26:27.140

29

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 => { … });
})

edited Feb 12 '18 at 16:26

answered Feb 12 '18 at 16:19

Bergi

630,263
148
957
1,375

I have multiple promises where I have to wait for all of them to complete before I move on (see my comment under my post), how do I do that without using promises? – Tometoyou Feb 12 '18 at 16:24
1

What do you mean by "without using promises"? `profilePromise` in my code is still a promise - it just does not use the `Promise` *constructor*, it is created by the `then` call directly. You can just use `Promise.all([profilePromise, …]).then(…)` as you did before. – Bergi Feb 12 '18 at 16:25
I see, ok I'll give it a whirl! – Tometoyou Feb 12 '18 at 16:26

Santosh Shinde · Answer 3 · 2018-02-12T16:52:38.383

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 !!

you can create a global variable and assign the result to that variable — Santosh Shinde, Apr 12 '19 at 13:39
before variable updates next line executes !, how to prevent next line execution still global variable update? My Question - https://stackoverflow.com/q/55650858/2034750 — 151291, Apr 12 '19 at 13:41

score 0 · Answer 4 · answered May 03 '20 at 07:36

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

Only try this if you can't solve the issue. most of the time you can solve this by just return statement inside .then() — Abhin Krishna KA, May 03 '20 at 07:38

score 0 · Answer 5 · answered Jan 21 '22 at 19:36

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:

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"
  }

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

5 Answers5

Linked

Related