nchaulet / node-geocoder

nodejs geocoding library
http://nchaulet.github.io/node-geocoder/
MIT License
926 stars 213 forks source link

Fetch adapter should not call body.text() if body.json() throws an error #340

Closed holly-cummins closed 2 months ago

holly-cummins commented 2 years ago

If my geo data provider fails to give a healthy response, the error handling is not correct. I'm seeing errors of the following form

Error getting geography information for San Francisco, CA, USA
Error: HttpError: body used already for: https://nominatim.openstreetmap.org/search?addressdetails=1&q=San+Francisco%2C+CA%2C+USA&format=json
    at /home/styff/platform/node_modules/node-geocoder/lib/httpadapter/fetchadapter.js:58:15
    at tryCatcher (/home/stuff/platform/node_modules/bluebird/js/release/util.js:16:23)

This seems to be the same as https://github.com/node-fetch/node-fetch/issues/533. The cause is this block in https://github.com/nchaulet/node-geocoder/blob/master/lib/httpadapter/fetchadapter.js:

        try {
          return await res.json();
        } catch (e) {
          throw new HttpError(await res.text(), {
            code: res.statusCode
          });
        }

Because the response is a stream, calling res.json() and then res.text() is not valid. The recommended solution seems to be either:

Ephigenia commented 1 year ago

@martinpfannemueller your fix is not visible here? maybe you can check with my fix: https://github.com/nchaulet/node-geocoder/pull/346 and vote here so that it gets merged?