-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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("aaa") has incorrect behavior #7436
Comments
This is tricky. The error thrown is from the native side, and due to the async nature of the bridge, it won't be possible to throw an error on JS side. I see 2 options,
|
I think we should always do 2 and sometimes do 1 as well. That is, convert all native failures to promise rejections, but also do extra validation in JS when we think we can provide better error messages closer to the source of the error. |
@cancan101 Want to a PR? |
+1 for JS URL validation, since it will ensure consistency in behavior between platforms, as well as all the other listed benefits. We'll need to keep the validation on the native side as a backup too though (iOS will crash hard for certain invalid url strings). |
I should add that on ios the promise is rejected as expected for this invalid url string. |
@lacker I think it's different. In the browser console you call |
Ah, you are correct, when I call something invalid in the browser like |
This is still happening on RN0.39.2 on Android.
|
@npomfret no one has worked on this. if you want to send a PR it'll be great. |
Closing this issue because it has been inactive for a while. If you think it should still be opened let us know why. |
Re-open it, please. |
Please re-open as this is still an issue in latest React Native. This is especially an issue when doing a Thankfully the user notice the extra space eventually but it's the kind of crash that could have taken a while to solve. |
Is anyone working on a PR? The issue was closed after five months of inactivity. Seems like a straightforward fix (the promise should be rejected), if this is something that affects you. |
I've looked at it at some point, but was looking for a JavaScript fetch function where I wanted to add checks as suggested by @satya164, but there's no such JS function as it seems it's purely native. I don't really have the skills to fix the native side, but I might take another look. |
It's not native. fetch is a polyfill over XMLHttpRequest whose API is implemented in JavaScript over the native networking module. |
…lid URL on Android Currently if you invoke `fetch()` with an invalid URL ("aaa" for example) you cannot catch the error in javascript since it's not reported. Instead the entire app crashes. Fixes facebook#7436 and facebook#18087 Hopefully.
I opened a PR that hopefully fixes this issue #18103 |
…lid URL on Android Summary: Currently if you invoke `fetch()` with an invalid URL ("aaa" for example) you cannot catch the error in javascript since it's not reported. Instead the entire app crashes. Fixes #7436 and #18087 Hopefully. <!-- Thank you for sending the PR! We appreciate you spending the time to work on these changes. Help us understand your motivation by explaining why you decided to make this change. You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html Happy contributing! --> Fix using fetch on Android with user generated input. Added relevant unit test `./scripts/run-android-local-unit-tests.sh` all pass [ANDROID] [BUGFIX] [fetch] - Allow "unexpected url" exception to be caught on Android when using fetch <!-- Help reviewers and the release process by writing your own release notes **INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.** CATEGORY [----------] TYPE [ CLI ] [-------------] LOCATION [ DOCS ] [ BREAKING ] [-------------] [ GENERAL ] [ BUGFIX ] [-{Component}-] [ INTERNAL ] [ ENHANCEMENT ] [ {File} ] [ IOS ] [ FEATURE ] [ {Directory} ] |-----------| [ ANDROID ] [ MINOR ] [ {Framework} ] - | {Message} | [----------] [-------------] [-------------] |-----------| [CATEGORY] [TYPE] [LOCATION] - MESSAGE EXAMPLES: [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see --> Closes #18103 Differential Revision: D7097110 Pulled By: hramos fbshipit-source-id: 69144e8a0f7404d9bcc7c71a94650de36a48c84a
I am having the same issue in my application. It would be a huge help if someone could provide a code for the the promise rejection with |
In my case I've ended up wrapping fetch in my own function that does the URL validation: const urlValidator = require('valid-url');
function myfetch(url, options = null) {
const validatedUrl = urlValidator.isUri(url);
if (!validatedUrl) throw new Error('Not a valid URL: ' + url);
return fetch(validatedUrl, options);
} |
@anurag1018 The issue has been fixed in #18103 we just need to wait for it to be released. |
Unable to catch error thrown from
fetch
with invalid url. Instead I get a message about "unexpected url".0.24.1
Android
OS X
The text was updated successfully, but these errors were encountered: