From e5e4d6e77b4faf4d62f0fccbdfec61d49d595976 Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Wed, 28 Aug 2019 09:45:56 -0700 Subject: [PATCH 1/6] fix: correct connect timeout setting for ApacheHttpRequest --- .../com/google/api/client/http/apache/v2/ApacheHttpRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-http-client-apache-v2/src/main/java/com/google/api/client/http/apache/v2/ApacheHttpRequest.java b/google-http-client-apache-v2/src/main/java/com/google/api/client/http/apache/v2/ApacheHttpRequest.java index 447edae66..7f1ddbfe2 100644 --- a/google-http-client-apache-v2/src/main/java/com/google/api/client/http/apache/v2/ApacheHttpRequest.java +++ b/google-http-client-apache-v2/src/main/java/com/google/api/client/http/apache/v2/ApacheHttpRequest.java @@ -51,7 +51,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); } From f4b7044ef5e8ea0535e634f7a4b4c8635d583dbe Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Thu, 29 Aug 2019 13:46:58 -0700 Subject: [PATCH 2/6] test: add timeout test --- .../http/apache/v2/ApacheHttpTransportTest.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java index dabd9bf3d..7d25f43e5 100644 --- a/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java +++ b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java @@ -22,6 +22,8 @@ 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 java.io.IOException; @@ -37,6 +39,7 @@ import org.apache.http.HttpVersion; import org.apache.http.client.HttpClient; import org.apache.http.client.methods.HttpUriRequest; +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; @@ -175,4 +178,18 @@ public void process(HttpRequest request, HttpContext context) } assertTrue("Expected to have called our test interceptor", interceptorCalled.get()); } + + @Test(timeout = 1000L) + public void testConnectTimeout() { + HttpTransport httpTransport = new ApacheHttpTransport(); + GenericUrl url = new GenericUrl("http://google.com:81"); + try { + httpTransport.createRequestFactory().buildGetRequest(url).setConnectTimeout(10).execute(); + fail("should have thrown an exception"); + } catch (HttpHostConnectException expected) { + // expected + } catch (IOException e) { + fail("unexpected IOException: " + e.getClass().getName()); + } + } } From 96e305c39a62248a4c243ee9e91f6a5fe8fd4b62 Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Fri, 30 Aug 2019 19:37:29 -0700 Subject: [PATCH 3/6] test: deflake test by increasing test timeout --- .../api/client/http/apache/v2/ApacheHttpTransportTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java index b79c8f3a5..c02b72e85 100644 --- a/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java +++ b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java @@ -185,7 +185,7 @@ public void process(HttpRequest request, HttpContext context) assertTrue("Expected to have called our test interceptor", interceptorCalled.get()); } - @Test(timeout = 1000L) + @Test(timeout = 2000L) public void testConnectTimeout() { HttpTransport httpTransport = new ApacheHttpTransport(); GenericUrl url = new GenericUrl("http://google.com:81"); From cd4bd67f60264fe0652fcee8a27c3c14db7448b2 Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Fri, 30 Aug 2019 20:04:26 -0700 Subject: [PATCH 4/6] test: try to deflake connect timeout test --- .../api/client/http/apache/v2/ApacheHttpTransportTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java index c02b72e85..5989caa9e 100644 --- a/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java +++ b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java @@ -185,12 +185,12 @@ public void process(HttpRequest request, HttpContext context) assertTrue("Expected to have called our test interceptor", interceptorCalled.get()); } - @Test(timeout = 2000L) + @Test(timeout = 10_000L) public void testConnectTimeout() { HttpTransport httpTransport = new ApacheHttpTransport(); GenericUrl url = new GenericUrl("http://google.com:81"); try { - httpTransport.createRequestFactory().buildGetRequest(url).setConnectTimeout(10).execute(); + httpTransport.createRequestFactory().buildGetRequest(url).setConnectTimeout(5).execute(); fail("should have thrown an exception"); } catch (HttpHostConnectException expected) { // expected From 9f8749a85370bd193b5561ed0184afcac7c78a0b Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Fri, 30 Aug 2019 20:28:08 -0700 Subject: [PATCH 5/6] test: try a higher timeout --- .../api/client/http/apache/v2/ApacheHttpTransportTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java index 5989caa9e..f5c1fa42a 100644 --- a/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java +++ b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java @@ -190,7 +190,7 @@ public void testConnectTimeout() { HttpTransport httpTransport = new ApacheHttpTransport(); GenericUrl url = new GenericUrl("http://google.com:81"); try { - httpTransport.createRequestFactory().buildGetRequest(url).setConnectTimeout(5).execute(); + httpTransport.createRequestFactory().buildGetRequest(url).setConnectTimeout(100).execute(); fail("should have thrown an exception"); } catch (HttpHostConnectException expected) { // expected From cc2bbb818a2efb16990043a29b04ca5cd924499e Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Tue, 3 Sep 2019 09:15:29 -0700 Subject: [PATCH 6/6] test: skip timeout test on windows --- .../http/apache/v2/ApacheHttpTransportTest.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java index f5c1fa42a..e6ca850ce 100644 --- a/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java +++ b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java @@ -18,6 +18,7 @@ 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; @@ -45,6 +46,7 @@ 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; @@ -187,12 +189,15 @@ public void process(HttpRequest request, HttpContext context) @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 expected) { + } catch (HttpHostConnectException | ConnectTimeoutException expected) { // expected } catch (IOException e) { fail("unexpected IOException: " + e.getClass().getName()); @@ -227,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"); + } }