2

Axios is described as Promise-based, so is there a need for returning a new Promise when using Axios to query for data?

app.get('/api/nearbyRecommendations', async (req, res) => {

    if(!req.query) return res.send({ error: 'Please enable location to get recommendations.' })

    try {
        const { longitude, latitude } = req.query
        const locationName = await location.getLocationName(longitude, latitude)
        res.send(locationName)
    } catch (error) {
        res.send(error)
    }
})   

I am making a GET request to the MapBox API, but I do not seem to ever get any errors despite setting up the catch block for my Axios request, even if I throw a new Error in the .then() block.

const getLocationName = async (latitude, longitude) => {
    return new Promise((resolve, reject) => {
        axios.get(`https://api.mapbox.com/geocoding/v5/mapbox.places/${longitude},${latitude}.json?access_token=${darkSkyAPIKey}`, {json: true})
        .then(response => {
            if(!response.data) return reject({ error: 'No location found.' })

            resolve(response.data)
        }).catch(error => {
            reject(error)
        })
    })
}

If possible, do help and point out anything that may be altered to follow best practices.

Leon Kho
  • 97
  • 2
  • 8
  • 3
    **is there a need for returning a new Promise when using Axios**: Nope. Axios fully supports promises and no need to wrap promise in a promise. – ambianBeing Sep 08 '19 at 17:55
  • 2
    @ambianBeing not to mention that `async` functions implicitly return a Promise anyway. – Jared Smith Sep 08 '19 at 17:56
  • @ambianBeing However, the Promise that I've returned would work as intended with a normal fetch request, right? – Leon Kho Sep 08 '19 at 17:59
  • 1
    @LeonKho `fetch()` also returns a promise so creating a new one is part of the above linked anti-pattern – charlietfl Sep 08 '19 at 18:01
  • @LeonKho Yes it should. Having said that, returning the `axios` result simply would be a neat/cleaner way and less error prone. Also do read that link @AsafAviv shared, pretty good. – ambianBeing Sep 08 '19 at 18:02

1 Answers1

5

You can just return the promise immeditately without using an async function:

const getLocationName = (latitude, longitude) => {
  return axios.get(`https://api.mapbox.com/geocoding/v5/mapbox.places/${longitude},${latitude}.json?access_token=${darkSkyAPIKey}`, {json: true})
  .then(response => {
      if(!response.data) 
        throw Error('No location found.')
      return response.data;
  }).catch(error => {
      console.log(error);
      throw error;
  })
}

Axios.get already returns a promise to you. If you also define the function as async that means that the returned promise will be wrapped in a promise again. So in your example you were triple wrapping the response in a promise. If you replace it with the getLocationName function with a regular function, usage in the first code snippet will remain exactly the same.

etarhan
  • 4,138
  • 2
  • 15
  • 29
  • 1
    Where are resolve and reject coming from? – Jared Smith Sep 08 '19 at 17:56
  • @etarhan That makes sense, thank you for your feedback! – Leon Kho Sep 08 '19 at 17:59
  • @JaredSmith Good point, I made a copy paste error, adapted my response to reflect a working example. – etarhan Sep 08 '19 at 18:00
  • 2
    If all you do is log the error in catch ... `getLocationName(lat, lng).then(res...` will receive `undefined` if error does occur – charlietfl Sep 08 '19 at 18:09
  • @charlietfl you are correct. It would be better to let the error bubble up and handle it in the try catch, or log it like this and rethrow the error. I have adapted the code example to reflect the second case. – etarhan Sep 08 '19 at 18:19