Repository: square/retrofit
Issue: UrlConnectionClient default timeouts behavior

I think that the connection and read timeout in `UrlConnectionClient.java` should be set to specific values other than the defaults.

With OkHttp the default are 0 for both of them.
The [documentation](http://developer.android.com/reference/java/net/URLConnection.html#setReadTimeout%28int%29) for ReadTimeout says:

> Sets the maximum time to wait for an input stream read to complete before giving up. Reading will fail with a SocketTimeoutException if the timeout elapses before data becomes available. The default value of 0 disables read timeouts; read attempts will block indefinitely.

This might not be what we want as a default behavior. Maybe this apply to Picasso also..


Issue comments:
Yep. Good catch!

Is there any method allowing custom timeout value in retrofit?

@uKL No. It's an HTTP client configuration and thus should be configured there. We will provide defaults for use with the default HTTP clients, though.

Roger that. So I'll have to implement _Client_ interface and create my own Http client. Do you think it's a good idea to extend _UrlConnectionClient_ and just deal with protected _HttpURLConnection openConnection(Request request)_ method?

Yes. That's a good strategy for `HttpUrlConnection`. If you're using OkHttp or Apache HttpClient you can configure the client and just pass it to the client constructor.

``` java
OkHttpClient ok = new OkHttpClient();
// TODO configure timeouts and such

RestAdapter ra = new RestAdapter.Builder()
  .client(new OkClient(ok))
  .build();
```

Thanks. Anyway, personally I think that the `Client` interface should implement some common stuff like timeouts. After digging in the source code, it'd be useful if I'd be able to use default implementation of `Platform.defaultClient()` and still be able to set most commonly used values.

Hi, I having the same request. I try to configure the timeout by myself as @JakeWharton indicates in the previous comment, but I cannot set the HttpParams to OkHttpClient.

I tried something like:

HttpParams httpParams = new BasicHttpParams();
            HttpConnectionParams.setConnectionTimeout(httpParams, CONNECTION_TIMEOUT);
            DefaultHttpClient client = new DefaultHttpClient(httpParams);

But then I cannot set the client to RestAdapter. Any ideas?

Thx!

``` java
RestAdapter r = new RestAdapter.Builder()
    .setSever(...)
    .setClient(new ApacheClient(client))
    .build();
```

How can I configure an `OkHttpClient` to use custom timeouts? Should I extend the `OkClient` from retrofit to set the timeouts for each `HttpURLConnection` created via `UrlConnectionClient.openConnection(Request)` ?

You don't need to extend.

``` java
OkHttpClient client = new OkHttpClient();
client.setBlahTimeout(1234);

RestAdapter r = new RestAdapter.Builder()
    .setServer(..)
    .setClient(new OkClient(client))
    .build();
```

I lied. You need to subclass!

Thx, I got it.


Pull request review comments:

Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit/client/ApacheClient.java b/retrofit/src/main/java/retrofit/client/ApacheClient.java
index 1b75da3d19..480f1cd14c 100644
--- a/retrofit/src/main/java/retrofit/client/ApacheClient.java
+++ b/retrofit/src/main/java/retrofit/client/ApacheClient.java
@@ -32,17 +32,27 @@
 import org.apache.http.entity.AbstractHttpEntity;
 import org.apache.http.impl.client.DefaultHttpClient;
 import org.apache.http.message.BasicHeader;
+import org.apache.http.params.BasicHttpParams;
+import org.apache.http.params.HttpConnectionParams;
+import org.apache.http.params.HttpParams;
 import org.apache.http.util.EntityUtils;
 import retrofit.mime.TypedByteArray;
 import retrofit.mime.TypedOutput;
 
 /** A {@link Client} which uses an implementation of Apache's {@link HttpClient}. */
 public class ApacheClient implements Client {
+  private static HttpClient createDefaultClient() {
+    HttpParams params = new BasicHttpParams();
+    HttpConnectionParams.setConnectionTimeout(params, Defaults.CONNECT_TIMEOUT);
+    HttpConnectionParams.setSoTimeout(params, Defaults.READ_TIMEOUT);
+    return new DefaultHttpClient(params);
+  }
+
   private final HttpClient client;
 
   /** Creates an instance backed by {@link DefaultHttpClient}. */
   public ApacheClient() {
-    this(new DefaultHttpClient());
+    this(createDefaultClient());
   }
 
   public ApacheClient(HttpClient client) {
diff --git a/retrofit/src/main/java/retrofit/client/Defaults.java b/retrofit/src/main/java/retrofit/client/Defaults.java
new file mode 100644
index 0000000000..95de30500e
--- /dev/null
+++ b/retrofit/src/main/java/retrofit/client/Defaults.java
@@ -0,0 +1,6 @@
+package retrofit.client;
+
+class Defaults {
+  static final int CONNECT_TIMEOUT = 15 * 1000; // 15s
+  static final int READ_TIMEOUT = 20 * 1000; // 20s
+}
diff --git a/retrofit/src/main/java/retrofit/client/OkClient.java b/retrofit/src/main/java/retrofit/client/OkClient.java
index 122fb55edb..6286cdd6c3 100644
--- a/retrofit/src/main/java/retrofit/client/OkClient.java
+++ b/retrofit/src/main/java/retrofit/client/OkClient.java
@@ -33,6 +33,9 @@ public OkClient(OkHttpClient client) {
   }
 
   @Override protected HttpURLConnection openConnection(Request request) throws IOException {
-    return client.open(new URL(request.getUrl()));
+    HttpURLConnection connection = client.open(new URL(request.getUrl()));
+    connection.setConnectTimeout(Defaults.CONNECT_TIMEOUT);
+    connection.setReadTimeout(Defaults.READ_TIMEOUT);
+    return connection;
   }
 }
diff --git a/retrofit/src/main/java/retrofit/client/UrlConnectionClient.java b/retrofit/src/main/java/retrofit/client/UrlConnectionClient.java
index fcca8c4846..d7375d8b29 100644
--- a/retrofit/src/main/java/retrofit/client/UrlConnectionClient.java
+++ b/retrofit/src/main/java/retrofit/client/UrlConnectionClient.java
@@ -34,7 +34,11 @@ public class UrlConnectionClient implements Client {
   }
 
   protected HttpURLConnection openConnection(Request request) throws IOException {
-    return (HttpURLConnection) new URL(request.getUrl()).openConnection();
+    HttpURLConnection connection =
+        (HttpURLConnection) new URL(request.getUrl()).openConnection();
+    connection.setConnectTimeout(Defaults.CONNECT_TIMEOUT);
+    connection.setReadTimeout(Defaults.READ_TIMEOUT);
+    return connection;
   }
 
   void prepareRequest(HttpURLConnection connection, Request request) throws IOException {


Test patch: