React-native: XHR fails with "Invalid response for blob" on Android

Created on 19 Mar 2018  路  79Comments  路  Source: facebook/react-native


React crash on fetch, Android only
Similar to https://github.com/facebook/react-native/issues/18223 (but this is only ios)
Similar to https://github.com/facebook/react-native/issues/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

screenshot_1521418646

// 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;

Android Ran Commands Stale 馃寪Networking

Most helpful comment

@satya164

Here is a repro:

https://github.com/TheSavior/react-native-blob-repro

The relevant lines are the server:

app.post('/', (req, res) => res.end('', 'binary'));

and the react-native app:

fetch('http://localhost:8798', {method: 'POST'});

All 79 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 console.log

[{"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:

screen shot 2018-04-01 at 1 31 43 am

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.

@satya164

Here is a repro:

https://github.com/TheSavior/react-native-blob-repro

The relevant lines are the server:

app.post('/', (req, res) => res.end('', 'binary'));

and the react-native app:

fetch('http://localhost:8798', {method: 'POST'});

Any more thoughts on this @satya164? I鈥檓 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鈥檛 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 was just released with the latest release with blob support.

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 react-native init (RN 0.55.2), copied your code and I don't get an error.

Same error in Android app using RN 0.55.1

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鈥檛 be throwing there because it isn鈥檛 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?

peacechen: It sounds like you don't trust us, which quite frankly is insulting

Take it personally if you will. But as a professional, you should know better than treating causes than fixing the symptoms.

we shouldn鈥檛 be throwing there because it isn鈥檛 an error caused by the developer

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.

Returning null seems to make the most sense as that is what is done for other issues that are from the server response

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.

The native side should be fixed, but regardless of the cause, the JS side shouldn't be throwing an unhandled exception because of an empty body blob. That is easily checked for and prevents breakage in the future as well. Sounding like a broken record...

@satya164, I didn't realize the issue you are trying to track down is a native one.

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.

The difference with the JS code we have as is, is that it doesn't trigger the error handler. There is literally know way to catch this exception. There is nothing a user can do to catch and handle this exception gracefully. You can't wrap something in a try/catch, you can't use unhandled exception handlers, etc. Since this exception is thrown from an internal callback, there is literally no way to handle this exception from product code.

There is nothing a user can do to catch and handle this exception gracefully

The error is thrown in the response getter, so it'll be thrown when accessing xhr.response property, and the user should be able to catch it if they are using the XMLHttpRequest API. It's not an internal callback, but it seems that it's not catchable only if using fetch.

Also, there are many places in RN which throw a non-catchable exception. They are mostly due to programmer errors, i.e. bug in code. This is not a special case.

Anyway, this path shouldn't be triggered at all if the code is alright. It doesn't on happen iOS, so I assume it's the Java code. That's why I'm looking for a repro so I can fix the bug.

The proper way would be to let the user handle the error via the error handler of XMLHttpRequest rather than suppressing it entirely. But in this case, it seems it's not actually an error and just a bug. So let's focus on finding the root cause please.

@satya164 I just tried to reproduce the error and got nothing
here it is, maybe it can save someone some time : link

tell me if I can provide you with any further help or informations

(the api key provided in the repo for bugsnag is a "junk" one, don't worry)

iOS also triggers this exception (issue posted in the OP). Both platforms crash with RN 0.55, and after a month there's still push back when a fix has been identified.

I have the exact same stack trace that @obsidianart posted on Android when triggering a mutation in react-apollo, the mutation fails but it's expected since it's an attempt to log in a user and I'm testing bad credentials behaviour. Failure sends back a 403 HTTP status but it's supposed to be handled on my side.
NB: it still has the good behaviour on iOS, it seems to be an Android-only bug!

After further debugging and investigating, Android native code is really sending empty response to JS when responseType is "blob".
I'm not an expert but it seems to me that the code preceding this line is incomplete:

              // Otherwise send the data in one big chunk, in the format that JS requested.
              String responseString = "";
              if (responseType.equals("text")) {
                try {
                  responseString = responseBody.string();
                } catch (IOException e) {
                  if (response.request().method().equalsIgnoreCase("HEAD")) {
                    // The request is an `HEAD` and the body is empty,
                    // the OkHttp will produce an exception.
                    // Ignore the exception to not invalidate the request in the
                    // Javascript layer.
                    // Introduced to fix issue #7463.
                  } else {
                    ResponseUtil.onRequestError(eventEmitter, requestId, e.getMessage(), e);
                  }
                }
              } else if (responseType.equals("base64")) {
                responseString = Base64.encodeToString(responseBody.bytes(), Base64.NO_WRAP);
              }
              ResponseUtil.onDataReceived(eventEmitter, requestId, responseString);
              ResponseUtil.onRequestSuccess(eventEmitter, requestId);

It seems to be populating responseString only if responseType is "text" or "base64", in our case (if I correctly identified the native code) "blob" being neither, the response sent is effectively empty!

@satya164 I'd submit a PR myself but I don't want to break anything, what do you think about this section of code?

Maybe just extending the first if to
if (responseType.equals("text") || responseType.equals("blob")) {
would do the trick?

Can you send a PR with a test plan?

@satya164 I'll try to do it correctly, I'll link this issue.

Just for the sake of reporting (I'm not making a repro for this), I get the same error while running the debug build of my app (RN 0.54.4) on a real Android device (while connected to my Mac on USB) that happens to be out of network connection, such as being in airplane mode. The redbox apparently appears while trying to fetch http://localhost:8081/symbolicate (not sure), so there's nothing I can do to stop it.

If anyone wants a quick and dirty fix until this is sorted out, add this in node_modules/react-native/Libraries/Network/XMLHttpRequest.js (you can fork the repo, add this change and use that version in package.json, pls don't edit node_modules directly) 馃槂

    case 'blob':
        if (this._response.toString() === '') {
          this._cachedResponse = new Blob([]);
          break;
        }

        ...

        break;

@petarjs unfortunately, I want the blob returned by the server :'(
I'm working on a fix currently, hopefully I'll do a PR soon.

There is a well tested fix at
https://github.com/xiamx/react-native/commit/b35071b5daabf370e7789e00e1593788fcf5aecf

@petarjs Forking the repo doesn't work because RN has native code that needs to be built. FB does this during the release process and includes those binary bits in the npm release.

FWIW I'm still getting this on on 55.3 for Android, but only if I have my WiFi disconnected. I'm also using bugsnag and it could potentially be related to the blob being null or something/network request failing. If I am connected to WiFi it seems to work. This issue is also being tracked here: https://github.com/react-native-community/react-native-releases/issues/11

A crashing React Native app just because HTTP requests fail due to no wifi/reception is just unacceptable. Why is this still a problem when there's a clear solution?

I have to downgrade all the way to 0.53.3 just to precede this bug.

@tmaly1980: Why is this still a problem when there's a clear solution?

Because nobody has provided a way to reproduce this bug and there's no way to test whatever fix we do actually works.

@satya164 It's actually pretty easy if you have a project that uses code-push. Just turn off your dev machine's network connection and then reload the code-push enabled app. It'll trigger that error because the fetch to query for updates fails and has no content to return.

I need an isolated repro.

@tmaly1980
Comment on and vote for the JS fix:
https://github.com/react-native-community/react-native-releases/issues/11#issuecomment-385759634

That fixes both platforms, Android and iOS.

@peacechen this fix doesn't fix anything, it masks the issue.
I use react-apollo and the "fix" only moves the crash to the next instructions in the pipeline: the lib wants to exploit the result of the request which is null... result is a network error... :(

@psegalen That's interesting. I'm not using react-apollo, but it sounds like it should be fixed to handle network error cases. After all, the network can and will fail on any given request eventually.

As @TheSavior noted, throwing an exception in this case is the worst action because it's difficult or impossible to catch the error, particularly when using a library that wraps XMLHttpRequest.

throwing an exception in this case is the worst action because it's difficult or impossible to catch the error

It is a "BUG". There shouldn't be any error. The only case that condition will be executed was if there was a bug. The user "shouldn't" need to handle this. It's not any different from other places where we throw runtime exception which crashes the app. The library doesn't know how to handle an inconsistent state and hence throws an error.

If we had silenced the error from the beginning, we'd probably have never known that there is a bug in the code, and even when we did, would've been way harder to know what part of the code has the bug. Do you see why silencing the error is not a good idea?

If temporarily silencing the error works for you, great. You can temporarily fork it while the root cause it fixed. But it's not a fix, it's an workaround.

vote for the JS fix

Seriously?

@psegalen if you can't share your code publicly, you could also DM me on twitter @satya164 if that's alright with you.

@peacechen I may have been unclear in my answer but as @satya164 said it's a very real bug, react-apollo do handle network errors but the usecase we're talking about isn't one, the server correctly handled the request and returned a correct response, it's RN Android that loses the response's body along the way... :'(

@satya164 I made several tests on my code and failed to fix the bug in RN Android code for now, I don't have enough time because of some iOS deadlines for my app.
Interestingly, I was just about to share a repro with you but the Android app of my repro is actually functional! 馃槀
I'll try to downgrade RN to my real code version to see if the error is there.

Ok so my repro is OK even with the same RN version that has the bug on my app.
It must be an interaction between another lib... 馃槶
I'll try to add them one by one to see if I can reproduce...

@psegalen maybe I can help debug this? at this point, I think I'm fine with a repro which doesn't have a reduced test case

@satya164 unfortunately for now it isn't a repro at all since it's functional, I'm working on making it a repro

Still occurring on 0.55.4, will also try and produce a small repro.

Also I can confirm with @peacechen that this fix solves the issue:
xiamx/react-native@b35071b

@satya164 I'm beginning to wonder if it's not something form the server, I'll dig deeper tomorrow, when my repro uses a GraphQL server from Microsoft everything's fine but when I try to make it query my server it fails with a Network error... 馃槙
Same test on iOS works just fine... I'll hunt for a particular header / encoding we would be using... tomorrow!

Ok, I may have nailed my problem and it may be the same for many among us: it seems to be something to do with SSL certificates on the server, when SSL isn't A-grade (you can test your server with this online tool: https://www.ssllabs.com/ssltest/), Android seems to have a weird behaviour with requests and responses!

EDIT: we temporary activated HTTP on our server and I've tried to connect my Android app to the server with HTTP and now I get responses body to my Blob requests! 馃帀
Thanks @satya164 for your help!
Next step is fixing our SSL certificates to connect the Android app with HTTPS again.

@tmaly1980 maybe CodePush server has SSL issue? If you have the host name, give the ssllabs tool a go, if the grade isn't A, it may be the source of the problem!
@hmenzagh maybe the same applies to the BugSnag server?
@obsidianart could be your problem too?

@psegalen no my server (top of this thread) is GradeA, I just checked

@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:

  "scripts": {
    "postinstall": "rn-patches/apply-patches.sh"
  },

And the apply-patches.sh script:

#!/bin/bash
# Critical patches for RN 0.55

# Fix "Invalid response for blob" with fetch https://github.com/facebook/react-native/issues/18223
curl -O https://github.com/xiamx/react-native/commit/b35071b5daabf370e7789e00e1593788fcf5aecf.patch
patch $PWD/node_modules/react-native/Libraries/Network/XMLHttpRequest.js b35071b5daabf370e7789e00e1593788fcf5aecf.patch
rm -f b35071b5daabf370e7789e00e1593788fcf5aecf.patch

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.
It's possible it's working for you because with 0.55 something changed where you are no longer getting that "" response, but this issue should remain open as it hasn't gone anywhere.

@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 "" blob.

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 https://github.com/facebook/react-native/issues/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?
Please notice on line 4 "Similar to #18190 (but my response is not empty and the response code is 200)"

@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 f5207ba9c764f33ef83fa897f6014d67193be0e2 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 v0.55.4 tag next to master in the lower left corner:

screen shot 2018-05-24 at 10 29 50 am

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.

@hramos build from source ? i only knew to use normal yarn/npm only. I see this patch https://github.com/facebook/react-native/pull/19567/commits/4edc83ea4fe5c859ab6dfcc3c579a61f610132e4 ..And it work as intended. Any hope anybody merge as updated ?

@idcmsbeta the fix for #18223 is in the 0.56.0-rc release.

@hramos It seems the fix has actually been included in 0.55.4 via this commit https://github.com/facebook/react-native/commit/093a78d99d0

@wadim that fixes it on iOS - but the issue remains on 0.55.4 on Android, which I'm currently running.

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鈥檒l 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.

screenshot-2018-08-08_22 04 33 732

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.

Was this page helpful?
0 / 5 - 0 ratings