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

New API to disable retry for an OkHttpClient. #1259

Merged
merged 1 commit into from
Dec 31, 2014
Merged

Conversation

swankjesse
Copy link
Collaborator

Most applications won't want to disable retry globally. Instead, use
clone() to get an OkHttpClient for a specific, non-idempotent request,
then configure that client with the setting.

Closes #1043

@codefromthecrypt
Copy link

Excellent idea and docs.

:shipit:

Most applications won't want to disable retry globally. Instead, use
clone() to get an OkHttpClient for a specific, non-idempotent request,
then configure that client with the setting.

Closes #1043
JakeWharton added a commit that referenced this pull request Dec 31, 2014
New API to disable retry for an OkHttpClient.
@JakeWharton JakeWharton merged commit 8f3cae8 into master Dec 31, 2014
@ValleZ
Copy link

ValleZ commented Dec 31, 2014

yay! Thanks!

@floating-cat
Copy link

Can I ask a stupid question?

  /**
   * Configure this client to retry or not when a connectivity problem is encountered. By default,
   * this client silently recovers from the following problems:
   *
   * <ul>
   *   <li><strong>Unreachable IP addresses.</strong> If the URL's host has multiple IP addresses,
   *       failure to reach any individual IP address doesn't fail the overall request. This can
   *       increase availability of multi-homed services.
   *   <li><strong>Stale pooled connections.</strong> The {@link ConnectionPool} reuses sockets
   *       to decrease request latency, but these connections will occasionally time out.
   *   <li><strong>Unreachable proxy servers.</strong> A {@link ProxySelector} can be used to
   *       attempt multiple proxy servers in sequence, eventually falling back to a direct
   *       connection.
   * </ul>
   *
   * Set this to false to avoid retrying requests when doing so is destructive. In this case the
   * calling application should do its own recovery of connectivity failures.
   */
  public final void setRetryOnConnectionFailure(boolean retryOnConnectionFailure) {
    this.retryOnConnectionFailure = retryOnConnectionFailure;
  }

If I send a non-idempotent request to server (like comment in GitHub), and one of those problems above happens. Won't this request handled by server twice (like comment in GitHub twice)?
I guess Unreachable proxy servers won't, but I am not sure others.

@JakeWharton
Copy link
Collaborator

If this is set to true (which is the default) then yes that will happen if all of the request body successfully made it to the server. This was the existing behavior of OkHttp. The added API in this pull request only adds the ability to disable this functionality for those poorly written APIs.

@floating-cat
Copy link

Thank you!

@narayank
Copy link
Collaborator

The "stale pooled connections" case worries me a little bit. See https://code.google.com/p/android/issues/detail?id=61706 for why this will be quite common on linux based OSes. Depending on request patterns, some users might hit this 100% of the time.

@swankjesse
Copy link
Collaborator Author

@narayank agreed. Which is why you have to explicitly opt-out of retry. We're also conservative about pooled connections for non-GET requests.

@swankjesse swankjesse deleted the jwilson_1230_fail branch January 3, 2015 19:19
@1zaman
Copy link

1zaman commented Feb 11, 2015

Why not provide an option for disabling retries based on the potential non-idempotency of the HTTP method itself? I am sure that the overwhelming majority of use-cases for disabling retries are for this. Absent this option, I ended up writing my own delegator class to implement this.

Note that while HttpUrlConnection has always been broken, it is required by the IETF standard for HTTP clients to not retry requests with non-idempotent methods. #1043 was closed against this feature, although it requested compliant behavior.

@afinas-em
Copy link

afinas-em commented Nov 29, 2019

OkHttpClient okHttpClient= null;
        okHttpClient = new OkHttpClient.Builder()
                .sslSocketFactory(new TLSSocketFactory(),trustManager)
                .connectTimeout(2, TimeUnit.MINUTES)
                .readTimeout(2, TimeUnit.MINUTES)
                .writeTimeout(2, TimeUnit.MINUTES)
                //.sslSocketFactory(sslSocketFactory, trustManager)
                .followRedirects(false)
                .followSslRedirects(false)
                .retryOnConnectionFailure(false)
                .cache(null)//new Cache(sContext.getCacheDir(),10*1024*1024)
                .build();

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

Successfully merging this pull request may close these issues.

Don't recover if a non-GET request fails
9 participants