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

After SocketTimeoutException, request is retried with different TLS mode #442

Closed
efung opened this issue Jan 13, 2014 · 2 comments
Closed
Labels
bug Bug in existing code
Milestone

Comments

@efung
Copy link

efung commented Jan 13, 2014

When I set a connect timeout on OkHttpClient and attempt to reach an HTTPS endpoint that throws a SocketTimeoutException, OkHttp is retrying the request once before giving up. This is what I believe is happening:

  • Upon the initial timeout exception, HttpURLConnectionImpl.handleFailure() adds the failed route (with TLS_MODE_MODERN flag) to the RouteDatabase. The database also adds the route with TLS_MODE_COMPATIBLE since it wasn't a handshake exception.
  • handleFailure() checks to see if the error is recoverable. The test routeSelector.hasNext() returns true, since RouteSelector.hasNextTlsMode() returns true. Another engine is created to try again.
  • The next time through, the route matches a failed route in the RouteDatabase and is added to the set of postponed routes, and is eventually tried again.
  • After this attempt fails with a SocketTimeoutException, there are no more TLS modes, addresses, proxies or postponed routes to try, so the RouteSelector will return null, and HttpURLConnectionImpl returns with a permanent failure.

I would expect that a route is tried again with the fallback TLS behaviour, only if there was a handshake exception, and not when a SocketTimeoutException is encountered.

I'm using OkHttp 1.3.0 on Android 4.2.2.

@swankjesse
Copy link
Member

Yup, great investigation. There's a disconnect between hasNext() and failed TLS modes. We need to tell the route selector that neither TLS mode will work for this host.

@swankjesse
Copy link
Member

Fixed. Thanks for the helpful report!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug in existing code
Projects
None yet
Development

No branches or pull requests

2 participants