Ahmed Afify
Ahmed Afify

Reputation: 37

Nodejs appears to return a response before waiting for functions containing Promises to resolve

I am trying to hit my API endpoint 'categories' to return a list of documents from my categories collection in firebase. Using the following code:

function getCategories() {
  let arr = [];
  categories
    .get()
    .then(snapshot => {
      snapshot.forEach(doc => {
        console.log(doc.data().category);
        arr.push(doc.data().category);
        return arr;
      });
    })
    .catch(err => {
      console.log(err);
      return [1, 2, 3];
    });
}

router.route("/categories").get(async function(req, res, next) {
  let arr = await getCategories();
  res.json(arr);
});

While I don't get any errors I receive an empty response which when debugged shows that arr is undefined. The thing is when using the debugger the console logs inside the forEach get called and I see my categories in the console but for some reason, it doesn't wait for the Firebase Promise to resolve and the router just responds with an undefined arr variable.

Upvotes: 1

Views: 57

Answers (4)

Sujay Kundu
Sujay Kundu

Reputation: 71

You can try something like this :

function getCategories() {
  let arr = [];
  categories
    .get()
    .then(snapshot => {
      snapshot.forEach(doc => {
        console.log(doc.data().category);
        arr.push(doc.data().category);
        });
        console.log('arr',arr); //check the array data
        return arr;
    })
    .catch(err => {
      console.log(err);
      return [1, 2, 3];
    });
}

router.route("/categories").get(async function(req, res, next) {
  let arr = await getCategories().catch(err => {
         res.status(400).send(err);
   });
  res.json(arr);
});

Upvotes: 0

Ryan Wheale
Ryan Wheale

Reputation: 28390

Your .then callback should look more like this (there is no need for a separate arr variable):

function getCategories() {
  // notice we are returning the promise
  return categories
    .get()
    .then(snapshot => {
      // notice we are returning the new array using .map()
      return snapshot.map(doc => {
        console.log(doc.data().category);
        return doc.data().category;
      });
    })
    .catch(err => {
      console.log(err);
      return [1, 2, 3];
    });
}

And since you are using async/await already, you can rewrite this all to look like this so that your users can know when an error happened:

// notice the function is now async
async function getCategories() {
    const snapshot = await categories.get()
    // this can be a one-liner in the example above too
    return snapshot.map(doc => doc.data().category);
}

router.route("/categories").get(async function(req, res, next) {
   try {
     let arr = await getCategories();
     res.json(arr);
   } catch(ex) {
     res.status(500).json({ msg: ex.message });
   }
});

Upvotes: 2

Doug Stevenson
Doug Stevenson

Reputation: 317372

getCategories needs to return a promise if you want to use await on it. Right now, it's returning nothing. You'll need to return the promise that you created:

function getCategories() {
  let arr = [];
  return categories
    .get()
    .then(snapshot => {
      snapshot.forEach(doc => {
        console.log(doc.data().category);
        arr.push(doc.data().category);
        return arr;
      });
    })
    .catch(err => {
      console.log(err);
      return [1, 2, 3];
    });
}

Note the return keyword.

You also possibly have another problem here. You're iterating the snapshot using forEach, and returning arr from within the lambda function that you passed to forEach. This will not return the snapshot data to the caller. You will need to return a value from the then lambda:

    .then(snapshot => {
      const arr = []
      snapshot.forEach(doc => {
        console.log(doc.data().category);
        arr.push(doc.data().category);
      });
      return arr;
    })

Upvotes: 1

Ashish Modi
Ashish Modi

Reputation: 7770

You are not returning your promise and value inside the .then. Add return in front of categories.get() call and return value inside then.

Upvotes: 1

Related Questions