sleep function during .map in async-await not working

Question

I'm trying to fetch data using the Spotify API endpoint, this is my code:

  //gets around 300+ items from my MongoDB
  let artistsArray = []
  const artists = await Artist.find({ createdBy: userId })

  const sleep = m => new Promise(r => setTimeout(r, m))

  const fetchData = async (artist, groupType) => {
    try {
      const data = await axios.get(
        `/artists/${artist.artistSpotifyId}/albums?include_groups=${groupType}&limit=5`,
      )
      
      // working with data here, putting it into the artistArray not important
      
    } catch (error) {
      if (error.response.status === 429) {
        //rate limit error, if received, sleep for suggested time and retry
        const retryAfter = parseInt(error.response.headers['retry-after']) * 1000
        await sleep(retryAfter)
        await fetchData(artist, groupType)
      }
    }
  }

  //should go through each artist and get albums for each artist
  await Promise.all(
    artists.map(async artist => {
      await fetchData(artist, 'album')
      await fetchData(artist, 'single')
    }),
  )

When I tested that with few items, it worked fine, however with higher amount of items(mentioned 300+) I started getting rate limit errors 429 from Spotify. They always suggest retry-after period so I tried to implement it in the catch, so there'd be a sleep triggered every time error is catched but I can't seem to figure out why it doesn't work using the .map. When error 429 gets encountered, the loop just continues.

When using for loop, it works just fine, but it's kind of slow and therefore I haven't encountered any rate limit error.

for (let artist of artists) {
    await fetchData(artist, 'album')
    await fetchData(artist, 'single')
  }

Any suggestion why is .map ignoring the sleep function?

Promise.all executes all functions concurrently. If you want them to run in sequence, you need to go back to the for loop. — , Aug 22 '22 at 15:35
`.map()` doesn't await each iteration, so you're calling the API simultaneously for all the artists. Each of them is waiting before its next call, but then they all retry simultaneously, so you get rate-limited again. — Barmar, Aug 22 '22 at 15:35
It will be slower, but that's because it's obeying the rate limit. — Barmar, Aug 22 '22 at 15:36
It's not ignoring anything. You misunderstand how async await works. Use a for loop. It makes more sense to do so — Jaromanda X, Aug 22 '22 at 15:37
`Promise.all` This is the reason I've always said that this function should come with warnings. And one of the reasons I think it would be great if ES spec implement a promise based concurrency map. The amount of times people reply on SO with use `Promise.all` without considering the potential of thrashing, this question is a classic example.. — Keith, Aug 22 '22 at 15:46

score 3 · Accepted Answer · answered Aug 22 '22 at 15:40

Array.map() is not going to wait for the completion of the previous function call before calling the next one, it's just not how it works.

As in this example you can see that all the console statements print at the same time.

const sleep = m => new Promise(r => setTimeout(r, m));
const arr = [
    1,2,3,4
];

arr.map(async item => {
    await sleep(2000);
  console.log(item);
});

They do so after a 2 second delay. But all print at roughly the same time (after the same 2 seconds wait time).

I would recommend using a for...of loop since that will cause each pass through the loop to await the completion of the previous.

async function main(){
  const sleep = m => new Promise(r => setTimeout(r, m));
  const arr = [
    1,2,3,4
  ];

  let newArray = []; //to simulate the behavior of Array.map();
  for(const item of arr){
    await sleep(2000);
    console.log(item);
    newArray.push(item); //to simulate the behavior of Array.map();
  }

  console.log(newArray);
}

main();

score 1 · Answer 2 · answered Aug 22 '22 at 15:36

The sleep isn't ignored, I don't think (unless the delay ends up being e.g. zero). (You could find that out by e.g. adding suitable console.logs.)

The issue is that by using artists.map() like that you're shooting off 300 requests in a single instant, some of which may succeed, some of which may not, and some may have "overlapping" retry-after hints.

You should probably limit the overall concurrency of your requests using e.g. p-queue or p-limit.

danh · Answer 3 · 2022-08-22T16:12:14.757

Others have explained that the failing requests are those that are started before the rate limit is hit. A simple fix is to perform the requests in chunks with delays in between.

// thanks to https://stackoverflow.com/a/60779547/294949
const chunk = (array, size) =>
  array.reduce((acc, _, i) => {
    if (i % size === 0) acc.push(array.slice(i, i + size))
    return acc
  }, []);

// like the OP's but nothing fancy in the catch
const oneFetch = async (artist, groupType) => {
  try {
    const data = await axios.get(
      `/artists/${artist.artistSpotifyId}/albums
      include_groups=${groupType}&limit=5`,
    )
    // working with data here
    } catch (error) {
      console.log(error)
    }
  }
}

// the OP's sleep
const sleep = m => new Promise(r => setTimeout(r, m))

Doing a chunk is just like the original logic with a sleep at the end.

const doAChunk = async (array, groupType, thenDelay) => {
  const promises = array.map(a => oneFetch(a, groupType))
  const results = await Promise.all(promises)
  await sleep(thenDelay);
  return results;
}

// the OP function modified. find the right chunk size
// and retry time from api docs (and/or experimentation)
const fetchData = async (artist, groupType, chunkSize, delayBetween) => {
  let artistsArray = [];
  const artists = await Artist.find({ createdBy: userId })
  const chunks = chunk(artists, chunkSize);
  const promises = chunks.map(chunk => doAChunk(chunk, 'album', delayBetween);
  // notice the flat(), since chunking produces arrays of arrays
  const results = await Promise.all(promises).flat();
  return results;
}

sleep function during .map in async-await not working

3 Answers3