Using Promise.all() to fetch a list of urls with await statements

Question

tl;dr - if you have to filter the promises (say for errored ones) don't use async functions

I'm trying to fetch a list of urls with async and parse them, the problem is that if there's an error with one of the urls when I'm fetching - let's say for some reason the api endpoint doesn't exists - the program crushes on the parsing with the obvious error:

UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: ext is not iterable

I've tried checking if the res.json() is undefined, but obviously that's not it as it complains about the entire 'ext' array of promises not being iterable.

async function fetchAll() {
  let data
  let ext
  try {
    data = await Promise.all(urls.map(url=>fetch(url)))
  } catch (err) {
    console.log(err)
  }
  try {
    ext = await Promise.all(data.map(res => {
      if (res.json()==! 'undefined') { return res.json()}
    }))
  } catch (err) {
    console.log(err)
  }
  for (let item of ext) {
    console.log(ext)
  }
}

Question 1:

How do I fix the above so it won't crash on an invalid address?

Question 2:

My next step is to write the extracted data to the database. Assuming the data size of 2-5mgb of content, is my approach of using Promise.all() memory efficient? Or will it be more memory efficient and otherwise to write a for loop which handles each fetch then on the same iteration writes to the database and only then handles the next fetch?

Answer 1

You have several problems with your code on a fundamental basis. We should address those in order and the first is that you're not passing in any URLS!

async function fetchAll(urls) {
  let data
  let ext
  try {
    data = await Promise.all(urls.map(url=>fetch(url)))
  } catch (err) {
    console.log(err)
  }
  try {
    ext = await Promise.all(data.map(res => {
      if (res.json()==! 'undefined') { return res.json()}
    }))
  } catch (err) {
    console.log(err)
  }
  for (let item of ext) {
    console.log(ext)
  }
}

First you have several try catch blocks on DEPENDANT DATA. They should all be in a single try catch block:

async function fetchAll(urls) {
  try {
    let data = await Promise.all(urls.map(url=>fetch(url)))
    let ext = await Promise.all(data.map(res => {
      // also fixed the ==! 'undefined'
      if (res.json() !== undefined) { return res.json()}
    }))
    for (let item of ext) {
      console.log(ext)
    }
  } catch (err) {
    console.log(err)
  }
}

Next is the problem that res.json() returns a promise wrapped around an object if it exists

if (res.json() !== undefined) { return res.json()}

This is not how you should be using the .json() method. It will fail if there is no parsable json. You should be putting a .catch on it

async function fetchAll(urls) {
  try {
    let data = await Promise.all(urls.map(url => fetch(url).catch(err => err)))
    let ext = await Promise.all(data.map(res => res.json ? res.json().catch(err => err) : res))
    for (let item of ext) {
      console.log(ext)
    }
  } catch (err) {
    console.log(err)
  }
}

Now when it cannot fetch a URL, or parse a JSON you'll get the error and it will cascade down without throwing. Now your try catch block will ONLY throw if there is a different error that happens.

Of course this means we're putting an error handler on each promise and cascading the error, but that's not exactly a bad thing as it allows ALL of the fetches to happen and for you to distinguish which fetches failed. Which is a lot better than just having a generic handler for all fetches and not knowing which one failed.

But now we have it in a form where we can see that there is some better optimizations that can be performed to the code

async function fetchAll(urls) {
  try {
    let ext = await Promise.all(
      urls.map(url => fetch(url)
        .then(r => r.json())
        .catch(error => ({ error, url }))
      )
    )
    for (let item of ext) {
      console.log(ext)
    }
  } catch (err) {
    console.log(err)
  }
}

Now with a much smaller footprint, better error handling, and readable, maintainable code, we can decide what we eventually want to return. Now the function can live wherever, be reused, and all it takes is a single array of simple GET URLs.

Next step is to do something with them so we probably want to return the array, which will be wrapped in a promise, and realistically we want the error to bubble since we've handled each fetch error, so we should also remove the try catch. At that point making it async no longer helps, and actively harms. Eventually we get a small function that groups all URL resolutions, or errors with their respective URL that we can easily filter over, map over, and chain!

function fetchAll(urls) {
  return Promise.all(
    urls.map(url => fetch(url)
      .then(r => r.json())
      .then(data => ({ data, url }))
      .catch(error => ({ error, url }))
    )
  )
}

Now we get back an array of similar objects, each with the url it fetched, and either data or an error field! This makes chaining and inspecting SUPER easy.

Answer 2

You are getting a TypeError: ext is not iterable - because ext is still undefined when you caught an error and did not assign an array to it. Trying to loop over it will then throw an exception that you do not catch.

I guess you're looking for

async function fetchAll() {
  try {
    const data = await Promise.all(urls.map(url => fetch(url)));
    const ext = await Promise.all(data.map(res => res.json()));
    for (let item of ext) {
      console.log(item);
    }
  } catch (err) {
    console.log(err);
  }
}

Answer 3

Instead of fetch(url) on line 5, make your own function, customFetch, which calls fetch but maybe returns null, or an error object, instead of throwing.

something like

async customFetch(url) {
    try {
       let result = await fetch(url);
       if (result.json) return await result.json();
    }
    catch(e) {return e}
}

Answer 4

if (res.json()==! 'undefined')

Makes no sense whatsoever and is an asynchronous function. Remove that condition and just return res.json():

try {
    ext = await Promise.all(data.map(res => res.json()))
} catch (err) {
    console.log(err)
}

Whether or not your approach is "best" or "memory efficient" is up for debate. Ask another question for that.

Answer 5

You can have fetch and json not fail by catching the error and return a special Fail object that you will filter out later:

function Fail(reason){this.reason=reason;};
const isFail = o => (o&&o.constructor)===Fail;
const isNotFail = o => !isFail(o);
const fetchAll = () =>
  Promise.all(
    urls.map(
      url=>
        fetch(url)
       .then(response=>response.json())
       .catch(error=>new Fail([url,error]))
    )
  );

//how to use:
fetchAll()
.then(
  results=>{
    const successes = results.filter(isNotFail);
    const fails = results.filter(isFail);

    fails.forEach(
      e=>console.log(`failed url:${e.reason[0]}, error:`,e.reason[1])
    )
  }
)

As for question 2:

Depending on how many urls you got you may want to throttle your requests and if the urls come from a large file (gigabytes) you can use stream combined with the throttle.

Answer 6

async function fetchAll(url) {
    return Promise.all(
      url.map(
        async (n) => fetch(n).then(r => r.json())
      )
    );
 }

 fetchAll([...])
   .then(d => console.log(d))
   .catch(e => console.error(e));

  Will this work for you?

Answer 7

If you don't depend on every resource being a success I would have gone back to basics skipping async/await

I would process each fetch individual so I could catch the error for just the one that fails

function fetchAll() {
  const result = []
  const que = urls.map(url => 
    fetch(url)
    .then(res => res.json())
    .then(item => {
      result.push(item)
    })
    .catch(err => {
      // could't fetch resource or the
      // response was not a json response
    })
  )

  return Promise.all(que).then(() => result)
}

Something good @TKoL said:

Promise.all errors whenever one of the internal promises errors, so whatever advice anyone gives you here, it will boil down to -- Make sure that you wrap the promises in an error handler before passing them to Promise.all

Answer 8

Regarding question 1, please refer to this:

Handling errors in Promise.all

Promise.all is all or nothing. It resolves once all promises in the array resolve, or reject as soon as one of them rejects. In other words, it either resolves with an array of all resolved values, or rejects with a single error.