Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetch needs patched for try/catch on JSON.parse #4376

Closed
niftylettuce opened this issue Nov 26, 2015 · 6 comments
Closed

fetch needs patched for try/catch on JSON.parse #4376

niftylettuce opened this issue Nov 26, 2015 · 6 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@niftylettuce
Copy link
Contributor

Please fix

return this.text().then(JSON.parse)
to have try/catch surrounding JSON.parse?

@niftylettuce niftylettuce changed the title node-fetch needs upgraded once patched fetch needs patched for try/catch on JSON.parse Nov 26, 2015
@niftylettuce
Copy link
Contributor Author

also why are you throwing errors instead of sending them back with Promise.reject?

return Promise.reject(new TypeError('Already read'))
uses reject but this doesn't
throw new Error('could not read FormData body as blob')

@niftylettuce
Copy link
Contributor Author

Basically because you are throwing errors and not wrapping JSON.parse in a try/catch block - I had to dig deep to find out that my 404 response could not use res.json() in this example:

        fetch(apiUri + path, {
          method: 'GET',
          headers: that.headers
        })
        .then((res) => res.json())
        .then((res) => {
          if (res && res.error)
            return fn(res.error);
          fn(null, res);
        })
        .catch(fn)

The error is thrown that it can't parse JSON (the body is text response)

@niftylettuce
Copy link
Contributor Author

The reason I have to use fetch polyfill is because superagent doesn't work (because it's trying to run in a node runtime environment and not a browser environment or something). This is broken and we can't even hook in a simple API into react-native...

@satya164
Copy link
Contributor

if an error is thrown inside the .then callback, isn't it supposed to be propagated to the .catch callback?

@facebook-github-bot
Copy link
Contributor

Hey niftylettuce, thanks for reporting this issue!

React Native, as you've probably heard, is getting really popular and truth is we're getting a bit overwhelmed by the activity surrounding it. There are just too many issues for us to manage properly.

  • If this is a feature request or a bug that you would like to be fixed by the team, please report it on Product Pains. It has a ranking feature that lets us focus on the most important issues the community is experiencing.
  • If you don't know how to do something or not sure whether some behavior is expected or a bug, please ask on StackOverflow with the tag react-native or for more real time interactions, ask on Discord in the #react-native channel.
  • We welcome clear issues and PRs that are ready for in-depth discussion; thank you for your contributions!

@brentvatne
Copy link
Collaborator

Closing as per discussion in JakeChampion/fetch#235

@facebook facebook locked as resolved and limited conversation to collaborators Jul 20, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

5 participants