fix(DatafileClient): add timeouts to UrlConnections (#432)

Author: jaeopt

datafile-handler/src/androidTest/java/com/optimizely/ab/android/datafile_handler/DatafileClientTest.java

@@ -37,6 +37,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.contains;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doThrow;
@@ -193,8 +194,6 @@ else if (url == url2) {
         verify(client).saveLastModified(urlConnection2);
         verify(client).readStream(urlConnection2);
         verify(urlConnection2).disconnect();
-
-
     }
 
 
@@ -303,4 +302,23 @@ public void handlesEmptyStringResponse() throws MalformedURLException {
         when(client.execute(any(Client.Request.class), eq(DatafileClient.REQUEST_BACKOFF_TIMEOUT), eq(DatafileClient.REQUEST_RETRIES_POWER))).thenReturn("");
         assertEquals("", datafileClient.request(url.toString()));
     }
+
+    @Test
+    public void handlesConnectionAndReadTimeout() throws IOException {
+        URL url = new URL(new DatafileConfig("1", null).getUrl());
+        when(client.openConnection(url)).thenReturn(urlConnection);
+        when(urlConnection.getResponseCode()).thenReturn(200);
+        doThrow(new IOException()).when(urlConnection).connect();
+
+        datafileClient.request(url.toString());
+        ArgumentCaptor<Client.Request> captor1 = ArgumentCaptor.forClass(Client.Request.class);
+        verify(client).execute(captor1.capture(), anyInt(), anyInt());
+        Object response = captor1.getValue().execute();
+
+        verify(logger).error(contains("Error making request"), any(IOException.class));
+        verify(urlConnection).setConnectTimeout(10*1000);
+        verify(urlConnection).setReadTimeout(60*1000);
+        verify(urlConnection).disconnect();
+    }
+
 }

datafile-handler/src/main/java/com/optimizely/ab/android/datafile_handler/DatafileClient.java

@@ -33,7 +33,9 @@
  */
 public class DatafileClient {
     // easy way to set the connection timeout
-    public static int CONNECTION_TIMEOUT = 5 * 1000;
+    public static int CONNECTION_TIMEOUT = 10 * 1000;
+    public static int READ_TIMEOUT = 60 * 1000;
+
     // the numerical base for the exponential backoff
     public static final int REQUEST_BACKOFF_TIMEOUT = 2;
     // power the number of retries
@@ -81,8 +83,10 @@ public String execute() {
                     }
 
                     client.setIfModifiedSince(urlConnection);
-
+                    // set timeouts for releasing failed connections (default is 0 = no timeout).
                     urlConnection.setConnectTimeout(CONNECTION_TIMEOUT);
+                    urlConnection.setReadTimeout(READ_TIMEOUT);
+
                     urlConnection.connect();
 
                     int status = urlConnection.getResponseCode();

event-handler/src/androidTest/java/com/optimizely/ab/android/event_handler/EventClientTest.java

@@ -24,6 +24,7 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.slf4j.Logger;
 
@@ -33,10 +34,10 @@
 import java.net.HttpURLConnection;
 import java.net.URL;
 
-import static junit.framework.Assert.assertEquals;
-import static junit.framework.Assert.assertFalse;
-import static junit.framework.Assert.assertTrue;
 import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyInt;
+import static org.mockito.Matchers.contains;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -71,4 +72,23 @@ public void testEventClient() {
 
         verify(logger).debug("SendEvent completed: {}", event);
     }
+
+    @Test
+    public void testConnectionAndReadTimeout() throws IOException {
+        when(client.openConnection(event.getURL())).thenReturn(urlConnection);
+        when(urlConnection.getResponseCode()).thenReturn(200);
+        doThrow(new IOException()).when(urlConnection).getOutputStream();
+
+        eventClient.sendEvent(event);
+
+
+        ArgumentCaptor<Client.Request> captor1 = ArgumentCaptor.forClass(Client.Request.class);
+        verify(client).execute(captor1.capture(), anyInt(), anyInt());
+        Object response = captor1.getValue().execute();
+
+        verify(logger).error(contains("Unable to send event"), any(Event.class), any(IOException.class));
+        verify(urlConnection).setConnectTimeout(10*1000);
+        verify(urlConnection).setReadTimeout(60*1000);
+        verify(urlConnection).disconnect();
+    }
 }

event-handler/src/main/java/com/optimizely/ab/android/event_handler/EventClient.java

@@ -30,6 +30,10 @@
  * Makes network requests related to events
  */
 class EventClient {
+    // easy way to set the connection timeout
+    public static int CONNECTION_TIMEOUT = 10 * 1000;
+    public static int READ_TIMEOUT = 60 * 1000;
+
     private final Client client;
     // Package private and non final so it can easily be mocked for tests
     private final Logger logger;
@@ -57,6 +61,10 @@ public Boolean execute() {
                         return Boolean.FALSE;
                     }
 
+                    // set timeouts for releasing failed connections (default is 0 = no timeout).
+                    urlConnection.setConnectTimeout(CONNECTION_TIMEOUT);
+                    urlConnection.setReadTimeout(READ_TIMEOUT);
+
                     urlConnection.setRequestMethod("POST");
                     urlConnection.setRequestProperty("Content-Type", "application/json");
                     urlConnection.setDoOutput(true);