[Android] Update OkHttp to 3.4.1 and Okio to 1.9.0

[AndrewJack]

Motivation

• 3.3.1 wasn't compatible with Robolectric 3.0, however 3.4.0 fixes this issue.
• Few other fixes to OkHttp mentioned in the CHANGELOG.md

Motivation from #7746

Fixes: #7743

• Android apps can recover from a REFUSED_STREAM in HTTP/2.
• A few other fixes mentioned in the CHANGELOG.md

Test plan

• CircleCi
• Test with /Examples ✅

[ghost]

By analyzing the blame information on this pull request, we identified @cjhopman and @mkonicek to be potential reviewers.

[AndrewJack]

cc @mkonicek & @bestander

Upgrading to 3.4.0 fixes the issue we had with 3.3.1

[AndrewJack]

Following issues should be resolved by this PR:

• Network Request Failure Android on SSL only #8659
• OkHttp Unable to extract the trust manager #8322
• [iOS and Android] Network request failed when HTTP/2 enabled. #8027
• Fetch fails intermittently on first request with 'POST' method when using nginx 1.10.x #7943

[ide]

This is really awesome and great to see the tests are passing. @bestander can you help land this in case the shipit bot isn't enough?

[ide]

@facebook-github-bot shipit

[ghost]

Something went wrong when importing this pull request. Please cc someone from the team at fb to help with importing this.

[bestander]

Great work, Andrew.
I'll need to update internal okhttp as well.

[bestander]

@facebook-github-bot import

[ghost]

Thanks for importing. If you are an FB employee go to Phabricator to review.

[AndrewJack]

https://github.com/square/okhttp/blob/master/CHANGELOG.md#version-341

Will need to update OkHttp to 3.4.1 due to a major bug in 3.4.0

[AndrewJack]

Created PR for 3.4.1 #8680 (includes fix for OkHttp encoding HTTP headers bug).

I'll update #8680 once this has been imported.

[bestander]

@AndrewJack, what is the point of importing 3.4.0 if we can do 3.4.1?

[bestander]

I mean it is easier to do in one go then multiple times.
Also what do you think, should we wait a week to see if there are any more critical issues found in 3.4.1?

[AndrewJack]

@bestander lets just do #8680, I thought you had already imported this one.

[AndrewJack]

@bestander We already have a few issues with the version we are currently using (3.2.0).

I think it is worth updating now, but I'll let you decide.

[bestander]

I just wrote import command, some manual work to update internal app will be required, so feel free to bump this PR to the needed version please :)

[AndrewJack]

@bestander that's done and I've closed #8680

[bestander]

@facebook-github-bot import

[ghost]

Thanks for importing. If you are an FB employee go to Phabricator to review.

[bestander]

It'll take a couple of days to allow people to review things and see if anything gets raised in okhttp repo

[bestander]

Hey @AndrewJack, I stumbled upon internal tests failing because of square/okhttp#2533.
We have quite a few robolectric tests internally that fail because of isCleartextTrafficPermitted assert.

[bestander]

I wonder what we should do about it.
Patching tests mocks is possible but I am not sure how the isCleartextTrafficPermitted changes will affect our production apps.
Also I don't want us to diverge okhttp versions between OSS and what FB uses internally because eventually it will result in the code that does not compile in one of the systems.

[bestander]

Any ideas?

This problem of difference between monorepo of dependencies at FB VS package managers in OSS projects is raised quite often.

Maybe OSS community could provide a layer of abstraction that allows native dependencies to be overridden?

[AndrewJack]

In this case an app could actually force the upgrade with Gradle, by excluding okhttp and okio from react native dependency and adding on okhttp 3.4.1 and okio 1.9.0 as a project dependency.

For example:

dependencies {
    compile('com.facebook.react:react-native:+') {
        exclude group: 'com.squareup.okhttp3'
        exclude group: 'com.squareup.okio'
    }
    compile 'com.squareup.okhttp3:okhttp:3.4.1'
    compile 'com.squareup.okhttp3:okhttp-urlconnection:3.4.1'
    compile 'com.squareup.okhttp3:okhttp-ws:3.4.1'
    compile 'com.squareup.okio:okio:1.9.0'
}

This does seem to be a recurring issue.
OkHttp is very embedded into the react native implementation, so any abstraction would not be a simple task.

[AndrewJack]

@bestander square/okhttp#2533 (comment) might fix your issue, if this what you want to do.

Robolectric 3.1 fixed the issue with OkHttp, but also broke compatibility with power mock.
robolectric/robolectric#2429

[bestander]

I would not want to add some custom mocks until the issue is resolved by okhttp people.
Let's wait till it gets resolved

[AndrewJack]

@bestander ok well in the meantime people can add the latest okhttp dependency to their app dependencies as a workaround.

[bestander]

Thanks, Andrew.

I'll keep an eye on the issue.
Feel free to nudge if there are some changes about it.

On Tuesday, 19 July 2016, Andrew Jack notifications@github.com wrote:

@bestander https://github.com/bestander ok well in the meantime people
can add the latest okhttp dependency as a workaround.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#8672 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACBdWF1vKSTc0ebC152BFQvGxQDPkl46ks5qXN7PgaJpZM4JIqVG
.

[bestander]

actually my team mates decided to do the robolectrick hack to unblock this, so we will be landing this soon, @AndrewJack