Recover from REFUSED_STREAM errors in HTTP/2.#2571
Conversation
| return recover(e, routeException, requestBodyOut); | ||
| } | ||
|
|
||
| private boolean isRecoverable(IOException e, boolean routeException) { |
There was a problem hiding this comment.
(This code is just moved)
swankjesse
force-pushed
the
jwilson.0521.recover_from_refused_stream
branch
from
71c7a9d to
51326d2
Compare
| server.enqueue(new MockResponse() | ||
| .setBody("abc")); | ||
|
|
||
| Call call = client.newCall(new Request.Builder() |
There was a problem hiding this comment.
What happens if you use the SingleInetAddressDns here? I was testing with nginx 1.9.15 and I noticed the refuse stream stack trace. I think it's because I was using the IP address directly, but still investigating.
There was a problem hiding this comment.
You saw something I didn’t see! I turned that on and of course the test started to fail. I fixed it by changing hasMoreRoutes() to return true if the current route is still valid.
There was a problem hiding this comment.
Oh, gotcha. I couldn't get those tests to run in intellij, but just got it working by upgrading my JDK version.
swankjesse
force-pushed
the
jwilson.0521.recover_from_refused_stream
branch
from
51326d2 to
bf0c639
Compare
|
Nice, OkHttp can now persevere in yet another hostile scenario. The only thing I had left in my notes - if we wanted this to work with nginx 1.9.15 specifically, I believe we'll need to flush the header frames. Right now, when OkHttp tries to write the data frames, it blocks indefinitely because the window size is 0. The header frames are buffered, so never get sent to nginx and we never receive a window update. See https://github.com/square/okhttp/blob/master/okhttp/src/main/java/okhttp3/internal/framed/FramedConnection.java#L278 I'll defer to Jake. |
| return http2ErrorCode; | ||
| } | ||
|
|
||
| public MockResponse setHttp2ErrorCode(int http2ErrorCode) { |
There was a problem hiding this comment.
+Javadoc explaining that this is used only for SocketPolicy. RESET_STREAM_AT_START
There was a problem hiding this comment.
The code is defined by HTTP/2. Definitely needs doc. Added.
|
only minor nits, lgtm |
This implements the following policy: - If a REFUSED_STREAM error is received, OkHttp will retry the same stream on the same socket 1x. - If any other error is received, or an additional REFUSED_STREAM error is received, OkHttp will retry on a different route if one exists. We may want to follow up by going through HTTP/2 error codes and deciding a per-code retry policy, but this should be good enough for now. Closes: #2543
swankjesse
force-pushed
the
jwilson.0521.recover_from_refused_stream
branch
from
bf0c639 to
c9290b7
Compare
3 participants
This implements the following policy:
on the same socket 1x.
received, OkHttp will retry on a different route if one exists.
We may want to follow up by going through HTTP/2 error codes and deciding
a per-code retry policy, but this should be good enough for now.
Closes: #2543