Promise.all() and catching errors

Question

I wrote a snippet of code that gets some a JSON from the Foursquare API. From this JSON, I get IDs of venues. These IDs are then used to get more details from those specific venues by issuing a fetch() request for every ID, and mapping those requests in an array. That array is then passed into Promise.all(). When the API is available, everything works, but it's the error catching that I can't get my head around.

fetch(`https://api.foursquare.com/v2/venues/search?${params}`)
  .then(response => response.json())
  .then(data => {
      const venueIds = data.response.venues.map(venue => venue.id)

      const venuePromises = venueIds.map(venueId => {
        fetch(`https://api.foursquare.com/v2/venues/${venueId}?${otherParams}`)
          .then(response => {
            // Must check for response.ok, because 
            // catch() does not catch 429
            if (response.ok) {
              console.log('ok')
              return response.json()
            } else {
              Promise.reject('Error when getting venue details')
            }
          })
      })

      Promise.all(venuePromises).then(data => {
        const venues = data.map(entry => entry.response.venue)  // Error for this line
        this.parseFsqData(venues)
      }).catch((e) => {console.log(e); getBackupData()})
  }).catch((e) => {console.log(e); getBackupData()})

  function getBackupData() {
    console.log('backup')
  }

When the API is not available, I get the following console errors (and more of the same):

TypeError: Cannot read property 'response' of undefined
    at MapsApp.js:97
    at Array.map (<anonymous>)
    at MapsApp.js:97

backup

api.foursquare.com/v2/venues/4b7efa2ef964a520c90d30e3?client_id=ANDGBLDVCRISN1JNRWNLLTDNGTBNB2I4SZT4ZQYKPTY3PDNP&client_secret=QNVYZRG0JYJR3G45SP3RTOTQK0SLQSNTDCYXOBWUUYCGKPJX&v=20180323:1 Failed to load resource: the server responded with a status of 429 ()

Uncaught (in promise) Error when getting venue details

I don't understand why then() after Promise.all() is entered, because response is never ok (there is no ok logged in console). Also, I don't understand why the console.log()'s in the catch() blocks aren't executed, or why they are empty. I don't see any caught error information in console, but still the getBackupData function is called. Finally, it is unclear why the last message in console indicates that the error is uncaught, as I expected reject() to make Promise.all() fail.

How can I tactfully catch any errors (included those not normally caught by catch(), such as 429 errors) and call getBackupData when any errors occur?

Answer 1

Your issues are related: namely, the Promise chain must be returned. If you do not return the Promise, you disconnect any of the caller's Promise#catch handling, and any errors in your Promise / then code will result in unhandled promise rejection errors, such as what you have obtained:

Uncaught (in promise) Error when getting venue details

This uncaught promise rejection appears in your code that handles the resolution of fetch:

if (response.ok) {
  console.log('ok')
  return response.json()
} else {
  Promise.reject('Error when getting venue details')  // <----
}

Since this code is being used to construct your venuePromises array, its return value will populate venuePromises. If the response was ok, that array element will have the response JSON from return response.json(). If the response failed, there is no return statement that executes, so the array element has the value undefined. Thus, venuePromises would look like this:

[
  { /** some object for successful response */ },
  undefined,
  { /** some other object */ },
  ...
]

Thus when this array is accessed by your Promise.all's success handler, you get the TypeError since you expected all elements of venuePromises to be valid. This TypeError is caught by the Promise.all's .catch handler (which is why it is logged, and you receive the "backup" text in your log).

To fix, you need to return the Promise.reject, and also the Promise.all. Note that there are some cases of implicit return, but I find it nicer to be explicit, especially if the statement spans multiple lines. Since you're returning the Promise.all statement, you can offload its .then and .catch to the caller, resulting in one less nesting level, and one fewer duplicated .catch handler.

fetch(`https://api.foursquare.com/v2/venues/search?${params}`)
    .then(response => response.json())
    .then(jsonData => {
        const venueIds = jsonData.response.venues.map(venue => venue.id);
        const venuePromises = venueIds.map(venueId => {
            let link = `https://api.foursquare.com/v2/venues/${venueId}?${otherParams}`;
            return fetch(link).then(response => {
                // Must check for response.ok, because catch() does not catch 429
                if (response.ok) {
                    console.log('ok');
                    return response.json();
                } else {
                    console.log(`FAILED: ${link}`);
                    // Return a Promise
                    return Promise.reject(`Error when getting venue details for '${venueId}'`);
                }
            });
        });

        return Promise.all(venuePromises);
    })
    .then(venueData => {
        const venues = venueData.map(entry => entry.response.venue);
        this.parseFsqData(venues);
    })
    .catch(e => {console.log(e); getBackupData()});

function getBackupData() {
    console.log('backup')
}