fix: correct connect timeout setting for ApacheHttpRequest (#803)

* fix: correct connect timeout setting for ApacheHttpRequest * test: add timeout test * test: deflake test by increasing test timeout * test: try to deflake connect timeout test * test: try a higher timeout * test: skip timeout test on windows
googleapis · Sep 4, 2019 · df21ebc · df21ebc
1 parent 5862e7d
commit df21ebc
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 1 deletion.
diff --git a/...lient-apache-v2/src/main/java/com/google/api/client/http/apache/v2/ApacheHttpRequest.java b/...lient-apache-v2/src/main/java/com/google/api/client/http/apache/v2/ApacheHttpRequest.java
@@ -52,7 +52,7 @@ public void addHeader(String name, String value) {
 
   @Override
   public void setTimeout(int connectTimeout, int readTimeout) throws IOException {
-    requestConfig.setConnectionRequestTimeout(connectTimeout)
+    requestConfig.setConnectTimeout(connectTimeout)
         .setSocketTimeout(readTimeout);
   }
 

diff --git a/...apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java b/...apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java
@@ -18,11 +18,13 @@
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import com.google.api.client.http.GenericUrl;
+import com.google.api.client.http.HttpTransport;
 import com.google.api.client.http.LowLevelHttpResponse;
 import com.google.api.client.util.ByteArrayStreamingContent;
 import com.sun.net.httpserver.HttpExchange;
@@ -44,6 +46,8 @@
 import org.apache.http.HttpVersion;
 import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.conn.ConnectTimeoutException;
+import org.apache.http.conn.HttpHostConnectException;
 import org.apache.http.impl.client.HttpClients;
 import org.apache.http.message.BasicHttpResponse;
 import org.apache.http.protocol.HttpContext;
@@ -183,6 +187,23 @@ public void process(HttpRequest request, HttpContext context)
     assertTrue("Expected to have called our test interceptor", interceptorCalled.get());
   }
 
+  @Test(timeout = 10_000L)
+  public void testConnectTimeout() {
+    // Apache HttpClient doesn't appear to behave correctly on windows
+    assumeTrue(!isWindows());
+
+    HttpTransport httpTransport = new ApacheHttpTransport();
+    GenericUrl url = new GenericUrl("http://google.com:81");
+    try {
+      httpTransport.createRequestFactory().buildGetRequest(url).setConnectTimeout(100).execute();
+      fail("should have thrown an exception");
+    } catch (HttpHostConnectException | ConnectTimeoutException expected) {
+      // expected
+    } catch (IOException e) {
+      fail("unexpected IOException: " + e.getClass().getName());
+    }
+  }
+
   @Test
   public void testNormalizedUrl() throws IOException {
     HttpServer server = HttpServer.create(new InetSocketAddress(0), 0);
@@ -211,4 +232,8 @@ public void handle(HttpExchange httpExchange) throws IOException {
     assertEquals(200, response.getStatusCode());
     assertEquals("/foo//bar", response.parseAsString());
   }
+
+  private boolean isWindows() {
+    return System.getProperty("os.name").startsWith("Windows");
+  }
 }