Whole operation timeouts

Strictly-speaking this change is backwards-incompatible because it adds a new method to the Call interface. The method returns the call's timeout. The trickiest part of this is signaling the end of the call, which occurs after the last byte is consumed of the last follow up request, or when the call fails. Fortunately this is made easier by borrowing the sites used by EventListener, which already plots out where calls end. #2840
square · Nov 4, 2018 · 5373160 · 5373160
1 parent 2b0a9f4
commit 5373160
Show file tree

Hide file tree

Showing 6 changed files with 321 additions and 1 deletion.
diff --git a/okhttp-tests/src/test/java/okhttp3/WholeOperationTimeoutTest.java b/okhttp-tests/src/test/java/okhttp3/WholeOperationTimeoutTest.java
@@ -0,0 +1,274 @@
+/*
+ * Copyright (C) 2018 Square, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package okhttp3;
+
+import java.io.IOException;
+import java.io.InterruptedIOException;
+import java.net.HttpURLConnection;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import okhttp3.mockwebserver.MockResponse;
+import okhttp3.mockwebserver.MockWebServer;
+import okio.BufferedSink;
+import org.junit.Rule;
+import org.junit.Test;
+
+import static okhttp3.TestUtil.defaultClient;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public final class WholeOperationTimeoutTest {
+  /** A large response body. Smaller bodies might successfully read after the socket is closed! */
+  private static final String BIG_ENOUGH_BODY = TestUtil.repeat('a', 64 * 1024);
+
+  @Rule public final MockWebServer server = new MockWebServer();
+
+  private OkHttpClient client = defaultClient();
+
+  @Test public void timeoutWritingRequest() throws Exception {
+    server.enqueue(new MockResponse());
+
+    Request request = new Request.Builder()
+        .url(server.url("/"))
+        .post(sleepingRequestBody(500))
+        .build();
+
+    Call call = client.newCall(request);
+    call.timeout().timeout(250, TimeUnit.MILLISECONDS);
+    try {
+      call.execute();
+      fail();
+    } catch (IOException e) {
+      assertTrue(call.isCanceled());
+    }
+  }
+
+  @Test public void timeoutWritingRequestWithEnqueue() throws Exception {
+    server.enqueue(new MockResponse());
+
+    Request request = new Request.Builder()
+        .url(server.url("/"))
+        .post(sleepingRequestBody(500))
+        .build();
+
+    final CountDownLatch latch = new CountDownLatch(1);
+    final AtomicReference<Throwable> exceptionRef = new AtomicReference<>();
+
+    Call call = client.newCall(request);
+    call.timeout().timeout(250, TimeUnit.MILLISECONDS);
+    call.enqueue(new Callback() {
+      @Override public void onFailure(Call call, IOException e) {
+        exceptionRef.set(e);
+        latch.countDown();
+      }
+
+      @Override public void onResponse(Call call, Response response) throws IOException {
+        response.close();
+        latch.countDown();
+      }
+    });
+
+    latch.await();
+    assertTrue(call.isCanceled());
+    assertNotNull(exceptionRef.get());
+  }
+
+  @Test public void timeoutProcessing() throws Exception {
+    server.enqueue(new MockResponse()
+        .setHeadersDelay(500, TimeUnit.MILLISECONDS));
+
+    Request request = new Request.Builder()
+        .url(server.url("/"))
+        .build();
+
+    Call call = client.newCall(request);
+    call.timeout().timeout(250, TimeUnit.MILLISECONDS);
+    try {
+      call.execute();
+      fail();
+    } catch (IOException e) {
+      assertTrue(call.isCanceled());
+    }
+  }
+
+  @Test public void timeoutProcessingWithEnqueue() throws Exception {
+    server.enqueue(new MockResponse()
+        .setHeadersDelay(500, TimeUnit.MILLISECONDS));
+
+    Request request = new Request.Builder()
+        .url(server.url("/"))
+        .build();
+
+    final CountDownLatch latch = new CountDownLatch(1);
+    final AtomicReference<Throwable> exceptionRef = new AtomicReference<>();
+
+    Call call = client.newCall(request);
+    call.timeout().timeout(250, TimeUnit.MILLISECONDS);
+    call.enqueue(new Callback() {
+      @Override public void onFailure(Call call, IOException e) {
+        exceptionRef.set(e);
+        latch.countDown();
+      }
+
+      @Override public void onResponse(Call call, Response response) throws IOException {
+        response.close();
+        latch.countDown();
+      }
+    });
+
+    latch.await();
+    assertTrue(call.isCanceled());
+    assertNotNull(exceptionRef.get());
+  }
+
+  @Test public void timeoutReadingResponse() throws Exception {
+    server.enqueue(new MockResponse()
+        .setBody(BIG_ENOUGH_BODY));
+
+    Request request = new Request.Builder()
+        .url(server.url("/"))
+        .build();
+
+    Call call = client.newCall(request);
+    call.timeout().timeout(250, TimeUnit.MILLISECONDS);
+    Response response = call.execute();
+    Thread.sleep(500);
+    try {
+      response.body().source().readUtf8();
+      fail();
+    } catch (IOException e) {
+      assertTrue(call.isCanceled());
+    }
+  }
+
+  @Test public void timeoutReadingResponseWithEnqueue() throws Exception {
+    server.enqueue(new MockResponse()
+        .setBody(BIG_ENOUGH_BODY));
+
+    Request request = new Request.Builder()
+        .url(server.url("/"))
+        .build();
+
+    final CountDownLatch latch = new CountDownLatch(1);
+    final AtomicReference<Throwable> exceptionRef = new AtomicReference<>();
+
+    Call call = client.newCall(request);
+    call.timeout().timeout(250, TimeUnit.MILLISECONDS);
+    call.enqueue(new Callback() {
+      @Override public void onFailure(Call call, IOException e) {
+        latch.countDown();
+      }
+
+      @Override public void onResponse(Call call, Response response) throws IOException {
+        try {
+          Thread.sleep(500);
+        } catch (InterruptedException e) {
+          throw new AssertionError();
+        }
+        try {
+          response.body().source().readUtf8();
+          fail();
+        } catch (IOException e) {
+          exceptionRef.set(e);
+        } finally {
+          latch.countDown();
+        }
+      }
+    });
+
+    latch.await();
+    assertTrue(call.isCanceled());
+    assertNotNull(exceptionRef.get());
+  }
+
+  @Test public void singleTimeoutForAllFollowUpRequests() throws Exception {
+    server.enqueue(new MockResponse()
+        .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP)
+        .setHeader("Location", "/b")
+        .setHeadersDelay(100, TimeUnit.MILLISECONDS));
+    server.enqueue(new MockResponse()
+        .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP)
+        .setHeader("Location", "/c")
+        .setHeadersDelay(100, TimeUnit.MILLISECONDS));
+    server.enqueue(new MockResponse()
+        .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP)
+        .setHeader("Location", "/d")
+        .setHeadersDelay(100, TimeUnit.MILLISECONDS));
+    server.enqueue(new MockResponse()
+        .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP)
+        .setHeader("Location", "/e")
+        .setHeadersDelay(100, TimeUnit.MILLISECONDS));
+    server.enqueue(new MockResponse()
+        .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP)
+        .setHeader("Location", "/f")
+        .setHeadersDelay(100, TimeUnit.MILLISECONDS));
+    server.enqueue(new MockResponse());
+
+    Request request = new Request.Builder()
+        .url(server.url("/a"))
+        .build();
+
+    Call call = client.newCall(request);
+    call.timeout().timeout(250, TimeUnit.MILLISECONDS);
+    try {
+      call.execute();
+      fail();
+    } catch (IOException e) {
+      assertTrue(call.isCanceled());
+    }
+  }
+
+  @Test public void noTimeout() throws Exception {
+    server.enqueue(new MockResponse()
+        .setHeadersDelay(250, TimeUnit.MILLISECONDS)
+        .setBody(BIG_ENOUGH_BODY));
+
+    Request request = new Request.Builder()
+        .url(server.url("/"))
+        .post(sleepingRequestBody(250))
+        .build();
+
+    Call call = client.newCall(request);
+    call.timeout().timeout(1000, TimeUnit.MILLISECONDS);
+    Response response = call.execute();
+    Thread.sleep(250);
+    response.body().source().readUtf8();
+    response.close();
+    assertFalse(call.isCanceled());
+  }
+
+  private RequestBody sleepingRequestBody(final int sleepMillis) {
+    return new RequestBody() {
+      @Override public MediaType contentType() {
+        return MediaType.parse("text/plain");
+      }
+
+      @Override public void writeTo(BufferedSink sink) throws IOException {
+        try {
+          sink.writeUtf8("abc");
+          sink.flush();
+          Thread.sleep(sleepMillis);
+          sink.writeUtf8("def");
+        } catch (InterruptedException e) {
+          throw new InterruptedIOException();
+        }
+      }
+    };
+  }
+}
diff --git a/okhttp/src/main/java/okhttp3/Call.java b/okhttp/src/main/java/okhttp3/Call.java
@@ -16,6 +16,7 @@
 package okhttp3;
 
 import java.io.IOException;
+import okio.Timeout;
 
 /**
  * A call is a request that has been prepared for execution. A call can be canceled. As this object
@@ -80,6 +81,12 @@ public interface Call extends Cloneable {
 
   boolean isCanceled();
 
+  /**
+   * Returns a timeout that applies to the entire call: writing the request, server processing,
+   * and reading the response.
+   */
+  Timeout timeout();
+
   /**
    * Create a new, identical call to this one which can be enqueued or executed even if this call
    * has already been.

diff --git a/okhttp/src/main/java/okhttp3/OkHttpClient.java b/okhttp/src/main/java/okhttp3/OkHttpClient.java
@@ -15,6 +15,7 @@
  */
 package okhttp3;
 
+import java.io.IOException;
 import java.net.Proxy;
 import java.net.ProxySelector;
 import java.net.Socket;
@@ -37,11 +38,11 @@
 import okhttp3.internal.Internal;
 import okhttp3.internal.Util;
 import okhttp3.internal.cache.InternalCache;
-import okhttp3.internal.proxy.NullProxySelector;
 import okhttp3.internal.connection.RealConnection;
 import okhttp3.internal.connection.RouteDatabase;
 import okhttp3.internal.connection.StreamAllocation;
 import okhttp3.internal.platform.Platform;
+import okhttp3.internal.proxy.NullProxySelector;
 import okhttp3.internal.tls.CertificateChainCleaner;
 import okhttp3.internal.tls.OkHostnameVerifier;
 import okhttp3.internal.ws.RealWebSocket;
@@ -187,6 +188,10 @@ public void apply(ConnectionSpec tlsConfiguration, SSLSocket sslSocket, boolean
         return ((RealCall) call).streamAllocation();
       }
 
+      @Override public @Nullable IOException timeoutExit(Call call, @Nullable IOException e) {
+        return ((RealCall) call).timeoutExit(e);
+      }
+
       @Override public Call newWebSocketCall(OkHttpClient client, Request originalRequest) {
         return RealCall.newRealCall(client, originalRequest, true);
       }