1

I'm trying to only make one http call at time but when I log the response from getUrl they are piling up and I start to get 409s (Too many requests)

function getUrl(url, i, cb) {
  const fetchUrl = `https://api.scraperapi.com?api_key=xxx&url=${url.url}`;

  fetch(fetchUrl).then(async res => {
    console.log(fetchUrl, 'fetched!');
    if (!res.ok) {
      const err = await res.text();
      throw err.message || res.statusText;
    }

    url.data = await res.text();
    cb(url);
  });
 }


let requests = urls.map((url, i) => {
  return new Promise(resolve => {
    getUrl(url, i, resolve);
  });
});

const all = await requests.reduce((promiseChain, currentTask) => {
  return promiseChain.then(chainResults =>
    currentTask.then(currentResult => [...chainResults, currentResult]),
  );
}, Promise.resolve([]));

Basically I don't want the next http to start until the previous one has finished. Otherwise I hammer their server.

BONUS POINTS: Make this work with 5 at a time in parallel.

chovy
  • 72,281
  • 52
  • 227
  • 295
  • make the request and make it wait with await. since you make the request in a promise it all resolves at the same time. – moficodes Apr 16 '20 at 04:05

4 Answers4

1

Since you're using await, it would be a lot easier to use that everywhere instead of using confusing .thens with reduce. It'd also be good to avoid the explicit Promise construction antipattern. This should do what you want:

const results = [];
for (const url of urls) {
  const response = await fetch(url);
  if (!response.ok) {
    throw new Error(response); // or whatever logic you need with errors
  }
  results.push(await response.text());
}

Then your results variable will contain an array of response texts (or an error will have been thrown, and the code won't reach the bottom).

The syntax for an async function is an async keyword before the argument list, just like you're doing in your original code:

const fn = async () => {
  const results = [];
  for (const url of urls) {
    const response = await fetch(url);
    if (!response.ok) {
      throw new Error(response); // or whatever logic you need with errors
    }
    results.push(await response.text());
  }
  // do something with results
};

To have a limited number of requests at a time, make a queue system - when a request completes, recursively call a function that makes another request, something like:

const results = [];
const queueNext = async () => {
  if (!urls.length) return;
  const url = urls.shift();
  const response = await fetch(url);
  if (!response.ok) {
    throw new Error(response); // or whatever logic you need with errors
  }
  results.push(await response.text());
  await queueNext();
}
await Promise.all(Array.from({ length: 5 }, queueNext));
// do something with results
CertainPerformance
  • 356,069
  • 52
  • 309
  • 320
  • it doesn't like the await fetch in the for loop – chovy Apr 16 '20 at 04:14
  • 1
    To use `await`, you have to be in an `async` function - make sure you're in an `async` function. – CertainPerformance Apr 16 '20 at 04:17
  • ok, this works but my vscode editor thinks its missing an async declaration. – chovy Apr 16 '20 at 04:20
  • 1
    Like I said, you need to put it in an `async` function, just like you're doing in your original code. Did you try doing that? It'll work. – CertainPerformance Apr 16 '20 at 04:21
  • yeah it works. its inside an async function but the editor just thinks its missing something. Oh well. I can live with this. Any idea how I would make this run 5 at a time in parallel? I know its not part of my original question. But this is taking a long time 2-3 seconds per request. – chovy Apr 16 '20 at 04:24
0

You cannot use Array methods to sequentually run async operations because array methods are all synchronous.

The easiest way to achieve sequential async tasks is through a loop. Otherwise, you will need to write a custom function to imitate a loop and run .then after a async task ends, which is quite troublesome and unnecessary.

Also, fetch is already returning a Promise, so you don't have to create a Promise yourself to contain that promise returned by fetch.

The code below is a working example, with small changes to your original code (see comments).

// Fake urls for example purpose
const urls = [{ url: 'abc' }, { url: 'def', }, { url: 'ghi' }];

// To imitate actual fetching
const fetch = (url) => new Promise(resolve => {
  setTimeout(() => {
    resolve({
      ok: true,
      text: () => new Promise(res => setTimeout(() => res(url), 500))
    });
  }, 1000);
});

function getUrl(url, i, cb) {
  const fetchUrl = `https://api.scraperapi.com?api_key=xxx&url=${url.url}`;
  return fetch(fetchUrl).then(async res => { // <-- changes here
    console.log(fetchUrl, 'fetched!');
    if (!res.ok) {
      const err = await res.text();
      throw err.message || res.statusText;
    }

    url.data = await res.text();
    return url; // <--- changes here
  });
}

async function getAllUrls(urls){
  const result = [];
  for (const url of urls){
    const response = await getUrl(url);
    result.push(response);
  }
  return result;
}

getAllUrls(urls)
  .then(console.log);
yqlim
  • 6,898
  • 3
  • 19
  • 43
0

async/await is perfect for this.

Assuming you have an array of URLs as strings:

let urls = ["https://example.org/", "https://google.com/", "https://stackoverflow.com/"];

You simply need to do:

for (let u of urls) {
  await fetch(u).then(res => {
    // Handle response
  }).catch(e => {
    // Handle error
  });
}

The loop will not iterate until the current fetch() has resolved, which will serialise things.


The reason array.map doesn't work is as follows:

async function doFetch(url) {
  return await fetch(url).then(res => {
    // Handle response
  }).catch(e => {
    // Handle error
  });
}
let mapped = urls.map(doFetch);

is equivalent to:

let mapped;
for (u of urls) {
  mapped.push(doFetch(u));
}

This will populate mapped with a bunch of Promises immediately, which is not what you want. The following is what you want:

let mapped;
for (u of urls) {
  mapped.push(await doFetch(u));
}

But this is not what array.map() does. Therefore using an explicit for loop is necessary.

cyqsimon
  • 2,752
  • 2
  • 17
  • 38
0

Many people provided answers using for loop. But in some situation await in for loop is not welcome, for example, if you are using Airbnb style guide.

Here is a solution using recursion.

// Fake urls for example purpose
const urls = [{ url: 'abc' }, { url: 'def', }, { url: 'ghi' }];

async function serialFetch(urls) {
  return await doSerialRecursion(
    async (url) => {
      return result = await fetch(url)
        .then((response) => {
          // handle response
        })
        .catch((err) => {
          // handle error
        });
    },
    urls,
    0
  );
}

async function doSerialRecursion(fn, array, startIndex) {
  if (!array[startIndex]) return [];
  const currResult = await fn(array[startIndex]);
  return [currResult, ...(await doSerialRecursion(array, fn, startIndex + 1))];
}

const yourResult = await serialFetch(urls);

The doSerialRecursion function will serially execute the function you passed in, which is fetch(url) in this example.