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

Android crash when network changes from Good to Bad #10423

Closed
armands-malejevs opened this issue Oct 17, 2016 · 29 comments
Closed

Android crash when network changes from Good to Bad #10423

armands-malejevs opened this issue Oct 17, 2016 · 29 comments
Labels
Platform: Android Android applications. Ran Commands One of our bots successfully processed a command. Resolution: Fixed A PR that fixes this issue has been merged. Stale There has been a lack of activity on this issue and it may be closed soon.

Comments

@armands-malejevs
Copy link

armands-malejevs commented Oct 17, 2016

Issue Description

When there are fetch requests made on Android and the network changes from a good connection to a very bad connection, sometimes a crash occurs in RealBufferedSink.java:39.

Steps to Reproduce / Code Snippets

  1. Make constant fetch requests (Once every couple of seconds)
  2. Switch between a good network connection and a very bad one (if an iPhone is available it can be used as a hotspot for the Android device and can switch between LTE and very Bad Network)

Switching between the networks should reproduce the problem fairly quickly.

Expected Results

The error:

java.lang.IllegalStateException: closed
at okio.RealBufferedSink.write(RealBufferedSink.java:39)
at okio.ForwardingSink.write(ForwardingSink.java:35)
at com.facebook.react.modules.network.ProgressRequestBody$1.write(ProgressRequestBody.java:58)
at okio.RealBufferedSink.flush(RealBufferedSink.java:216)
at com.facebook.react.modules.network.ProgressRequestBody.writeTo(ProgressRequestBody.java:48)
at okhttp3.internal.http.CallServerInterceptor.intercept(CallServerInterceptor.java:47)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:92)
at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.java:45)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:92)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:67)
at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.java:109)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:92)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:67)
at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.java:93)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:92)
at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.java:124)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:92)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:67)
at okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:170)
at okhttp3.RealCall.access$100(RealCall.java:33)
at okhttp3.RealCall$AsyncCall.execute(RealCall.java:120)
at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1113)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:588)
at java.lang.Thread.run(Thread.java:818)

Additional Information

  • React Native version: 0.41.0 (All previous version also have this issue)
  • Platform(s) (iOS, Android, or both?): Android
  • Operating System (macOS, Linux, or Windows?): macOS
@saleehk
Copy link
Contributor

saleehk commented Nov 4, 2016

@armadilio3 Same issue here. did you got that fixed .?

@armands-malejevs
Copy link
Author

@saleeh93 still looking into it, maybe you could describe how you can replicate the crash since I am working on a fairly large project and I am only beginning to pinpoint the exact place of origin for this bug

@saleehk
Copy link
Contributor

saleehk commented Nov 7, 2016

@armadilio3 I am also working on large project and I never had that issue when I am testing it, I got those errors from Crashlytics. I didn't even understand when that is coming up

@saleehk
Copy link
Contributor

saleehk commented Dec 9, 2016

@armadilio3 Any updates ?

@LucasSouzaa
Copy link

Same issue here :(

@saleehk
Copy link
Contributor

saleehk commented Feb 10, 2017

@lucasfeliciano Still waiting to get this fixed

@armands-malejevs
Copy link
Author

So, I have found a temporary solution to this problem which is based on catching the error. After some testing it seems that if you catch the OkHttp error the network request will fail but the app won't crash. So if anyone is interested here is the temporary solution:

Edit your MainApplication.java

    @Override
    public void onCreate() {
        super.onCreate();

        // Setup handler for uncaught exceptions.
        Thread.setDefaultUncaughtExceptionHandler (new Thread.UncaughtExceptionHandler()
        {
        @Override
        public void uncaughtException (Thread thread, Throwable e)
        {
            handleUncaughtException (thread, e);
        }
        });
    }

   public void handleUncaughtException (Thread thread, Throwable e)
    {

        if(e.getMessage() != "closed" || thread.getName() != "OkHttp Dispatcher"){
            //Kill the app
            System.exit(1);
        }else{
            //If the OkHttp error occurs we ignore it
            Log.e("OkHttp Exception","Received exception " + e.getMessage() + "From thread " + thread.getName());
        }
       
    }

When I have the time I will look into the root cause of this issue as this is a very poor fix. If I find a solution, will make PR.

@saleehk
Copy link
Contributor

saleehk commented Mar 11, 2017

@armadilio3 You are tricky 😉

@yunlongz
Copy link

@armadilio3 I use yours,but It`s not running.

@LucasSouzaa
Copy link

@yunlongz are you building from source?
I use this "solution" and work fine.

@abrantes01
Copy link

abrantes01 commented Apr 18, 2017

@armadilio3's solution works fine for me. But does anyone have any news on this problem?

@Tekosawa
Copy link

Tekosawa commented Jul 7, 2017

Anyone found a real fix ? Problem still exist.

@loiclouvet
Copy link

I tried @armadilio3 issue but I still have same crashes (maybe because I use react-native-fabric).

@arryanggaputra
Copy link

I got the same issue, and @armadilio3 solutions is better...

@stale
Copy link

stale bot commented Nov 26, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Nov 26, 2017
@peter-mach
Copy link

It's not stale it's real and happens in my app too as @armadilio3 described it.

@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Nov 29, 2017
@peter-mach
Copy link

peter-mach commented Nov 29, 2017

seems related to this bug in okhttp: square/okhttp#3521

That was fixed in okhttp v3.9 but react-native is using v3.6. Can we update the dependency?

@wobenzym
Copy link

@pmachowski We have the same issue seems like there is a solution in #11016 in the last post. We will test it tomorrow. Is there any finite solution to this problem?

@derakhshanfar
Copy link

Same issue for me.. any solution?

@derakhshanfar
Copy link

@Pavle93 could you find a solution?

@DevilSoulcz
Copy link

@ pavle93 the same problem.... could you find a solution?

facebook-github-bot pushed a commit that referenced this issue Feb 3, 2018
…h on closed connection

Summary:
This PR includes the same changes made in #16541, for addressing issues #11853/#15724. It adds upload progress updates for uploads with any request body type, and not just form-data.

Additionally, this PR also includes a commit for fixing an `IllegalStateException` when a user's connection gets closed or times out (issues #10423/#11016). Since this exception was occurring within the progress updates logic, it started being thrown more frequently as a result of adding progress updates to all uploads, which was why the original PR was reverted.

To test the upload progress updates, run the following JS to ensure events are now being dispatched:
```
const fileUri = 'file:///my_file.dat';
const url = 'http://my_post_url.com/';
const xhr = new XMLHttpRequest();

xhr.upload.onprogress = (event) => {
    console.log('progress: ' + event.loaded + ' / ' + event.total);
}

xhr.onreadystatechange = () => {if (xhr.readyState === 4) console.log('done');}

console.log('start');

xhr.open('POST', url);

// sending a file (wasn't sending progress)
xhr.setRequestHeader('Content-Type', 'image/jpeg');
xhr.send({ uri: fileUri });

// sending a string (wasn't sending progress)
xhr.setRequestHeader('Content-Type', 'text/plain');
xhr.send("some big string");

// sending form data (was already working)
xhr.setRequestHeader('Content-Type', 'multipart/form-data');
const formData = new FormData(); formData.append('test', 'data');
xhr.send(formData);
```

To test the crash fix:
In the RN Android project, before this change, set a breakpoint at `mRequestBody.writeTo(mBufferedSink);` of `ProgressRequestBody`, and wait a short while for a POST request with a non-null body to time out before resuming the app. Once resumed, if the connection was closed (the `closed` variable will be set to true in `RealBufferedSink`), an `IllegalStateException` will be thrown, which crashes the app. After the changes, an `IOException` will get thrown instead, which is already being properly handled.

As mentioned above, includes the same changes as #16541, with an additional commit.

[ANDROID] [BUGFIX] [XMLHttpRequest] - Added progress updates for all XMLHttpRequest upload types / fix crash on closed connection

Previously, only form-data request bodies emitted upload progress updates. Now, other request body types will also emit updates. Also, Android will no longer crash on certain requests when user has a poor connection.

Addresses issues: 11853/15724/10423/11016
Closes #17312

Differential Revision: D6712377

Pulled By: mdvacca

fbshipit-source-id: bf5adc774703e7e66f7f16707600116f67201425
Plo4ox pushed a commit to Plo4ox/react-native that referenced this issue Feb 17, 2018
…h on closed connection

Summary:
This PR includes the same changes made in facebook#16541, for addressing issues facebook#11853/facebook#15724. It adds upload progress updates for uploads with any request body type, and not just form-data.

Additionally, this PR also includes a commit for fixing an `IllegalStateException` when a user's connection gets closed or times out (issues facebook#10423/facebook#11016). Since this exception was occurring within the progress updates logic, it started being thrown more frequently as a result of adding progress updates to all uploads, which was why the original PR was reverted.

To test the upload progress updates, run the following JS to ensure events are now being dispatched:
```
const fileUri = 'file:///my_file.dat';
const url = 'http://my_post_url.com/';
const xhr = new XMLHttpRequest();

xhr.upload.onprogress = (event) => {
    console.log('progress: ' + event.loaded + ' / ' + event.total);
}

xhr.onreadystatechange = () => {if (xhr.readyState === 4) console.log('done');}

console.log('start');

xhr.open('POST', url);

// sending a file (wasn't sending progress)
xhr.setRequestHeader('Content-Type', 'image/jpeg');
xhr.send({ uri: fileUri });

// sending a string (wasn't sending progress)
xhr.setRequestHeader('Content-Type', 'text/plain');
xhr.send("some big string");

// sending form data (was already working)
xhr.setRequestHeader('Content-Type', 'multipart/form-data');
const formData = new FormData(); formData.append('test', 'data');
xhr.send(formData);
```

To test the crash fix:
In the RN Android project, before this change, set a breakpoint at `mRequestBody.writeTo(mBufferedSink);` of `ProgressRequestBody`, and wait a short while for a POST request with a non-null body to time out before resuming the app. Once resumed, if the connection was closed (the `closed` variable will be set to true in `RealBufferedSink`), an `IllegalStateException` will be thrown, which crashes the app. After the changes, an `IOException` will get thrown instead, which is already being properly handled.

As mentioned above, includes the same changes as facebook#16541, with an additional commit.

[ANDROID] [BUGFIX] [XMLHttpRequest] - Added progress updates for all XMLHttpRequest upload types / fix crash on closed connection

Previously, only form-data request bodies emitted upload progress updates. Now, other request body types will also emit updates. Also, Android will no longer crash on certain requests when user has a poor connection.

Addresses issues: 11853/15724/10423/11016
Closes facebook#17312

Differential Revision: D6712377

Pulled By: mdvacca

fbshipit-source-id: bf5adc774703e7e66f7f16707600116f67201425
@nishantonline1
Copy link

@armadilio3's solution works fine for me. but there is an error in it, we have to import android.util.log in MainApplication.java.

import android.util.Log;
@pmachowski Problem still in version 3.9.0

@react-native-bot react-native-bot added Android Ran Commands One of our bots successfully processed a command. labels Mar 18, 2018
@react-native-bot react-native-bot added Platform: Android Android applications. Ran Commands One of our bots successfully processed a command. labels Mar 18, 2018
@workostap
Copy link

Worked for me by adding this line to android/app/build.gradle
dependencies {
...
compile 'com.squareup.okhttp3:okhttp:3.9.0'
}

@nishantonline1
Copy link

@workostap that works for me too but the problem still persist sometimes.

@urieluvd
Copy link

To use the latest okhttp you should also upgrade your react native version.

You can also solve it by using
com.squareup.okhttp3:okhttp:3.5.0 or com.squareup.okhttp3:okhttp:3.6.0
with com.facebook.react:react-native:0.48.X

trying to combine higher versions with lower of each library will lead you to this error
Undefined is not an object (evaluating 'Sn[e]')

@stale
Copy link

stale bot commented Aug 14, 2018

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.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Aug 14, 2018
@hramos hramos added the Resolution: PR Submitted A pull request with a fix has been provided. label Aug 23, 2018
@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Aug 23, 2018
facebook-github-bot pushed a commit that referenced this issue Aug 23, 2018
Summary:
This change fixes this issue:
#10423
Pull Request resolved: #20802

Differential Revision: D9478422

Pulled By: hramos

fbshipit-source-id: ce3a098a71c8f50a62b011a2a277004ab7a7b655
@kelset
Copy link
Contributor

kelset commented Aug 23, 2018

(quick update, we merged a fix for it and we'll probably cherry pick for 0.57-rc3)

@kelset kelset added Resolution: Fixed A PR that fixes this issue has been merged. and removed Resolution: PR Submitted A pull request with a fix has been provided. labels Aug 23, 2018
kelset pushed a commit that referenced this issue Aug 23, 2018
Summary:
This change fixes this issue:
#10423
Pull Request resolved: #20802

Differential Revision: D9478422

Pulled By: hramos

fbshipit-source-id: ce3a098a71c8f50a62b011a2a277004ab7a7b655
aleclarson pushed a commit to aleclarson/react-native that referenced this issue Sep 16, 2018
Summary:
This change fixes this issue:
facebook#10423
Pull Request resolved: facebook#20802

Differential Revision: D9478422

Pulled By: hramos

fbshipit-source-id: ce3a098a71c8f50a62b011a2a277004ab7a7b655
@stale
Copy link

stale bot commented Nov 22, 2018

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.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Nov 22, 2018
@stale
Copy link

stale bot commented Nov 29, 2018

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.

@stale stale bot closed this as completed Nov 29, 2018
@facebook facebook locked as resolved and limited conversation to collaborators Nov 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Platform: Android Android applications. Ran Commands One of our bots successfully processed a command. Resolution: Fixed A PR that fixes this issue has been merged. Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
Development

No branches or pull requests