-
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
XHR fails with "Invalid response for blob" on Android #18440
Comments
I ran the following code and cannot repro the issue: const res = await fetch('https://crypto-viewer.rebeleo.com/updates/en-US/android');
const data = await res.json();
console.log(data); I get the following from [{"lang":"en","date":1516298651109,"IOs":true,"android":true,"title":"Release 66","messages":["Finally I have a way to let you know what I'm working on!","Added Kukoin, Cryptopia and Bitso. If you want a specific exchange write to me.","Added gain for exchanges which support history.","Thinking about dropping \"Market\" entirely, if you use it please let me know.","Donations address can be copied on click."]},{"lang":"en","date":1516469344176,"IOs":true,"android":true,"title":"Release 68","messages":["Update available on the store, go grab it!","Since you are there maybe you can help and leave a 5 star review, it would be very kind","-> Added Kraken, many of you have asked for it","-> Added an option to use exchanges prices instead of coinmarketcap prices"]},{"lang":"en","date":1516748562864,"IOs":true,"android":true,"title":"Release 73","messages":["Update available on the store (now or soon), go grab it!","I will not bore you with small bugfix, but there's a lot every time","-> Instant price update (optional)","-> Split options and API","-> More decimals on small numbers"]},{"lang":"en","date":1517243564828,"IOs":true,"android":true,"title":"Release 78","messages":["Update available on the store (now or soon), go grab it!","-> Added Coinbase (Please check what the difference between coinbase and Gdax is, maybe Gdax is better for you)","-> Changed Binance sync to also show amounts used for orders not filled","-> Improved gain calculation for multiple pairs on the same coin holding","-> Evaluate history completion for transactions","-> Added AUD, CHF, ETH and other currencies","In progress:","-> Poloniex history is way harder to get than it seems (you can only ask for a pair per time). I talked with a few other developers and they all have a way to \"guess\" what to ask but no tecnique would work 100%. I need a few changes in the app structure to run so many requests."]},{"lang":"en","date":1518279586343,"IOs":true,"android":true,"title":"Release 91","messages":["Update available on the store (now or soon), go grab it!","-> Created a Telegram channel called \"Crypto Viewer App\", I can't add links in the updates text but I'll add a link later on in the app :)","-> Added DSX and QuadrigaCX","-> Added Ghanaian cedi","-> More coin images. If you miss one, just let me know","-> Big changes on the backend to make it more reliable (multiple servers etc).","-> Binance History. Please let me know if it looks right to you, Binance is quite different from the other exchanges.","-> Way more news sources."]},{"lang":"en","date":1519633296957,"IOs":true,"android":true,"title":"Release 120","messages":["I write here once in a while, for a detailed daily update join on Telegram","-> We are over 20 thousands downloads!","-> Moved all the data to secure storage (it was safe before too but just in case...)","-> Added Yobit and Bleutrade to Wallet","-> Added Gdax History","-> Added few more currencies (ZAR, CAD, etc)","-> Calculate gain on multiple pairs (USDT, BTC, ETH) at the same time","-> Reduce install size from 10MB to 6MB","-> Bugfixes"]}] I'll be happy to look if you can provide a repo where I can repro the bug. |
Sure thing, I will create it soon |
I ran into this as well. I'm controlling a roku where it appears to respond with a blob url with an empty string response: The request went through successfully (and sent the right command to my roku) but is failing on the response (which I don't care about). Unfortunately, since this exception is thrown in a callback there is no way for me to catch it / ignore it. |
Can you please post a sample which reproduces the issue? It's impossible for me to debug and fix it without a repro. |
Having the same issue - when the response from the server is an empty string it throws. But I don't think this should happen, as maybe you care just about the headers in that response. |
Posting that "it happens for me" doesn't help. Please post a repro. |
I agree 100%, but not sure how to do that, as my use case is posting to a private IPFS node which returns an empty string and hash info in the header. Couldn't find any public writeable IPFS nodes that work, sorry. |
Here is a repro: https://github.com/TheSavior/react-native-blob-repro The relevant lines are the server:
and the react-native app:
|
Any more thoughts on this @satya164? I’m thinking that since the error is due to an invalid response from the server and not due to user error this should just return null instead of throw. Do you agree? |
wait, my issue was with a request with actual content not an empty one, I need to make this repo (I was away with almost no internet) |
Your best bet is probably to fork my repro and change the server response. It is likely the same problem though, some content that the server says is a binary response but the content isn't parseable as a Blob. There is nothing special about the empty string. The problem is that the client throws on invalid server responses but there is no way to handle it. |
@TheSavior if you still have it there ready would you mind trying with the url I posted in the bug? |
I can't spend the time on that right now, sorry. I also don't have that repo on this computer so it is no better than you forking it and doing it yourself. 👍 |
@TheSavior the project you provided uses CRNA which uses an older version of React Native. are you sure you have the same issue with the latest version of React Native? |
Crna was just released with the latest release with blob support. Even if it isn’t the actual latest, nothing has changed here with in the blob code and the throws still exists, right? |
Same error in Android app using RN 0.55.1 |
CRNA is always behind one release. Unless I repro with latest React Native, I won't be able to debug it. Anyway, I created a new project with
Can you post your code please? |
This error occurs in the production build of our app (reported by our crash analysis tool), but NOT the debug build. This seems very odd. I tested with the XMLHttpRequest fix in our production build and it no longer crashes. In this case I can't provide a demo. Experimentally the fix does work as intended. |
@satya164 I'm still getting that error on RN 0.55.3, it is triggered by BugSnag I think . |
@hmenzagh I still don't have a repro. I cannot help without a repro. |
@satya164 I understand that you need a repro, but not everything is easily reproducible. In this case one of the triggers is BugSnag (which we're using), so the repro would be to sign up for BugSnag, integrate their library and build a production release. I've validated that the XMLHttpRequest fix works. It sounds like you don't trust us, which quite frankly is insulting. We're professionals who have been developing and architecting for decades. |
Regardless of a repro, we shouldn’t be throwing there because it isn’t an error caused by the developer and there is no way for it to be handled by the developer. A bad server response will crash the app. Returning null seems to make the most sense as that is what is done for other issues that are from the server response. Do you agree or disagree with this approach? |
Take it personally if you will. But as a professional, you should know better than treating causes than fixing the symptoms.
It's likely that the error happens because of some incorrect code on the native side. In iOS, there was some incorrect handling of response which was found because of this error. Suppressing the error doesn't fix the issue and might also hide other bugs in the native code. Also, this error doesn't happen on iOS with the same exact JS code, which means that the bug is most likely in the native code, not in JS. Anyway, this code path shouldn't be called unless there's a bug in native side, which I want to fix.
Don't know what issues you're referring to, but it should behave the same as browser. If server gives a bad response, browser will trigger the error handler and we should do the same instead of ignoring any errors. It's probably a bug in the code than bad server response. |
@obsidianart sorry to hear that, maybe SSL issues are not the only thing Android doesn't handle well from a server? If you try to use a different server within your app, does it change anything or do you stil have empty blob responses? |
@psegalen the response is not empty. can you try with my url (first message of this thread) if you have the same problem? |
@obsidianart I got an answer from your server, here is my attempt to repro I spoked about earlier, I've modified it to fetch data from your server and it seems to be ok on my Android phone: https://github.com/psegalen/rn-android-blob-failure |
@psegalen No, the issue with CodePush is that I have my wifi/data turned off (as a part of testing my app's offline mode), so it's not able to connect to the server and gets an empty response. I believe it's asking for a binary file so that might explain the 'blob' part. Has nothing to do with SSL certificates in my case. Regardless, the code should be able to handle an empty/invalid response without crashing the app. At least with the case of CodePush, the request lifecycle is not something I have control over (ie the fetch code is so deeply buried in 3rd party code), so I can't put everything into a try/catch to ignore the error. |
I don't have the issue on 0.55 anymore. Not sure if I should close this (my problem is solved) or keep it open (other people have a related issue) |
We're using CodePush as well and this is a critical bug preventing us from deploying with 0.55. I've lobbied for the JS fix which improves the robustness of blob handling. Until that's merged, use a postinstall script to patch it as so in the package.json:
And the
PS: some folks asked about how to resolve this so posted here to help others. |
@obsidianart The code that causes this error to be thrown still exists in the latest here. It's caused by an empty string response being returned which fails that blob check because it is neither an object, nor truthy. @psegalen I just pulled down your repro and besides a yellow warning box it seems to be fine (no red screen). I assume because we aren't properly receiving the I for sure get this bug with my flavor and configuration of CodePush, however it's difficult to set up an isolated project that causes the bug. |
@CapitanRedBeard ahem, no, the empty blob response is #18190 , this ticket was about something else and the response has NEVER been empty for me. can you check the other 2 tickets linked in my original post and see if you think we need this one? |
@obsidianart Yeah my bad I missed that, got derailed with the CodePush empty string conversation. #18190 is closer to my ( and I think a few others in this thread) issue but it was closed out by @satya164 with the reason being that #18223 should land a fix soon, although it doesn't actually fix the issue. So this seems like the best place to talk about this issue. The status of this is we need an isolated repro and no one (including me) can seem to figure out how to put one together reliably. I'm going to keep trying to come up with one as this is pretty critical for my project, but I would love to keep this thread open until we have something concrete to go off of. That or we can reopen #18190 since #18223 doesn't actually solve it. Which would you prefer @satya164. |
The bug on latest android 0.55.4 .. @CapitanRedBeard . For me , non server connection is a test also. Empty respond also need to check but since the red page appear first.. It is bug.Long time ago i complain, can we get real status XHR like 400,404,200. |
The fix for #18223 (iOS) landed in f5207ba and as far as I can tell, it is not in 0.55.4. You can tell because the commit is not tagged as such. If this were in 0.55.4, you would see a For anyone who claims this is still an issue on Android, can you make sure you are building React Native from source? That's the best way to ensure you are actually using a release with Janic's fix. |
@idcmsbeta the fix for #18223 is in the |
@hramos It seems the fix has actually been included in |
@wadim that fixes it on iOS - but the issue remains on |
This appears to be the last remaining open issue on this bug as it relates to Android. Can we ressurect it? |
I'm currently getting this issue when the connection drops but, again, it's hard to create an example :( |
Guess I’ll bump this again. This issue is still on all versions for Android. iOS fixed but Android not so much. |
@stueynet did you try 0.56.0 on android? Is it still happening in 0.56.0 version? |
@shashank19909 with 0.56.0 on android I am no longer getting "Invalid response on blob", however I am getting a "Network request failed" red screen of death when hitting an intentionally offline API endpoint. I would expect to be able to catch that error in my code. |
Is anybody interested in supplying a fix? While it's useful to add any information here that can help people troubleshoot the source of the problem, let's keep in mind that this is a issue tracker and not a support forum. The best way to resolve this is to send a pull request with a fix, and work with other collaborators to get it merged. |
Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions. |
Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to create a new issue with up-to-date information. |
React crash on fetch, Android only
Similar to #18223 (but this is only ios)
Similar to #18190 (but my response is not empty and the response code is 200)
Environment
Environment:
OS: macOS High Sierra 10.13.3
Node: 9.6.1
Yarn: 1.5.0
npm: 5.6.0
Watchman: 4.9.0
Xcode: Xcode 9.2 Build version 9C40b
Android Studio: 3.0 AI-171.4443003
Packages: (wanted => installed)
react: 16.2.0 => 16.2.0
react-native: 0.54.0 => 0.54.0
Steps to Reproduce
that's enough to crash on Android (it works all right on ios)
Emulated on google Pixel android 27
fetch(
https://crypto-viewer.rebeleo.com/updates/en-US/android
).then(res => res.json())
.then(data => ({ data }))
.catch(err => {
alert('Something went wrong with the Updatesfeed :( => ' + err)
return { err }
})
Expected Behavior
Expect the type of the response to be seen as json
Actual Behavior
This is a regression issue.
The response is seen as empty, and as blob
// Is going in this branch
case 'blob':
if (typeof this._response === 'object' && this._response) {
this._cachedResponse = BlobManager.createFromOptions(this._response);
} else {
throw new Error(
Invalid response for blob: ${this._response}
);}
break;
The text was updated successfully, but these errors were encountered: