You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "rschmitt (via GitHub)" <gi...@apache.org> on 2025/06/03 19:16:21 UTC
[PR] Add integration tests for connection timeouts [httpcomponents-client]
rschmitt opened a new pull request, #640:
URL: https://github.com/apache/httpcomponents-client/pull/640
This change adds basic integration test coverage for connection timeouts for the sync and async clients. The tests work by sending a request to a non-routable IP address, with timeouts configured two different ways (`ConnectionConfig` and `RequestConfig`), using both HTTP and HTTPS. The connection request gets blackholed pretty reliably, and then the tests assert that we actually waited a reasonable length of time with respect to the configured timeout. Finally, there are some client-specific assertions for the type and content of the exception thrown.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Add integration tests for connection timeouts [httpcomponents-client]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #640:
URL: https://github.com/apache/httpcomponents-client/pull/640#discussion_r2126500077
##########
httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionTimeouts.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation. For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+
+package org.apache.hc.client5.testing.sync;
+
+import org.apache.hc.client5.http.ConnectTimeoutException;
+import org.apache.hc.client5.http.classic.HttpClient;
+import org.apache.hc.client5.http.classic.methods.HttpGet;
+import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase;
+import org.apache.hc.client5.http.config.ConnectionConfig;
+import org.apache.hc.client5.http.config.RequestConfig;
+import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
+import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
+import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
+import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
+import org.apache.hc.core5.http.ClassicHttpRequest;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+
+import static java.lang.String.format;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@SuppressWarnings("deprecation")
+class TestConnectionTimeouts {
+ private static final Duration EXPECTED_TIMEOUT = Duration.ofMillis(500);
+
+ @Timeout(5)
+ @ParameterizedTest
+ @ValueSource(strings = { "http", "https" })
+ void testRequestConfig(final String scheme) throws Exception {
+ try (final CloseableHttpClient client = HttpClientBuilder.create()
+ .setDefaultRequestConfig(RequestConfig.custom()
+ .setConnectTimeout(EXPECTED_TIMEOUT.toMillis(), MILLISECONDS)
+ .build())
+ .build()) {
+ assertTimeout(getRequest(scheme), client);
+ }
+ }
+
+ @Timeout(5)
+ @ParameterizedTest
+ @ValueSource(strings = { "http", "https" })
+ void testConnectionConfig(final String scheme) throws Exception {
+ final PoolingHttpClientConnectionManager connMgr = PoolingHttpClientConnectionManagerBuilder.create()
+ .setDefaultConnectionConfig(
+ ConnectionConfig.custom()
+ .setConnectTimeout(EXPECTED_TIMEOUT.toMillis(), MILLISECONDS)
+ .build())
+ .build();
+ try (final CloseableHttpClient client = HttpClientBuilder.create().setConnectionManager(connMgr).build()) {
+ assertTimeout(getRequest(scheme), client);
+ }
+ }
+
+ private static HttpUriRequestBase getRequest(final String scheme) {
+ return new HttpGet(scheme + "://198.51.100.1/ping");
+ }
+
+ private static void assertTimeout(final ClassicHttpRequest request, final HttpClient client) {
+ final long startTime = System.nanoTime();
+ final ConnectTimeoutException ex = assertThrows(ConnectTimeoutException.class,
+ () -> client.execute(request));
+ final Duration actualTime = Duration.of(System.nanoTime() - startTime, ChronoUnit.NANOS);
+ assertTrue(actualTime.toMillis() > EXPECTED_TIMEOUT.toMillis() / 2,
+ format("Connection attempt timed out too soon (only %,d out of %,d ms)",
+ actualTime.toMillis(),
+ EXPECTED_TIMEOUT.toMillis()));
+ assertTrue(actualTime.toMillis() < EXPECTED_TIMEOUT.toMillis() * 2,
+ format("Connection attempt timed out too late (%,d out of %,d ms)",
+ actualTime.toMillis(),
+ EXPECTED_TIMEOUT.toMillis()));
+ // Capitalization (`connect` vs `Connect`) varies by Java version
+ final String message = ex.getMessage();
Review Comment:
@rschmitt `#toLowerCase` maybe?
##########
httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncConnectionTimeouts.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation. For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+
+package org.apache.hc.client5.testing.async;
+
+import org.apache.hc.client5.http.ConnectTimeoutException;
+import org.apache.hc.client5.http.async.methods.SimpleHttpRequest;
+import org.apache.hc.client5.http.config.ConnectionConfig;
+import org.apache.hc.client5.http.config.RequestConfig;
+import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient;
+import org.apache.hc.client5.http.impl.async.HttpAsyncClientBuilder;
+import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager;
+import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder;
+import org.apache.hc.core5.reactor.IOReactorConfig;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.concurrent.ExecutionException;
+
+import static java.lang.String.format;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static org.apache.hc.core5.util.TimeValue.ofMilliseconds;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@SuppressWarnings("deprecation")
+class TestAsyncConnectionTimeouts {
+ private static final Duration EXPECTED_TIMEOUT = Duration.ofMillis(500);
+
+ @Timeout(5)
+ @ParameterizedTest
+ @ValueSource(strings = { "http", "https" })
+ void testRequestConfig(final String scheme) throws Exception {
+ try (
+ final CloseableHttpAsyncClient client = HttpAsyncClientBuilder.create()
+ .setIOReactorConfig(IOReactorConfig.custom().setSelectInterval(ofMilliseconds(50)).build())
+ .setDefaultRequestConfig(RequestConfig.custom()
+ .setConnectTimeout(EXPECTED_TIMEOUT.toMillis(), MILLISECONDS)
+ .build())
+ .build()
+ ) {
+ warmUpClient(client);
+ assertTimeout(getRequest(scheme), client);
+ }
+ }
+
+ @Timeout(5)
+ @ParameterizedTest
+ @ValueSource(strings = { "http", "https" })
+ void testConnectionConfig(final String scheme) throws Exception {
+ final PoolingAsyncClientConnectionManager connMgr = PoolingAsyncClientConnectionManagerBuilder.create()
+ .setDefaultConnectionConfig(
+ ConnectionConfig.custom()
+ .setConnectTimeout(EXPECTED_TIMEOUT.toMillis(), MILLISECONDS)
+ .build())
+ .build();
+ try (
+ final CloseableHttpAsyncClient client = HttpAsyncClientBuilder.create()
+ .setIOReactorConfig(IOReactorConfig.custom().setSelectInterval(ofMilliseconds(50)).build())
+ .setConnectionManager(connMgr)
+ .build()
+ ) {
+ warmUpClient(client);
+ assertTimeout(getRequest(scheme), client);
+ }
+ }
+
+ private static void warmUpClient(final CloseableHttpAsyncClient client) {
+ client.start();
+ }
+
+ private static SimpleHttpRequest getRequest(final String scheme) {
+ return SimpleHttpRequest.create("GET", scheme + "://198.51.100.1/ping");
+ }
+
+ private static void assertTimeout(final SimpleHttpRequest request, final CloseableHttpAsyncClient client) {
+ final long startTime = System.nanoTime();
+ final Throwable ex = assertThrows(ExecutionException.class, () -> client.execute(request, null).get())
Review Comment:
@rschmitt Is there a particular reason for using a deprecated method here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Add integration tests for connection timeouts [httpcomponents-client]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #640:
URL: https://github.com/apache/httpcomponents-client/pull/640#issuecomment-2936932290
Odd that the tests fail only on macOS.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Add integration tests for connection timeouts [httpcomponents-client]
Posted by "rschmitt (via GitHub)" <gi...@apache.org>.
rschmitt commented on PR #640:
URL: https://github.com/apache/httpcomponents-client/pull/640#issuecomment-2940919155
https://github.com/apache/httpcomponents-client/pull/642
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Add integration tests for connection timeouts [httpcomponents-client]
Posted by "rschmitt (via GitHub)" <gi...@apache.org>.
rschmitt commented on PR #640:
URL: https://github.com/apache/httpcomponents-client/pull/640#issuecomment-2936989548
They do? I _developed_ them on macOS, and my `/etc/hosts` file doesn't have an entry for the address I'm using.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Add integration tests for connection timeouts [httpcomponents-client]
Posted by "rschmitt (via GitHub)" <gi...@apache.org>.
rschmitt commented on PR #640:
URL: https://github.com/apache/httpcomponents-client/pull/640#issuecomment-2937082576
Those are JDK17/JDK21 failures occasioned by a change in capitalization. JDK8 and JDK11 report `connect timed out`, while JDK17 and JDK21 report `Connect timed out`.
There are some cases in integration testing where there are significant differences between Linux and macOS, such as the default loopback addresses, but to my knowledge this PR isn't affected by them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Add integration tests for connection timeouts [httpcomponents-client]
Posted by "rschmitt (via GitHub)" <gi...@apache.org>.
rschmitt merged PR #640:
URL: https://github.com/apache/httpcomponents-client/pull/640
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Add integration tests for connection timeouts [httpcomponents-client]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #640:
URL: https://github.com/apache/httpcomponents-client/pull/640#issuecomment-2940860203
We need Windows tests in the CI I think:
```
[ERROR] Tests run: 13, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.043 s <<< FAILURE! -- in org.apache.hc.client5.http.TestHttpRoute
[ERROR] org.apache.hc.client5.http.TestHttpRoute.testUnixDomainSocketModeling -- Time elapsed: 0.015 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: </var/run/docker.sock->{}->[http://target1.test.invalid:80]> but was: <\var\run\docker.sock->{}->[http://target1.test.invalid:80]>
at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
at org.apache.hc.client5.http.TestHttpRoute.testUnixDomainSocketModeling(TestHttpRoute.java:469)
at java.base/java.lang.reflect.Method.invoke(Method.java:580)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
```
I have Rancher Desktop 1.18.2 installed if that matters.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Add integration tests for connection timeouts [httpcomponents-client]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #640:
URL: https://github.com/apache/httpcomponents-client/pull/640#issuecomment-2937066143
I saw these failures:
https://github.com/apache/httpcomponents-client/actions/runs/15425891897
We're they random?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Add integration tests for connection timeouts [httpcomponents-client]
Posted by "rschmitt (via GitHub)" <gi...@apache.org>.
rschmitt commented on PR #640:
URL: https://github.com/apache/httpcomponents-client/pull/640#issuecomment-2940910411
Windows CI builds were disabled in 2022 due to "intermittent network failures." I'm not sure what that means. Do any of these tests talk to the internet?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Add integration tests for connection timeouts [httpcomponents-client]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #640:
URL: https://github.com/apache/httpcomponents-client/pull/640#issuecomment-2940952494
> Windows CI builds were disabled in 2022 due to "intermittent network failures." I'm not sure what that means. Do any of these tests talk to the internet?
@rschmitt @garydgregory That means that some integration tests fail intermittently with connection reset for no obvious reasons on Windows only. The same tests always pass on Ubuntu and MacOS. I tried really hard to find the cause of those failures to no avail. I will have to disable tests on Windows again if the problem recurs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org