JakeChampion/fetch

Stop throwing errors and wrap JSON.parse with try/catch block

niftylettuce opened this issue · 13 comments

matthew-andrews commented

I believe it's correct as per the spec for fetch to throw an error if you try to get json output from a non json response… ?

niftylettuce commented

so if that were the case, how do you gracefully show the user an error
message when the error doesn't get bubbled up to the callback/promise? now
im having to write try/catch blocks everywhere.
On Nov 26, 2015 4:51 PM, "Matt Andrews" notifications@github.com wrote:

I believe it's correct as per the spec for fetch to throw an error if you
try to get json output from a non json response… ?

—
Reply to this email directly or view it on GitHub
#235 (comment).

niftylettuce commented

e.g. an endpoint suddenly returns 404 Not Found instead of normal 200 JSON
On Nov 26, 2015 4:52 PM, "Nick Baugh" niftylettuce@gmail.com wrote:

so if that were the case, how do you gracefully show the user an error
message when the error doesn't get bubbled up to the callback/promise? now
im having to write try/catch blocks everywhere.
On Nov 26, 2015 4:51 PM, "Matt Andrews" notifications@github.com wrote:

I believe it's correct as per the spec for fetch to throw an error if you
try to get json output from a non json response… ?

—
Reply to this email directly or view it on GitHub
#235 (comment).

niftylettuce commented

Because of this mess, here's the fix I wrote for this issue we have now... I basically have to do my own JSON.parse:

        fetch(apiUri + path, {
          method: 'GET',
          headers: that.headers
        })
        .then((res) => {
          try {
            let response = JSON.parse(res);
            if (response && response.error)
              throw new Error(response.error);
            return res.json();
          } catch (e) {
            return res.text();
          }
        })
        .then((res) => {
          fn(null, res);
        })
        .catch(function(err) {
          fn(err);
        })
  • 👍6
matthew-andrews commented

We normally check the status (res.ok is handy for that) of the response before requesting json…

.then(res => {
  if (res.ok) {
    return res.json();
  } else {
    // handle error 
  }
});

Also you could always implement the try catch yourself and do:-

.then(res => res.text())
.then(res => {
  try {
    return JSON.parse(res);
  } catch(err) {
    // handle error
  }
});
  • 👍3
matthew-andrews commented

I would highly recommend checking the status of the response either via .ok or .status.

niftylettuce commented

good point

matthew-andrews commented

I will close this issue then :)

sethfatz commented

@matthew-andrews Attempting to return response.json() after reading response.ok (or response.status) throws 'Already read' log and 'undefined is not an object' error

mislav commented

@sethfatz Make sure to return response from a handler that checks response.ok.

acomito commented

Does anyone have any "real world" examples of how they're handling errors here? Particularly in react?

.then(res => res.text())
.then(res => {
  try {
    return JSON.parse(res);
  } catch(err) {
    // handle error
    // return error and use this.state to display it?
    // show an alert?
   // alert sentry
  }
});

I'm sure it's context specific, but I'm having trouble finding good examples beyond error handling 101.

jymbob commented

Pretty sure I can share this without giving away too many trade secrets 😉

Premise: a successfuly query to our API endpoint returns a valid JSON packet
Sometimes it returns an error packet with a suitable code e.g. 409 - conflict
Sometimes it returns a 404 (e.g. record not found)
Sometimes it returns an empty file, or text, or html (it's complicated)

I definitely lifted some of it from a helpful tutorial - will cite if I can find it, but worked on the error handler a lot since then

const fetchJSON = ((url, apiKey) => {
  return fetch(url, {
    mode: 'cors',
    credentials: 'include',
    cache: 'no-store',
    headers: {
      'Bearer': apiKey,
    }
  }).then(validateResponse)
    .then(readResponseAsJSON)
    .catch(handleResponseError)
})

const validateResponse = (response => {
  if (!response.ok) {
    return Promise.reject(response)
  }
  return response
})

const readResponseAsJSON = (response => {
  return response.json()
})

const handleResponseError = (response => {
  // explicitly returning a promise, because I can't figure out the right syntax otherwise
  return new Promise((resolve, reject) => {
    const statusText = response.status + ': ' + response.statusText
    response.text()  //response.json() doesn't appear to support sensible error handling of non-JSON
      .then(text => {
        let jsonData = {}
        try {
          jsonData = JSON.parse(text)  // try to do our own parsing of the json here
        } catch(err) {
          //no-op
        }
        return jsonData
      })
      .then(packet => {
        if (packet.error) {
          return reject(packet.error)
        } else {
          if (response.status == 404) {
            return reject('404: not found')  //we treat this as a special code in a couple of places
          } else {
            return reject(statusText)
          }
        }
      })
  })
})

We use that everywhere, in the format

fetchJSON('http://example.com/json', 'mySuperSecretApiKey123')
  .then(packet => {
    //success
    console.log(packet)
  }).catch(err => {
    //failure
    console.error(err)
  })
jaychsu commented

I think there is a good real-world solution@MDN to distinct JSON-formatted response and plain text.

fetch(myRequest).then(function(response) {
    var contentType = response.headers.get("content-type");
    if(contentType && contentType.includes("application/json")) {
      return response.json();
    }
    throw new TypeError("Oops, we haven't got JSON!");
  })
  .then(function(json) { /* process your JSON further */ })
  .catch(function(error) { console.log(error); });
  • 👍2