Skip to content

[Android] Fix for REFUSED_STREAM with Okhttp#7641

Closed
rahuljiresal wants to merge 1 commit into
react:masterfrom
aerofs:okhttp
Closed

[Android] Fix for REFUSED_STREAM with Okhttp#7641
rahuljiresal wants to merge 1 commit into
react:masterfrom
aerofs:okhttp

Conversation

@rahuljiresal

Copy link
Copy Markdown

Okhttp fails when there is REFUSED_STREAM in HTTP/2. This happens with newer versions of nginx that use HTTP/2. GET requests get processed correctly, however, for POST, it fails with nginx complaining that the client prematurely closed the stream.

client sent stream with data before settings were acknowledged while processing HTTP/2 connection

More info here - square/okhttp#2543 and square/okhttp#2506

Because of this, okhttp does not work with some of the newer nginx servers.

The fix is to configure OkHttpClient with other protocols for it to use. Here's the fix -- wallabag/android-app#256.

As per OkHttp's javadocs,

The client will prefer the most efficient transport available, falling back to more ubiquitous protocols.

If multiple protocols are specified, ALPN will be used to negotiate a transport.

This should make it work with them all nginx servers.

@ghost

ghost commented May 19, 2016

Copy link
Copy Markdown

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

@ghost ghost added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels May 19, 2016
@rahuljiresal

Copy link
Copy Markdown
Author

Sorry, the solution in this PR does not work because even if you enable all protocols, you still get PROTOCOL_ERROR quite a few times. I'm guessing it is because the server does that randomly? Not sure.

Anyway, I'm gonna close this PR and have an issue for anyone who faces the same problem.
#7642

List<Protocol> protocolList = new ArrayList<>();
protocolList.add(Protocol.HTTP_2);
protocolList.add(Protocol.SPDY_3);
protocolList.add(Protocol.HTTP_1_1);

@AndrewJack AndrewJack May 20, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Pull request that you referenced referenced, works around the issue by removing the HTTP_2 protocol.

@AndrewJack

AndrewJack commented May 20, 2016

Copy link
Copy Markdown
Contributor

This workaround will work if you remove the HTTP_2 protocol.

Still only a workaround, as Jessie said here it'll require a code change for Nginx and okhttp to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants