-1

What I'm trying to do in my endpoint, is:

  1. Make an API call, which returns a JSON
  2. for each item: search in our database for it
  3. If it's found, skip it.
  4. If it's not found, push it in an array "Response"

This is my code:

app.get("/test", (req,res) => {

  spotifyApi.getUserPlaylists({ limit: 50 })
  .then(function(data) {
    let finalres = [];
    const tbp = data.body.items;
    // res.send('ok stop loading');
    
    tbp.forEach(element => locateit(element,finalres));

    console.log('This is the length of finalres, which should be 1:', finalres.length);
    finalres.forEach(item =>{console.log(item)});

    function locateit(item, finalres){
      const thisplaylistid = item.id;

      collection.find({ "id" : thisplaylistid }).toArray((error, result) => {
        if(error) {
          return res.status(500).send(error);
        }

        if(result.length == 0) {    // if we don't find this playlist in our DB
          console.log('This playlist is not in our database: ');
          console.log(thisplaylistid);
          finalres.push(thisplaylistid);
        }
        else{  //if it's already in our DB
          console.log('This item is in our database.'); //This should be printed first, six times.
        }
      });
    };
  });
});

The content of data.body.items is 7 items, where only the first 6 of them are in our DB. This means, that the last item, should be pushed in finalres. Therefore, the expected console outcome should be:

This item is in our database.
This item is in our database.
This item is in our database.
This item is in our database.
This item is in our database.
This playlist is not in our database: 
3uDLmuYPeRUxXouxuTsWOe
This is the length of finalres, which should be 1: 1
3uDLmuYPeRUxXouxuTsWOe

But instead, I get this:

This is the length of finalres, which should be 1: 0
This should be displayed first, six times.
This should be displayed first, six times.
This should be displayed first, six times.
This should be displayed first, six times.
This should be displayed first, six times.
This should be displayed first, six times.
This playlist is not in our database: 
3uDLmuYPeRUxXouxuTsWOe

It is obviously not executed in the right order. I tried to use async-wait, but I'm struggling to understand where/how it should be implemented. Any help? This is the part where I tried it, but I get the same console outcome as before:

async function locateit(item, finalres){
      const thisplaylistid = item.id;

      await collection.find({ "id" : thisplaylistid }).toArray((error, result) => {
...

Update

After reading more about async-wait and promises, I tried to do it this way, but I'm still getting the same output.

app.get("/test", (req,res) => {

  spotifyApi.getUserPlaylists({ limit: 50 })
  .then(function(data) {
    let finalres = [];
    const tbp = data.body.items;
    // res.send('ok stop loading');
    
    for (const playlist of tbp) {
      async function doWork() {
        const found = await indb(playlist.id); //returns t/f if found or not found
        if (!found){
          finalres.push(playlist);
        }
      }
      doWork();
    }
    console.log('This is the length of finalres and it should be 1: ',finalres.length);
  })
});

and the indb function looks like that:

function indb(thisplaylistid){
  return new Promise((resolve, reject) =>{
      console.log('Searching in our DB...');
      collection.find({ "id" : thisplaylistid }).toArray((error, result) => {
          if(result.length == 0) {    // if we don't find this playlist in our DB
              console.log('This playlist is not in our database: ');
              console.log(thisplaylistid);
              resolve(false); //returns the id
          }
          else{  //if it's already in our DB
              console.log('This item is in our database.'); //This should be printed first, six times.
              resolve(true);
          }
      });
  })
}
neonpeach
  • 11
  • 6
  • 1
    The problem is [the usage of `forEach`](https://stackoverflow.com/q/37576685/1048572) (and that `locateit` does not return a promise, yes). – Bergi Dec 30 '20 at 16:41

2 Answers2

1

The problem here is that forEach resolves always resolves as void, no matter if you have async promises running within.

So, your code will return before executing the statements within the forEach

The correct would be wait for all promises to resolve using #Promise.all

Try this instead:

Updated
Using promise as suggested by Bergi instead of callback ( preferable )

app.get("/test", (req, res) => {

  spotifyApi.getUserPlaylists({ limit: 50 })
    .then((data) => {
      // :refac: more meaningful variable names
      const playlists = data.body.items
      return Promise.all(
        playlists.map(
          // :refac: destructuring to get only the id, other ain't necessary
          async({ id }) => 
              collection.find({ id }).toArray()  
        )
      )
      .then(playlistsById => 
        // :refac: no error occurred fetching playlists
        const nonEmptyPlaylists = playlistsById.filter(playlistById => playlistById.length !== 0)
        res.status(200).send(nonEmptyPlaylists)
      )
      .catch(error => {
        // :refac: some error occurred at searching some playlist
        console.log('error', error) 
        // :refac: if you might expect that is going to throw an error here, the code shouldn't be 500
        return res.status(400).send(error)
      })
    })
})
Felipe Malara
  • 1,796
  • 1
  • 7
  • 13
  • That `collection` looks like mongodb so `toArray` should return a promise already if you simply don't pass a callback – Bergi Dec 30 '20 at 19:11
  • If you use `.catch` at the end of your promise chain, there's no reason to `await` that and pass an `async` function to express (which won't properly handle the returned promise). – Bergi Dec 30 '20 at 19:13
  • Yeah, I figure that, but as he didn't mentioned that was specifically using mongo I didn't wanted to deduce that by myself. So I used the callback way to be similar to his code ( even that I dislike this way ). Thanks for the express info, I didn't knew that, gonna update it right here with both suggestions, appreciate Bergi . – Felipe Malara Dec 30 '20 at 20:55
  • Firstly, thank you both @FelipeMalara and @Bergi for replying! I am indeed using express and mongodb- I thought it was irrelevant hence why I didn't use the tags. I tested Felipe's reply, but it does the opposite of what I'm trying to do. `playlistsById` contains the playlists that already exist in my database, while I want to get the ones that are not. This is why I originally used the `finalres` array, to push there any playlists not found in my database. – neonpeach Dec 30 '20 at 21:50
  • Our pleasure (:, oh got it, yeah you would need to change that filter to retrieve where length is actually === 0 or something, but the error was really related to asynchronous handle – Felipe Malara Dec 30 '20 at 23:50
0

As others mentioned, your usage of async/await is wrong. I believe this should work and do what you want, and as a bonus its shorter and easier to read. mastering async and await will simplify your life and save you from callback/promise hell, i highly recommend it

app.get("/test", async (req, res) => {
    
  try {
    
    const data = await spotifyApi.getUserPlaylists({ limit: 50 });

    const tbp = data.body.items;

    const results = [];

    for(let item of tbp) {
      const found = await indb(item.id);
      if(!found){
        results.push(item);
      }
    }
    return res.status(200).send(results);
  }
  catch(err) {
    return res.status(400).send(err);  
  }
  
});
r3wt
  • 4,642
  • 2
  • 33
  • 55
  • 1
    This is indeed clearer and much easier to understand, thank you very much! The only change I had to make was `for(let item of tbp)` instead of `in` at the loop, as otherwise, `item` was becoming `undefined`, and the HTTP response looked something like the index of each field in the JSON. If you agree with that change, please edit your reply so I can choose it as an answer. – neonpeach Dec 30 '20 at 22:39
  • @neonpeach good catch. updated the answer. i'd actually never used this syntax with arrays before, and didn't know it was possible. – r3wt Dec 30 '20 at 22:51