5

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?

Bram Vanroy
  • 27,032
  • 24
  • 137
  • 239

4 Answers4

1

Try returning the rejected promise.

return Promise.reject('Error when getting venue details')
rnli
  • 575
  • 8
  • 16
  • In addition to your answer, the inner `fetch()` should also be returned to the `map()` function. Cf. tehhowch's and Martín Zaragoza's answers for additional fixes. – Bram Vanroy Aug 14 '18 at 17:58
1

When working with promises you should return inner promises instad of working with inner "thens".

Check this:

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 {
              return Promise.reject('Error when getting venue details')
            }
          })
      });

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


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

When returning Promise.all as a value, you're returning a promise so that you can chain further "then" callbacks. The last catch shall capture all promise rejects.

You're also missing the return in the else clause

Hope this helps

Martín Zaragoza
  • 1,717
  • 8
  • 19
  • In addition to your answer, the inner `fetch()` should also be returned to the `map()` function. Cf. tehhowch's answer. – Bram Vanroy Aug 14 '18 at 17:57
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')
}
tehhowch
  • 9,645
  • 4
  • 24
  • 42
  • Ah, this makes sense. I appreciate the thorough explanation. After applying your fixes, the code does indeed run as expected. *I think.* The API returns a 429 error for now (which I don't mind), and my code falls back on backup data. However, Chrome's console still shows all failed GET requests (error 429). Does that mean that I am still missing some errors that I didn't catch, or is this some browser behaviour that you can't get around when encountering failed GET requests? – Bram Vanroy Aug 14 '18 at 17:56
  • I'm not familiar with Chrome's console, so I can't help you there. You should probably also be passing the venue data or the backup data to `this.parseFsqData` (or something), i.e. `.then(venueData => venueData.map(entry => entry.response.venue)).catch(e => { console.log(e); return getBackupData()}).then(venues => { this.parseFsqData(venues); });` – tehhowch Aug 14 '18 at 17:58
1

I believe the solution is fairly simple; The response of the nested fetch method is missing a return statement. You should get rid of that mysterious error once it is in place.

const venuePromises = venueIds.map(venueId => {
    <missing return statement here> fetch(`https://api.foursquare.com/v2/venues/${venueId}?${otherParams}`)
      .then(response => {
therufa
  • 2,050
  • 2
  • 25
  • 39