Array empty in map returning promise - NODE JS

Question

i am newbie on this. My problem is when I tried to return array full of values, it returned empty. I faced some weeks before and I solved it, declaring the map as a const and returning its value in a function... but I can't remeber how I did it.

This is my code:

module.exports.deleteLockedEmail = (req, res, next) => {
  const maxDateAd = new Date().getTime() - 2592000000;
  const adIdsToDelete = [];
  Admin.find({})
    .then(emailLocked => {
      const mapToLockedEm =  emailLocked.map(element => {
        return User.findOne({email:element.email})
                .then(userLocked => {
                  return adIdsToDelete.push(userLocked)
                })
                .catch(error => console.log(error))
      })
      return mapToLockedEm
    })
    .catch(error => console.log(error))

  cron.schedule("* * * * *", function() {
  console.log("running a task every minute => delete locked ads");
  });
}

How can I fill this array?

adIdsToDelete = [];

Use Promise.all

Bergi
– Bergi

2020-04-13 20:34:05 +00:00
Commented Apr 13, 2020 at 20:34 — Bergi
– Bergi, Commented Apr 13, 2020 at 20:34
i feel like this could be done a little cleaner by $lookup

Abdullah Abid
– Abdullah Abid

2020-04-13 20:58:44 +00:00
Commented Apr 13, 2020 at 20:58 — Abdullah Abid
– Abdullah Abid, Commented Apr 13, 2020 at 20:58
@AbdullahAbid I will try too, it is a good point of view

titoih
– titoih

2020-04-14 06:42:19 +00:00
Commented Apr 14, 2020 at 6:42 — titoih
– titoih, Commented Apr 14, 2020 at 6:42

libik · Accepted Answer · 2020-04-14 14:29:56Z

2

Remember that all these calls to database are asynchronous. To get the values you have to wait till the promise is resolved and return values. In this case we can wait for all Promises with Promise.all to be executed and after that in next then context

module.exports.deleteLockedEmail = (req, res, next) => {
  const maxDateAd = new Date().getTime() - 2592000000;
  const adIdsToDelete = [];
  Admin.find({})
    .then(emailLocked => {
      const mapToLockedEm =  emailLocked.map(element => {
        return User.findOne({email:element.email})
                .then(userLocked => {
                  return adIdsToDelete.push(userLocked)
                })
                .catch(error => console.log(error))
      })
      return Promise.all(mapToLockedEm)
    }).then(() => {
      // In this context the adIdsToDelete will be filled
    });
    .catch(error => console.log(error))

  cron.schedule("* * * * *", function() {
  console.log("running a task every minute => delete locked ads");
  });
}

Also in general it can be a little tricky to work with variables like adIdsToDelete as they are in different scope than your promise chain. As it happened in your example - it can be confusing when this variable actually fills with values.

You can rewrite it as following

module.exports.deleteLockedEmail = (req, res, next) => {
  const maxDateAd = new Date().getTime() - 2592000000;
  Admin.find({})
    .then(emailLocked => {
      const mapToLockedEm =  emailLocked.map(element => {
        return User.findOne({email:element.email})
                .catch(error => console.log(error))
      })
      return Promise.all(mapToLockedEm)
    }).then(adIdsToDelete  => {
      // In this context the adIdsToDelete will be filled
    });
    .catch(error => console.log(error))

  cron.schedule("* * * * *", function() {
  console.log("running a task every minute => delete locked ads");
  });
}

edited Apr 14, 2020 at 14:29

answered Apr 13, 2020 at 20:56

libik

23.1k10 gold badges52 silver badges94 bronze badges

Sign up to request clarification or add additional context in comments.

7 Comments

Bergi Over a year ago

You should avoid the pushing though, the Promise.all promise already resolves to an array of all results

titoih Over a year ago

@libik you are absolutely right! I apreciate very much your answer. I have a few question before I started to read more about promise all. 1. Need I write one more .catch? 2. I tried before I posted my question promise.all, but creating new varaible testPromise = , but it didn't work. Promise.all don't need to be promise, just have inside a varaible a promise? Again, thank you! Thank you all.

titoih Over a year ago

@Bergi, it did't work, could you be more specific? thanks!

libik Over a year ago

@titoih - its enough to have just one catch at the end of the promise chain, then any error thrown in promise chain will bubble up to that catch. You should use other catches only when you dont want to break whole promise chain in case of error (like now, when User.findOne ends with error for any reason, it will be catched by internal catch and the promise chain will continue as nothing happens. If this is what you want, keep it. If you want to abort any processing in case of unexpected error, you should remove the internal catch

libik Over a year ago

@titoih - not sure what you mean in your Promise.all question. Just imagine Promise.all as some breakpoint, that will wait for all the promises inside to finish before it continues. It does not affect how the promises inside are handled.

|

Collectives™ on Stack Overflow

Array empty in map returning promise - NODE JS

1 Answer 1

7 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

7 Comments

Your Answer

Sign up or log in

Post as a guest

Related