Merge pull request #5789 from square/jwilson.0216.give_up

Don't do infinite retries on server-canceled calls

Author: swankjesse

okhttp/src/main/java/okhttp3/internal/connection/Exchange.kt

@@ -164,7 +164,7 @@ class Exchange(
 
   private fun trackFailure(e: IOException) {
     finder.trackFailure(e)
-    codec.connection.trackFailure(call.client, e)
+    codec.connection.trackFailure(call, e)
   }
 
   fun <E : IOException?> bodyComplete(

okhttp/src/main/java/okhttp3/internal/connection/RealConnection.kt

@@ -683,13 +683,13 @@ class RealConnection(
    * Track a failure using this connection. This may prevent both the connection and its route from
    * being used for future exchanges.
    */
-  internal fun trackFailure(client: OkHttpClient, e: IOException?) {
+  internal fun trackFailure(call: RealCall, e: IOException?) {
     connectionPool.assertThreadDoesntHoldLock()
 
     synchronized(connectionPool) {
       if (e is StreamResetException) {
-        when (e.errorCode) {
-          ErrorCode.REFUSED_STREAM -> {
+        when {
+          e.errorCode == ErrorCode.REFUSED_STREAM -> {
             // Stop using this connection on the 2nd REFUSED_STREAM error.
             refusedStreamCount++
             if (refusedStreamCount > 1) {
@@ -698,8 +698,8 @@ class RealConnection(
             }
           }
 
-          ErrorCode.CANCEL -> {
-            // Permit any number of CANCEL errors on each connection.
+          e.errorCode == ErrorCode.CANCEL && call.isCanceled() -> {
+            // Permit any number of CANCEL errors on locally-canceled calls.
           }
 
           else -> {
@@ -714,7 +714,7 @@ class RealConnection(
         // If this route hasn't completed a call, avoid it for new connections.
         if (successCount == 0) {
           if (e != null) {
-            connectFailed(client, route, e)
+            connectFailed(call.client, route, e)
           }
           routeFailureCount++
         }

okhttp/src/test/java/okhttp3/internal/http2/HttpOverHttp2Test.java

@@ -881,9 +881,17 @@ public void noRecoveryFromTwoRefusedStreams() throws Exception {
   }
 
   @Test public void recoverFromOneInternalErrorRequiresNewConnection() throws Exception {
+    recoverFromOneHttp2ErrorRequiresNewConnection(ErrorCode.INTERNAL_ERROR);
+  }
+
+  @Test public void recoverFromOneCancelRequiresNewConnection() throws Exception {
+    recoverFromOneHttp2ErrorRequiresNewConnection(ErrorCode.CANCEL);
+  }
+
+  private void recoverFromOneHttp2ErrorRequiresNewConnection(ErrorCode errorCode) throws Exception {
     server.enqueue(new MockResponse()
         .setSocketPolicy(SocketPolicy.RESET_STREAM_AT_START)
-        .setHttp2ErrorCode(ErrorCode.INTERNAL_ERROR.getHttpCode()));
+        .setHttp2ErrorCode(errorCode.getHttpCode()));
     server.enqueue(new MockResponse()
         .setBody("abc"));
 
@@ -1064,6 +1072,10 @@ private void callAndCancel(int expectedSequenceNumber, CountDownLatch responseDe
     noRecoveryFromErrorWithRetryDisabled(ErrorCode.INTERNAL_ERROR);
   }
 
+  @Test public void noRecoveryFromCancelWithRetryDisabled() throws Exception {
+    noRecoveryFromErrorWithRetryDisabled(ErrorCode.CANCEL);
+  }
+
   private void noRecoveryFromErrorWithRetryDisabled(ErrorCode errorCode) throws Exception {
     server.enqueue(new MockResponse()
         .setSocketPolicy(SocketPolicy.RESET_STREAM_AT_START)