Repository: square/retrofit
Issue: Support okhttp3.Call.timeout()
Labels: Feature

https://github.com/square/okhttp/blob/c273b3be385383842e44d25d6b7ca137ff6b1076/okhttp/src/main/java/okhttp3/Call.java#L84-L91

I have one endpoint in the retrofit `interface` that needs an increased timeout. Currently I have to create a modified `OkHttpClient`, and then create an additional `interface` with an endpoint that needs the increased timeout.

It would be great to have annotation for that

Issue comments:
You don't actually have to do what you described to get per-service timeouts. There's two ways this can be done currently:

 1. Add a `@Headers` annotation with a synthetic header that specifies the timeout value on the service method. Read and strip the header in an interceptor. Update the timeout on the `chain` based on the header value before calling `chain.proceed`.

 2. Add a custom `@Timeout` annotation on the service method. ~In an interceptor, read the `Invocation.class` tag instance from the `Call` object and adjust the timeout on the `chain` before calling `chain.proceed`.~ In a delegating `CallAdapter.Factory`, read the annotation and create a `CallAdapter` which applies the timeout. See https://github.com/square/retrofit/issues/2982#issuecomment-456608496

I'm not sure if we want to add static (i.e., annotation-based) and/or dynamic (i.e., parameter-based) timeouts. I'll have to think about it! And additional use cases are welcome.
now when I think about it, I'm not even sure if `okhttp3.Call.timeout()` will help me.

since I have a `OkHttpClient` with `X` ms connect & `Y` ms read timeouts, will `Call.timeout((X + Y) * 2)` have any effect? Need to try that one out
Suggestion 2 didn't work above, but this should: https://github.com/square/retrofit/issues/2982#issuecomment-456608496
I think we can do like this.
```java
    @Retention(RetentionPolicy.RUNTIME)
    @Target(ElementType.METHOD)
    public @interface Timeout {
        int value();

        TimeUnit unit();
    }

    public interface ApiService {
        @Timeout(value = 100, unit = TimeUnit.MILLISECONDS)
        @GET("something")
        retrofit2.Call<String> getSomething();
    }

public class CallFactoryProxy implements Call.Factory {

    protected final Call.Factory delegate;

    public CallFactoryProxy(Call.Factory delegate) {
        Utils.checkNotNull(delegate, "delegate==null");
        this.delegate = delegate;
    }

    @Override
    public Call newCall(Request request) {
        Call call = delegate.newCall(request);
        Invocation tag = request.tag(Invocation.class);
        Method method = tag.method();
        Timeout timeout = method.getAnnotation(Timeout.class);
        if (timeout != null) {
            call.timeout().timeout(timeout.value(), timeout.unit());
        }
        return call;
    }
}
      // create retrofit
       OkHttpClient client = new OkHttpClient.Builder()
                .build();
        Retrofit retrofit = new Retrofit.Builder()
                .baseUrl("https://xxxxx.com/")
                .callFactory(new CallFactoryProxy(client))
                .addConverterFactory(GsonConverterFactory.create())
                .build();

```
or  dynamic  you can use `@Header` or ` @Tag` and parse it from `newCall(Request request)` 

Pull request review comments:
Needs to be synchronized. Concurrent invocations can return different instances or see partial state.

I hope error-prone catches this because of the `@GuardedBy`...
It did. Whew.

Patch that resolved the issue:
diff --git a/retrofit-mock/src/main/java/retrofit2/mock/BehaviorCall.java b/retrofit-mock/src/main/java/retrofit2/mock/BehaviorCall.java
index a41664876c..d333d0f09b 100644
--- a/retrofit-mock/src/main/java/retrofit2/mock/BehaviorCall.java
+++ b/retrofit-mock/src/main/java/retrofit2/mock/BehaviorCall.java
@@ -23,6 +23,7 @@
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.GuardedBy;
 import okhttp3.Request;
+import okio.Timeout;
 import retrofit2.Call;
 import retrofit2.Callback;
 import retrofit2.Response;
@@ -54,6 +55,10 @@ final class BehaviorCall<T> implements Call<T> {
     return delegate.request();
   }
 
+  @Override public Timeout timeout() {
+    return delegate.timeout();
+  }
+
   @SuppressWarnings("ConstantConditions") // Guarding public API nullability.
   @Override public void enqueue(final Callback<T> callback) {
     if (callback == null) throw new NullPointerException("callback == null");
diff --git a/retrofit-mock/src/main/java/retrofit2/mock/Calls.java b/retrofit-mock/src/main/java/retrofit2/mock/Calls.java
index 65d3afd4af..1670e62ae8 100644
--- a/retrofit-mock/src/main/java/retrofit2/mock/Calls.java
+++ b/retrofit-mock/src/main/java/retrofit2/mock/Calls.java
@@ -20,6 +20,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import javax.annotation.Nullable;
 import okhttp3.Request;
+import okio.Timeout;
 import retrofit2.Call;
 import retrofit2.Callback;
 import retrofit2.Response;
@@ -136,6 +137,10 @@ private static <T extends Throwable> T sneakyThrow2(Throwable t) throws T {
           .url("http://localhost")
           .build();
     }
+
+    @Override public Timeout timeout() {
+      return Timeout.NONE;
+    }
   }
 
   static final class DeferredCall<T> implements Call<T> {
@@ -186,5 +191,9 @@ private synchronized Call<T> getDelegate() {
     @Override public Request request() {
       return getDelegate().request();
     }
+
+    @Override public Timeout timeout() {
+      return getDelegate().timeout();
+    }
   }
 }
diff --git a/retrofit/src/main/java/retrofit2/Call.java b/retrofit/src/main/java/retrofit2/Call.java
index 171d9e0c41..bb861fbaa7 100644
--- a/retrofit/src/main/java/retrofit2/Call.java
+++ b/retrofit/src/main/java/retrofit2/Call.java
@@ -16,7 +16,9 @@
 package retrofit2;
 
 import java.io.IOException;
+import okhttp3.OkHttpClient;
 import okhttp3.Request;
+import okio.Timeout;
 
 /**
  * An invocation of a Retrofit method that sends a request to a webserver and returns a response.
@@ -70,4 +72,11 @@ public interface Call<T> extends Cloneable {
 
   /** The original HTTP request. */
   Request request();
+
+  /**
+   * Returns a timeout that spans the entire call: resolving DNS, connecting, writing the request
+   * body, server processing, and reading the response body. If the call requires redirects or
+   * retries all must complete within one timeout period.
+   */
+  Timeout timeout();
 }
diff --git a/retrofit/src/main/java/retrofit2/DefaultCallAdapterFactory.java b/retrofit/src/main/java/retrofit2/DefaultCallAdapterFactory.java
index e7a9c3b5f7..ab22274bff 100644
--- a/retrofit/src/main/java/retrofit2/DefaultCallAdapterFactory.java
+++ b/retrofit/src/main/java/retrofit2/DefaultCallAdapterFactory.java
@@ -23,6 +23,7 @@
 import java.util.concurrent.Executor;
 import javax.annotation.Nullable;
 import okhttp3.Request;
+import okio.Timeout;
 
 final class DefaultCallAdapterFactory extends CallAdapter.Factory {
   private final @Nullable Executor callbackExecutor;
@@ -113,5 +114,9 @@ static final class ExecutorCallbackCall<T> implements Call<T> {
     @Override public Request request() {
       return delegate.request();
     }
+
+    @Override public Timeout timeout() {
+      return delegate.timeout();
+    }
   }
 }
diff --git a/retrofit/src/main/java/retrofit2/OkHttpCall.java b/retrofit/src/main/java/retrofit2/OkHttpCall.java
index cc348a6602..10f27da97a 100644
--- a/retrofit/src/main/java/retrofit2/OkHttpCall.java
+++ b/retrofit/src/main/java/retrofit2/OkHttpCall.java
@@ -26,6 +26,7 @@
 import okio.BufferedSource;
 import okio.ForwardingSource;
 import okio.Okio;
+import okio.Timeout;
 
 import static retrofit2.Utils.throwIfFatal;
 
@@ -58,28 +59,48 @@ final class OkHttpCall<T> implements Call<T> {
   }
 
   @Override public synchronized Request request() {
-    okhttp3.Call call = rawCall;
-    if (call != null) {
-      return call.request();
+    try {
+      return getRawCall().request();
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to create request.", e);
+    }
+  }
+
+  @Override public synchronized Timeout timeout() {
+    try {
+      return getRawCall().timeout();
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to create call.", e);
     }
+  }
+
+  /**
+   * Returns the raw call, initializing it if necessary. Throws if initializing the raw call throws,
+   * or has thrown in previous attempts to create it.
+   */
+  @GuardedBy("this")
+  private okhttp3.Call getRawCall() throws IOException {
+    okhttp3.Call call = rawCall;
+    if (call != null) return call;
+
+    // Re-throw previous failures if this isn't the first attempt.
     if (creationFailure != null) {
       if (creationFailure instanceof IOException) {
-        throw new RuntimeException("Unable to create request.", creationFailure);
+        throw (IOException) creationFailure;
       } else if (creationFailure instanceof RuntimeException) {
         throw (RuntimeException) creationFailure;
       } else {
         throw (Error) creationFailure;
       }
     }
+
+    // Create and remember either the success or the failure.
     try {
-      return (rawCall = createRawCall()).request();
-    } catch (RuntimeException | Error e) {
+      return rawCall = createRawCall();
+    } catch (RuntimeException | Error | IOException e) {
       throwIfFatal(e); // Do not assign a fatal error to creationFailure.
       creationFailure = e;
       throw e;
-    } catch (IOException e) {
-      creationFailure = e;
-      throw new RuntimeException("Unable to create request.", e);
     }
   }
 
@@ -159,26 +180,7 @@ private void callFailure(Throwable e) {
       if (executed) throw new IllegalStateException("Already executed.");
       executed = true;
 
-      if (creationFailure != null) {
-        if (creationFailure instanceof IOException) {
-          throw (IOException) creationFailure;
-        } else if (creationFailure instanceof RuntimeException) {
-          throw (RuntimeException) creationFailure;
-        } else {
-          throw (Error) creationFailure;
-        }
-      }
-
-      call = rawCall;
-      if (call == null) {
-        try {
-          call = rawCall = createRawCall();
-        } catch (IOException | RuntimeException | Error e) {
-          throwIfFatal(e); //  Do not assign a fatal error to creationFailure.
-          creationFailure = e;
-          throw e;
-        }
-      }
+      call = getRawCall();
     }
 
     if (canceled) {


Test patch:
diff --git a/retrofit/src/test/java/retrofit2/CallTest.java b/retrofit/src/test/java/retrofit2/CallTest.java
index f347189efe..2e13db5c66 100644
--- a/retrofit/src/test/java/retrofit2/CallTest.java
+++ b/retrofit/src/test/java/retrofit2/CallTest.java
@@ -16,9 +16,11 @@
 package retrofit2;
 
 import java.io.IOException;
+import java.io.InterruptedIOException;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Type;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
@@ -1248,4 +1250,58 @@ public Converter<ResponseBody, String> responseBodyConverter(Type type,
     }
     assertThat(writeCount.get()).isEqualTo(2);
   }
+
+  @Test public void timeoutExceeded() throws IOException {
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addConverterFactory(new ToStringConverterFactory())
+        .build();
+    Service example = retrofit.create(Service.class);
+
+    server.enqueue(new MockResponse()
+        .setHeadersDelay(500, TimeUnit.MILLISECONDS));
+
+    Call<String> call = example.getString();
+    call.timeout().timeout(100, TimeUnit.MILLISECONDS);
+    try {
+      call.execute();
+      fail();
+    } catch (InterruptedIOException expected) {
+    }
+  }
+
+  @Test public void deadlineExceeded() throws IOException {
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addConverterFactory(new ToStringConverterFactory())
+        .build();
+    Service example = retrofit.create(Service.class);
+
+    server.enqueue(new MockResponse()
+        .setHeadersDelay(500, TimeUnit.MILLISECONDS));
+
+    Call<String> call = example.getString();
+    call.timeout().deadline(100, TimeUnit.MILLISECONDS);
+    try {
+      call.execute();
+      fail();
+    } catch (InterruptedIOException expected) {
+    }
+  }
+
+  @Test public void timeoutEnabledButNotExceeded() throws IOException {
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addConverterFactory(new ToStringConverterFactory())
+        .build();
+    Service example = retrofit.create(Service.class);
+
+    server.enqueue(new MockResponse()
+        .setHeadersDelay(100, TimeUnit.MILLISECONDS));
+
+    Call<String> call = example.getString();
+    call.timeout().deadline(500, TimeUnit.MILLISECONDS);
+    Response<String> response = call.execute();
+    assertThat(response.isSuccessful()).isTrue();
+  }
 }
diff --git a/retrofit/src/test/java/retrofit2/DefaultCallAdapterFactoryTest.java b/retrofit/src/test/java/retrofit2/DefaultCallAdapterFactoryTest.java
index 85182a469c..5d1e008479 100644
--- a/retrofit/src/test/java/retrofit2/DefaultCallAdapterFactoryTest.java
+++ b/retrofit/src/test/java/retrofit2/DefaultCallAdapterFactoryTest.java
@@ -22,6 +22,7 @@
 import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
 import okhttp3.Request;
+import okio.Timeout;
 import org.junit.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -130,5 +131,9 @@ static class EmptyCall implements Call<String> {
     @Override public Request request() {
       throw new UnsupportedOperationException();
     }
+
+    @Override public Timeout timeout() {
+      return Timeout.NONE;
+    }
   }
 }