Repository: square/okhttp
Issue: Don't recover if a non-GET request fails

Currently it can silently retry requests, but now it's forbidden:

http://tools.ietf.org/html/rfc7230

A user agent MUST NOT automatically retry a request with a non-
   idempotent method unless it has some means to know that the request
   semantics are actually idempotent, regardless of the method, or some
   means to detect that the original request was never applied.

related to https://github.com/square/okhttp/issues/258


Changed files:
okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java, okhttp-tests/src/test/java/com/squareup/okhttp/CallTest.java, okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java

diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/CallTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/CallTest.java
index 25eb85f95..2255dd8f6 100644
--- a/okhttp-tests/src/test/java/com/squareup/okhttp/CallTest.java
+++ b/okhttp-tests/src/test/java/com/squareup/okhttp/CallTest.java
@@ -704,6 +704,34 @@ public final class CallTest {
     callback.await(request.url()).assertHandshake();
   }
 
+  @Test public void recoverWhenRetryOnConnectionFailureIsTrue() throws Exception {
+    server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_AT_START));
+    server.enqueue(new MockResponse().setBody("retry success"));
+
+    Internal.instance.setNetwork(client, new DoubleInetAddressNetwork());
+    assertTrue(client.getRetryOnConnectionFailure());
+
+    Request request = new Request.Builder().url(server.getUrl("/")).build();
+    Response response = client.newCall(request).execute();
+    assertEquals("retry success", response.body().string());
+  }
+
+  @Test public void noRecoverWhenRetryOnConnectionFailureIsFalse() throws Exception {
+    server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_AT_START));
+    server.enqueue(new MockResponse().setBody("unreachable!"));
+
+    Internal.instance.setNetwork(client, new DoubleInetAddressNetwork());
+    client.setRetryOnConnectionFailure(false);
+
+    Request request = new Request.Builder().url(server.getUrl("/")).build();
+    try {
+      // If this succeeds, too many requests were made.
+      client.newCall(request).execute();
+      fail();
+    } catch (IOException expected) {
+    }
+  }
+
   @Test public void recoverFromTlsHandshakeFailure() throws Exception {
     server.get().useHttps(sslContext.getSocketFactory(), false);
     server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.FAIL_HANDSHAKE));
diff --git a/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java b/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java
index 09716a78b..71e646097 100644
--- a/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java
+++ b/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java
@@ -178,6 +178,7 @@ public class OkHttpClient implements Cloneable {
   private Network network;
   private boolean followSslRedirects = true;
   private boolean followRedirects = true;
+  private boolean retryOnConnectionFailure = true;
   private int connectTimeout;
   private int readTimeout;
   private int writeTimeout;
@@ -208,6 +209,7 @@ public class OkHttpClient implements Cloneable {
     this.network = okHttpClient.network;
     this.followSslRedirects = okHttpClient.followSslRedirects;
     this.followRedirects = okHttpClient.followRedirects;
+    this.retryOnConnectionFailure = okHttpClient.retryOnConnectionFailure;
     this.connectTimeout = okHttpClient.connectTimeout;
     this.readTimeout = okHttpClient.readTimeout;
     this.writeTimeout = okHttpClient.writeTimeout;
@@ -449,6 +451,32 @@ public class OkHttpClient implements Cloneable {
     return followRedirects;
   }
 
+  /**
+   * 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;
+  }
+
+  public final boolean getRetryOnConnectionFailure() {
+    return retryOnConnectionFailure;
+  }
+
   final RouteDatabase routeDatabase() {
     return routeDatabase;
   }
diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java
index 461f066c8..d4d9b4cb1 100644
--- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java
+++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java
@@ -391,6 +391,11 @@ public final class HttpEngine {
   }
 
   private boolean isRecoverable(IOException e) {
+    // If the application has opted-out of recovery, don't recover.
+    if (!client.getRetryOnConnectionFailure()) {
+      return false;
+    }
+
     // If the problem was a CertificateException from the X509TrustManager,
     // do not retry, we didn't have an abrupt server-initiated exception.
     if (e instanceof SSLPeerUnverifiedException