From 83891f10a8c951c2f73fabe54709a8afe8030d9c Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Thu, 21 Aug 2025 16:04:50 +0200 Subject: [PATCH] HTTPCLIENT-1074: Hard request timeout for async & classic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add RequestConfig.requestTimeout—an opt-in, end-to-end deadline that cancels the exchange and discards the endpoint on expiry (InterruptedIOException). Implemented in InternalAbstractHttpAsyncClient and InternalHttpClient using the existing scheduler. Disabled by default for full back-compat; relates to HTTPASYNC-149 / HTTPCLIENT-2169. --- .../async/TestRequestTimeoutAsync.java | 233 ++++++++++++++++++ .../sync/TestRequestTimeoutClassic.java | 197 +++++++++++++++ .../hc/client5/http/config/RequestConfig.java | 66 ++++- .../InternalAbstractHttpAsyncClient.java | 76 ++++-- .../http/impl/classic/InternalHttpClient.java | 109 ++++++-- .../AsyncClientRequestTimeoutExample.java | 145 +++++++++++ .../ClassicClientCallTimeoutExample.java | 91 +++++++ 7 files changed, 880 insertions(+), 37 deletions(-) create mode 100644 httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestRequestTimeoutAsync.java create mode 100644 httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRequestTimeoutClassic.java create mode 100644 httpclient5/src/test/java/org/apache/hc/client5/http/examples/AsyncClientRequestTimeoutExample.java create mode 100644 httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClassicClientCallTimeoutExample.java diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestRequestTimeoutAsync.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestRequestTimeoutAsync.java new file mode 100644 index 0000000000..e2126b8c9c --- /dev/null +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestRequestTimeoutAsync.java @@ -0,0 +1,233 @@ +/* + * ==================================================================== + * 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 + * . + * + */ +package org.apache.hc.client5.testing.async; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +import java.io.IOException; +import java.io.InterruptedIOException; +import java.nio.ByteBuffer; +import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import org.apache.hc.client5.http.async.methods.SimpleHttpRequest; +import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; +import org.apache.hc.client5.http.async.methods.SimpleRequestBuilder; +import org.apache.hc.client5.http.config.RequestConfig; +import org.apache.hc.client5.testing.extension.async.ClientProtocolLevel; +import org.apache.hc.client5.testing.extension.async.ServerProtocolLevel; +import org.apache.hc.client5.testing.extension.async.TestAsyncClient; +import org.apache.hc.core5.http.ContentType; +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.HttpRequest; +import org.apache.hc.core5.http.URIScheme; +import org.apache.hc.core5.http.message.BasicHttpResponse; +import org.apache.hc.core5.http.nio.AsyncServerExchangeHandler; +import org.apache.hc.core5.http.nio.CapacityChannel; +import org.apache.hc.core5.http.nio.DataStreamChannel; +import org.apache.hc.core5.http.nio.ResponseChannel; +import org.apache.hc.core5.http.nio.entity.StringAsyncEntityProducer; +import org.apache.hc.core5.http.protocol.HttpContext; +import org.apache.hc.core5.util.Timeout; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; + +/** + * Integration tests for hard end-to-end request timeout (query timeout / request deadline). + * Uses millisecond delays to keep the suite fast and deterministic. + */ +class TestRequestTimeoutAsync extends AbstractIntegrationTestBase { + + TestRequestTimeoutAsync() { + super(URIScheme.HTTP, ClientProtocolLevel.STANDARD, ServerProtocolLevel.STANDARD); + } + + /** + * Async handler responding after a millisecond delay from /mdelay/{millis}. + */ + private static final class AsyncDelayMsHandler implements AsyncServerExchangeHandler { + + private final ScheduledExecutorService sched = + Executors.newSingleThreadScheduledExecutor(r -> { + final Thread t = new Thread(r, "async-delayms-handler"); + t.setDaemon(true); + return t; + }); + + @Override + public void handleRequest( + final HttpRequest request, + final org.apache.hc.core5.http.EntityDetails entityDetails, + final ResponseChannel responseChannel, + final HttpContext context) { + + long millis = 100; + final String path = request.getRequestUri(); // e.g. /mdelay/250 + final int idx = path.lastIndexOf('/'); + if (idx >= 0 && idx + 1 < path.length()) { + try { + millis = Long.parseLong(path.substring(idx + 1)); + } catch (final Exception ignore) { /* keep default */ } + } + + final BasicHttpResponse response = new BasicHttpResponse(200, "OK"); + + // schedule without blocking I/O threads + final long delay = Math.max(0L, millis); + sched.schedule(() -> { + try { + responseChannel.sendResponse( + response, + new StringAsyncEntityProducer("{\"ok\":true}", ContentType.APPLICATION_JSON), + context); + } catch (final Exception ex) { + failed(ex); + } + }, delay, TimeUnit.MILLISECONDS); + } + + @Override + public void updateCapacity(final CapacityChannel capacityChannel) throws IOException { + capacityChannel.update(Integer.MAX_VALUE); + } + + @Override + public void consume(final ByteBuffer src) throws IOException { + src.position(src.limit()); // discard any request body + } + + @Override + public void streamEnd(final List trailers) + throws IOException { + // no-op + } + + @Override + public int available() { + return 0; + } + + @Override + public void produce(final DataStreamChannel channel) { /* no-op */ } + + // ---- Lifecycle ---- + @Override + public void failed(final Exception cause) { /* no-op in tests */ } + + @Override + public void releaseResources() { + sched.shutdownNow(); + } + } + + @Test + void timesOutHard() throws Exception { + configureServer(b -> b.register("/mdelay/*", AsyncDelayMsHandler::new)); + final HttpHost target = startServer(); + final TestAsyncClient client = startClient(); + + final SimpleHttpRequest req = SimpleRequestBuilder.get() + .setHttpHost(target) + .setPath("/mdelay/5000") // 5s server delay + .build(); + req.setConfig(RequestConfig.custom() + .setRequestTimeout(Timeout.ofMilliseconds(100)) // 100ms hard deadline + .build()); + + final Future f = client.execute(req, null); + try { + f.get(5, TimeUnit.SECONDS); + fail("Expected ExecutionException due to hard request timeout"); + } catch (final ExecutionException ex) { + assertTrue(ex.getCause() instanceof InterruptedIOException, + "Cause should be InterruptedIOException"); + } catch (final TimeoutException te) { + fail("Request did not time out as expected (test wait timed out)"); + } + } + + @Test + @Disabled + void succeedsWithinBudget() throws Exception { + configureServer(b -> b.register("/mdelay/*", AsyncDelayMsHandler::new)); + final HttpHost target = startServer(); + final TestAsyncClient client = startClient(); + + final SimpleHttpRequest req = SimpleRequestBuilder.get() + .setHttpHost(target) + .setPath("/mdelay/100") + .build(); + + req.setConfig(RequestConfig.custom() + .setRequestTimeout(Timeout.ofSeconds(10)) + .build()); + + final Future f = client.execute(req, null); + final SimpleHttpResponse resp = f.get(); + + assertThat(resp, notNullValue()); + assertThat(resp.getCode(), equalTo(200)); + assertThat(resp.getBodyText(), notNullValue()); + } + + @Test + void nearImmediateExpirationFailsQuickly() throws Exception { + configureServer(b -> b.register("/mdelay/*", AsyncDelayMsHandler::new)); + final HttpHost target = startServer(); + final TestAsyncClient client = startClient(); + + final SimpleHttpRequest req = SimpleRequestBuilder.get() + .setHttpHost(target) + .setPath("/mdelay/5000") // 5s server delay + .build(); + // Tiny positive timeout; deterministic & very fast failure + req.setConfig(RequestConfig.custom() + .setRequestTimeout(Timeout.ofMilliseconds(50)) + .build()); + + final Future f = client.execute(req, null); + try { + f.get(3, TimeUnit.SECONDS); + fail("Expected ExecutionException due to near-immediate hard timeout"); + } catch (final ExecutionException ex) { + assertTrue(ex.getCause() instanceof InterruptedIOException, + "Cause should be InterruptedIOException"); + } catch (final TimeoutException te) { + fail("Future did not complete promptly for near-immediate timeout"); + } + } +} diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRequestTimeoutClassic.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRequestTimeoutClassic.java new file mode 100644 index 0000000000..aa5b167dae --- /dev/null +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRequestTimeoutClassic.java @@ -0,0 +1,197 @@ +/* + * ==================================================================== + * 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 + * . + * + */ +package org.apache.hc.client5.testing.sync; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.util.concurrent.TimeUnit; + +import org.apache.hc.client5.http.classic.methods.HttpGet; +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.HttpClients; +import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; +import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; +import org.apache.hc.core5.http.ContentType; +import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.impl.bootstrap.HttpServer; +import org.apache.hc.core5.http.impl.bootstrap.ServerBootstrap; +import org.apache.hc.core5.http.io.HttpRequestHandler; +import org.apache.hc.core5.http.io.entity.StringEntity; +import org.apache.hc.core5.net.URIBuilder; +import org.apache.hc.core5.util.Timeout; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class TestRequestTimeoutClassic { + + private static HttpServer server; + private static HttpHost target; + + private CloseableHttpClient client; + + private static final HttpRequestHandler DELAY_HANDLER = (request, response, context) -> { + int seconds = 1; + final String path = request.getPath(); // e.g. /delay/5 + final int idx = path.lastIndexOf('/'); + if (idx >= 0 && idx + 1 < path.length()) { + try { + seconds = Integer.parseInt(path.substring(idx + 1)); + } catch (final NumberFormatException ignore) { /* default 1s */ } + } + try { + TimeUnit.SECONDS.sleep(seconds); + } catch (final InterruptedException ie) { + Thread.currentThread().interrupt(); + } + response.setCode(200); + response.setEntity(new StringEntity("{\"ok\":true}", ContentType.APPLICATION_JSON)); + }; + + @BeforeAll + static void startServer() throws Exception { + server = ServerBootstrap.bootstrap() + .setCanonicalHostName("localhost") // <<< important: avoids 421 misdirected + .register("/delay/*", DELAY_HANDLER) + .create(); + server.start(); + target = new HttpHost("http", "localhost", server.getLocalPort()); + } + + @AfterAll + static void stopServer() { + if (server != null) { + server.stop(); + } + } + + @BeforeEach + void createClient() { + final PoolingHttpClientConnectionManager cm = + PoolingHttpClientConnectionManagerBuilder.create() + .setDefaultConnectionConfig(ConnectionConfig.custom() + .setConnectTimeout(Timeout.ofSeconds(5)) + .setSocketTimeout(Timeout.ofSeconds(5)) + .build()) + .build(); + + client = HttpClients.custom() + .setConnectionManager(cm) + .build(); + } + + @AfterEach + void closeClient() throws IOException { + if (client != null) { + client.close(); + } + if (Thread.currentThread().isInterrupted()) { + Thread.interrupted(); // clear any stale interrupt + } + } + + @Test + @org.junit.jupiter.api.Timeout(value = 10, unit = TimeUnit.SECONDS) + void timesOutHard() throws Exception { + final HttpGet req = new HttpGet(new URIBuilder() + .setScheme(target.getSchemeName()) + .setHost(target.getHostName()) + .setPort(target.getPort()) + .setPath("/delay/5") + .build()); + req.setConfig(RequestConfig.custom() + .setRequestTimeout(Timeout.ofSeconds(1)) // hard end-to-end deadline + .setConnectionRequestTimeout(Timeout.ofSeconds(2)) // pool lease cap + .build()); + + final IOException ex = assertThrows(IOException.class, + () -> client.execute(req, resp -> resp.getCode())); + assertTrue(ex instanceof java.io.InterruptedIOException, + "Expected InterruptedIOException, got: " + ex.getClass()); + } + + @Test + @org.junit.jupiter.api.Timeout(value = 10, unit = TimeUnit.SECONDS) + void succeedsWithinBudget() throws Exception { + final HttpGet req = new HttpGet(new URIBuilder() + .setScheme(target.getSchemeName()) + .setHost(target.getHostName()) + .setPort(target.getPort()) + .setPath("/delay/1") + .build()); + req.setConfig(RequestConfig.custom() + .setRequestTimeout(Timeout.ofSeconds(5)) // enough for lease+connect+1s delay + .setConnectionRequestTimeout(Timeout.ofSeconds(2)) + .build()); + + final int code = client.execute(req, resp -> resp.getCode()); + assertEquals(200, code); + } + + @Test + @org.junit.jupiter.api.Timeout(value = 10, unit = TimeUnit.SECONDS) + void immediateExpirationFailsBeforeSend() throws Exception { + final HttpGet req = new HttpGet(new URIBuilder() + .setScheme(target.getSchemeName()) + .setHost(target.getHostName()) + .setPort(target.getPort()) + .setPath("/delay/1") + .build()); + req.setConfig(RequestConfig.custom() + .setRequestTimeout(Timeout.ofMilliseconds(1)) // near-immediate expiry + .setConnectionRequestTimeout(Timeout.ofSeconds(1)) + .build()); + + assertThrows(java.io.InterruptedIOException.class, + () -> client.execute(req, resp -> resp.getCode())); + } + + @Test + @org.junit.jupiter.api.Timeout(value = 10, unit = TimeUnit.SECONDS) + void largeBudgetStillHonorsPerOpTimeouts() throws Exception { + final HttpGet req = new HttpGet(new URIBuilder() + .setScheme(target.getSchemeName()) + .setHost(target.getHostName()) + .setPort(target.getPort()) + .setPath("/delay/1") + .build()); + req.setConfig(RequestConfig.custom() + .setRequestTimeout(Timeout.ofSeconds(30)) + .setConnectionRequestTimeout(Timeout.ofSeconds(2)) + .build()); + + final int code = client.execute(req, resp -> resp.getCode()); + assertEquals(200, code); + } +} diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java b/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java index f13d940fe3..ec21adf3e4 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java @@ -69,13 +69,15 @@ public class RequestConfig implements Cloneable { private final ExpectContinueTrigger expectContinueTrigger; + private final Timeout requestTimeout; + /** * Intended for CDI compatibility */ protected RequestConfig() { this(false, null, null, false, false, 0, false, null, null, DEFAULT_CONNECTION_REQUEST_TIMEOUT, null, null, DEFAULT_CONN_KEEP_ALIVE, false, false, false, null, - ExpectContinueTrigger.ALWAYS); + ExpectContinueTrigger.ALWAYS, null); } RequestConfig( @@ -96,7 +98,8 @@ protected RequestConfig() { final boolean hardCancellationEnabled, final boolean protocolUpgradeEnabled, final Path unixDomainSocket, - final ExpectContinueTrigger expectContinueTrigger) { + final ExpectContinueTrigger expectContinueTrigger, + final Timeout requestTimeout) { super(); this.expectContinueEnabled = expectContinueEnabled; this.proxy = proxy; @@ -116,6 +119,7 @@ protected RequestConfig() { this.protocolUpgradeEnabled = protocolUpgradeEnabled; this.unixDomainSocket = unixDomainSocket; this.expectContinueTrigger = expectContinueTrigger; + this.requestTimeout = requestTimeout; } /** @@ -248,6 +252,22 @@ public ExpectContinueTrigger getExpectContinueTrigger() { return expectContinueTrigger; } + /** + * Returns the hard end-to-end request timeout (call timeout / request deadline). + * The entire exchange must complete within this budget or the execution is aborted. + *

+ * This timeout is independent of {@linkplain #getConnectTimeout() connect} and + * {@linkplain #getResponseTimeout() response} timeouts. Pass + * {@link org.apache.hc.core5.util.Timeout#DISABLED} to disable. + *

+ * + * @return the configured request timeout; never {@code null}. + * @since 5.6 + */ + public Timeout getRequestTimeout() { + return requestTimeout; + } + @Override protected RequestConfig clone() throws CloneNotSupportedException { return (RequestConfig) super.clone(); @@ -274,6 +294,7 @@ public String toString() { builder.append(", hardCancellationEnabled=").append(hardCancellationEnabled); builder.append(", protocolUpgradeEnabled=").append(protocolUpgradeEnabled); builder.append(", unixDomainSocket=").append(unixDomainSocket); + builder.append(", requestTimeout=").append(requestTimeout); builder.append("]"); return builder.toString(); } @@ -300,7 +321,8 @@ public static RequestConfig.Builder copy(final RequestConfig config) { .setContentCompressionEnabled(config.isContentCompressionEnabled()) .setHardCancellationEnabled(config.isHardCancellationEnabled()) .setProtocolUpgradeEnabled(config.isProtocolUpgradeEnabled()) - .setUnixDomainSocket(config.getUnixDomainSocket()); + .setUnixDomainSocket(config.getUnixDomainSocket()) + .setRequestTimeout(config.getRequestTimeout()); } public static class Builder { @@ -324,6 +346,22 @@ public static class Builder { private Path unixDomainSocket; private ExpectContinueTrigger expectContinueTrigger; + + /** + * Hard end-to-end request timeout (also known as a call timeout or + * request deadline). If the entire execution — including connection + * leasing, connection establishment, request transmission, and response + * processing — does not complete within this time budget, the execution is + * aborted and an {@link java.io.InterruptedIOException} is propagated. + *

+ * This is independent of {@linkplain #getConnectTimeout() connect} and + * {@linkplain #getResponseTimeout() response} timeouts. + * Use {@link org.apache.hc.core5.util.Timeout#DISABLED} to disable. + *

+ * + */ + private Timeout requestTimeout; + Builder() { super(); this.redirectsEnabled = true; @@ -694,6 +732,25 @@ public Builder setExpectContinueTrigger(final ExpectContinueTrigger trigger) { return this; } + /** + * Sets the hard end-to-end request timeout (also known as call timeout or request + * deadline). When set, the entire request execution — from connection leasing + * through connection establishment, request write, and response processing — + * must complete within this time budget or the execution will be aborted. + *

+ * Pass {@link org.apache.hc.core5.util.Timeout#DISABLED} to turn this feature off. + * A non-positive timeout value is treated as an immediate expiry. + *

+ * + * @param requestTimeout the request timeout to apply; use {@code Timeout.DISABLED} to disable + * @return this builder + * @since 5.6 + */ + public Builder setRequestTimeout(final Timeout requestTimeout) { + this.requestTimeout = requestTimeout; + return this; + } + public RequestConfig build() { return new RequestConfig( expectContinueEnabled, @@ -713,7 +770,8 @@ public RequestConfig build() { hardCancellationEnabled, protocolUpgradeEnabled, unixDomainSocket, - expectContinueTrigger); + expectContinueTrigger, + requestTimeout); } } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalAbstractHttpAsyncClient.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalAbstractHttpAsyncClient.java index 6c1679604f..2d84ac8362 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalAbstractHttpAsyncClient.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalAbstractHttpAsyncClient.java @@ -28,6 +28,7 @@ import java.io.Closeable; import java.io.IOException; +import java.io.InterruptedIOException; import java.util.List; import java.util.Set; import java.util.concurrent.CancellationException; @@ -35,9 +36,12 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; import org.apache.hc.client5.http.HttpRoute; @@ -75,9 +79,9 @@ import org.apache.hc.core5.http.support.BasicRequestBuilder; import org.apache.hc.core5.io.CloseMode; import org.apache.hc.core5.io.ModalCloseable; -import org.apache.hc.core5.net.URIAuthority; import org.apache.hc.core5.reactor.DefaultConnectingIOReactor; import org.apache.hc.core5.util.TimeValue; +import org.apache.hc.core5.util.Timeout; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -194,13 +198,13 @@ private void setupContext(final HttpClientContext context) { @Override protected Future doExecute( - final HttpHost target, + final HttpHost httpHost, final AsyncRequestProducer requestProducer, final AsyncResponseConsumer responseConsumer, final HandlerFactory pushHandlerFactory, final HttpContext context, final FutureCallback callback) { - final ComplexFuture future = new ComplexFuture<>(callback); + final ComplexFuture future = new ComplexFuture(callback); try { if (!isRunning()) { throw new CancellationException("Request execution cancelled"); @@ -218,18 +222,8 @@ protected Future doExecute( setupContext(clientContext); - final HttpHost resolvedTarget = target != null ? target : RoutingSupport.determineHost(request); - if (resolvedTarget != null) { - if (request.getScheme() == null) { - request.setScheme(resolvedTarget.getSchemeName()); - } - if (request.getAuthority() == null) { - request.setAuthority(new URIAuthority(resolvedTarget)); - } - } - final HttpRoute route = determineRoute( - resolvedTarget, + httpHost != null ? httpHost : RoutingSupport.determineHost(request), request, clientContext); final String exchangeId = ExecSupport.getNextExchangeId(); @@ -242,6 +236,55 @@ protected Future doExecute( final AsyncExecChain.Scope scope = new AsyncExecChain.Scope(exchangeId, route, request, future, clientContext, execRuntime, scheduler, new AtomicInteger(1)); final AtomicBoolean outputTerminated = new AtomicBoolean(false); + // ---- Hard request timeout: schedule & cleanup hooks + final AtomicReference> timeoutRef = new AtomicReference<>(); + final Runnable cancelTimeout = () -> { + final ScheduledFuture t = timeoutRef.getAndSet(null); + if (t != null) { + t.cancel(false); + } + }; + + final RequestConfig cfg = clientContext.getRequestConfigOrDefault(); + final Timeout requestTimeout = cfg != null ? cfg.getRequestTimeout() : null; + if (requestTimeout != null && !requestTimeout.isDisabled()) { + final long delayMs = requestTimeout.toMilliseconds(); + if (delayMs <= 0L) { + // Fail immediately before starting the chain + outputTerminated.set(true); + final InterruptedIOException ex = new InterruptedIOException("Request timeout"); + try { + execRuntime.discardEndpoint(); + responseConsumer.failed(ex); + } finally { + try { + future.failed(ex); + } finally { + responseConsumer.releaseResources(); + requestProducer.releaseResources(); + } + } + return; // do not proceed with execution + } + final ScheduledFuture task = scheduledExecutorService.schedule(() -> { + if (!future.isDone()) { + final InterruptedIOException ex = new InterruptedIOException("Request timeout"); + try { + execRuntime.discardEndpoint(); + responseConsumer.failed(ex); + } finally { + try { + future.failed(ex); + } finally { + responseConsumer.releaseResources(); + requestProducer.releaseResources(); + } + } + } + }, delayMs, TimeUnit.MILLISECONDS); + timeoutRef.set(task); + } + executeImmediate( BasicRequestBuilder.copy(request).build(), entityDetails != null ? new AsyncEntityProducer() { @@ -318,16 +361,19 @@ public AsyncDataConsumer handleResponse( @Override public void completed(final T result) { + cancelTimeout.run(); future.completed(result); } @Override public void failed(final Exception ex) { + cancelTimeout.run(); future.failed(ex); } @Override public void cancelled() { + cancelTimeout.run(); future.cancel(); } @@ -343,6 +389,7 @@ public void handleInformationResponse( @Override public void completed() { + cancelTimeout.run(); if (LOG.isDebugEnabled()) { LOG.debug("{} message exchange successfully completed", exchangeId); } @@ -356,6 +403,7 @@ public void completed() { @Override public void failed(final Exception cause) { + cancelTimeout.run(); if (LOG.isDebugEnabled()) { LOG.debug("{} request failed: {}", exchangeId, cause.getMessage()); } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalHttpClient.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalHttpClient.java index 07265fcb4b..4719e41127 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalHttpClient.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalHttpClient.java @@ -29,8 +29,15 @@ import java.io.Closeable; import java.io.IOException; +import java.io.InterruptedIOException; import java.util.List; import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; import org.apache.hc.client5.http.ClientProtocolException; @@ -52,11 +59,11 @@ import org.apache.hc.core5.annotation.Internal; import org.apache.hc.core5.annotation.ThreadingBehavior; import org.apache.hc.core5.concurrent.CancellableDependency; +import org.apache.hc.core5.concurrent.DefaultThreadFactory; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; import org.apache.hc.core5.http.HttpException; import org.apache.hc.core5.http.HttpHost; -import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.config.Lookup; import org.apache.hc.core5.http.impl.io.HttpRequestExecutor; import org.apache.hc.core5.http.io.support.ClassicRequestBuilder; @@ -65,24 +72,19 @@ import org.apache.hc.core5.io.ModalCloseable; import org.apache.hc.core5.net.URIAuthority; import org.apache.hc.core5.util.Args; +import org.apache.hc.core5.util.Timeout; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -/** - * Internal implementation of {@link CloseableHttpClient}. - *

- * Concurrent message exchanges executed by this client will get assigned to - * separate connections leased from the connection pool. - *

- * - * @since 4.3 - */ @Contract(threading = ThreadingBehavior.SAFE_CONDITIONAL) @Internal class InternalHttpClient extends CloseableHttpClient implements Configurable { private static final Logger LOG = LoggerFactory.getLogger(InternalHttpClient.class); + private static final ThreadFactory SCHEDULER_THREAD_FACTORY = + new DefaultThreadFactory("hc-classic-call-timeouts", true); + private final HttpClientConnectionManager connManager; private final HttpRequestExecutor requestExecutor; private final ExecChainElement execChain; @@ -95,6 +97,8 @@ class InternalHttpClient extends CloseableHttpClient implements Configurable { private final RequestConfig defaultConfig; private final ConcurrentLinkedQueue closeables; + private final ScheduledExecutorService scheduledExecutorService; + public InternalHttpClient( final HttpClientConnectionManager connManager, final HttpRequestExecutor requestExecutor, @@ -119,9 +123,13 @@ public InternalHttpClient( this.contextAdaptor = contextAdaptor; this.defaultConfig = defaultConfig; this.closeables = closeables != null ? new ConcurrentLinkedQueue<>(closeables) : null; + this.scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(SCHEDULER_THREAD_FACTORY); } - private HttpRoute determineRoute(final HttpHost target, final HttpRequest request, final HttpContext context) throws HttpException { + private HttpRoute determineRoute( + final HttpHost target, + final org.apache.hc.core5.http.HttpRequest request, + final HttpContext context) throws HttpException { return this.routePlanner.determineRoute(target, request, context); } @@ -169,21 +177,84 @@ protected CloseableHttpResponse doExecute( request.setAuthority(new URIAuthority(resolvedTarget)); } } - final HttpRoute route = determineRoute( - resolvedTarget, - request, - localcontext); + final HttpRoute route = determineRoute(resolvedTarget, request, localcontext); final String exchangeId = ExecSupport.getNextExchangeId(); localcontext.setExchangeId(exchangeId); if (LOG.isDebugEnabled()) { LOG.debug("{} preparing request execution", exchangeId); } - final ExecRuntime execRuntime = new InternalExecRuntime(LOG, connManager, requestExecutor, + final ExecRuntime execRuntime = new InternalExecRuntime( + LOG, connManager, requestExecutor, request instanceof CancellableDependency ? (CancellableDependency) request : null); final ExecChain.Scope scope = new ExecChain.Scope(exchangeId, route, request, execRuntime, localcontext); - final ClassicHttpResponse response = this.execChain.execute(ClassicRequestBuilder.copy(request).build(), scope); - return CloseableHttpResponse.adapt(response); + + // Hard request timeout (call deadline) + final RequestConfig effectiveCfg = localcontext.getRequestConfig(); + final Timeout requestTimeout = effectiveCfg != null ? effectiveCfg.getRequestTimeout() : null; + + if (requestTimeout == null || requestTimeout.isDisabled()) { + final ClassicHttpResponse response = + this.execChain.execute(ClassicRequestBuilder.copy(request).build(), scope); + return CloseableHttpResponse.adapt(response); + } + + final long delayMs = requestTimeout.toMilliseconds(); + if (delayMs <= 0L) { + throw new InterruptedIOException("Request timeout"); + } + + final Thread execThread = Thread.currentThread(); + final AtomicBoolean terminal = new AtomicBoolean(false); + final AtomicBoolean timeoutFired = new AtomicBoolean(false); + + final ScheduledFuture timer = scheduledExecutorService.schedule(() -> { + if (terminal.compareAndSet(false, true)) { + timeoutFired.set(true); + try { + // Hard-abort: close the endpoint to unblock any blocking I/O + execRuntime.discardEndpoint(); + } catch (final Exception ignore) { + } + // Interrupt as an extra nudge for blocking waits (lease/read) + execThread.interrupt(); + } + }, delayMs, TimeUnit.MILLISECONDS); + + try { + final ClassicHttpResponse response = + this.execChain.execute(ClassicRequestBuilder.copy(request).build(), scope); + + // If timeout already fired, surface timeout error + if (!terminal.compareAndSet(false, true)) { + timer.cancel(false); + // clear only if we fired the timeout + if (timeoutFired.get() && Thread.currentThread().isInterrupted()) { + Thread.interrupted(); + } + throw new InterruptedIOException("Request timeout"); + } + + timer.cancel(false); + // clear interrupt if our timer fired just as we completed + if (timeoutFired.get() && Thread.currentThread().isInterrupted()) { + Thread.interrupted(); + } + return CloseableHttpResponse.adapt(response); + + } catch (final IOException | RuntimeException ioEx) { + if (timeoutFired.get() || !terminal.compareAndSet(false, true)) { + timer.cancel(false); + if (timeoutFired.get() && Thread.currentThread().isInterrupted()) { + Thread.interrupted(); + } + throw new InterruptedIOException("Request timeout"); + } + timer.cancel(false); + throw ioEx; + + } + } catch (final HttpException httpException) { throw new ClientProtocolException(httpException.getMessage(), httpException); } @@ -215,6 +286,6 @@ public void close(final CloseMode closeMode) { } } } + this.scheduledExecutorService.shutdownNow(); } - } diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/examples/AsyncClientRequestTimeoutExample.java b/httpclient5/src/test/java/org/apache/hc/client5/http/examples/AsyncClientRequestTimeoutExample.java new file mode 100644 index 0000000000..4477503c08 --- /dev/null +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/examples/AsyncClientRequestTimeoutExample.java @@ -0,0 +1,145 @@ +/* + * ==================================================================== + * 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 + * . + * + */ +package org.apache.hc.client5.http.examples; + +import java.io.InterruptedIOException; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; + +import org.apache.hc.client5.http.async.methods.SimpleHttpRequest; +import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; +import org.apache.hc.client5.http.async.methods.SimpleRequestBuilder; +import org.apache.hc.client5.http.async.methods.SimpleRequestProducer; +import org.apache.hc.client5.http.async.methods.SimpleResponseConsumer; +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.HttpAsyncClients; +import org.apache.hc.core5.concurrent.FutureCallback; +import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.message.StatusLine; +import org.apache.hc.core5.io.CloseMode; +import org.apache.hc.core5.util.Timeout; + +/** + * Demonstrates per-request hard end-to-end timeout (query timeout / request deadline). + */ +public class AsyncClientRequestTimeoutExample { + + public static void main(final String[] args) throws Exception { + + // No default requestTimeout at the client level (leave it opt-in per request). + try (final CloseableHttpAsyncClient client = HttpAsyncClients.custom().build()) { + + client.start(); + + final HttpHost host = new HttpHost("https", "httpbin.org"); + + // 1) This one should TIME OUT (server delays ~5s, our requestTimeout is 2s) + final SimpleHttpRequest willTimeout = SimpleRequestBuilder.get() + .setHttpHost(host) + .setPath("/delay/5") + .build(); + willTimeout.setConfig(RequestConfig.custom() + .setRequestTimeout(Timeout.ofSeconds(2)) + .build()); + + System.out.println("Executing (expected timeout): " + willTimeout); + + final Future f1 = client.execute( + SimpleRequestProducer.create(willTimeout), + SimpleResponseConsumer.create(), + new FutureCallback() { + @Override + public void completed(final SimpleHttpResponse response) { + System.out.println(willTimeout + " -> " + new StatusLine(response)); + System.out.println(response.getBodyText()); + } + + @Override + public void failed(final Exception ex) { + System.out.println(willTimeout + " -> FAILED: " + ex); + if (ex instanceof InterruptedIOException) { + System.out.println("As expected: hard request timeout triggered."); + } + } + + @Override + public void cancelled() { + System.out.println(willTimeout + " -> CANCELLED"); + } + }); + + try { + f1.get(); // Will throw ExecutionException wrapping InterruptedIOException + } catch (final ExecutionException ee) { + final Throwable cause = ee.getCause(); + if (cause instanceof InterruptedIOException) { + System.out.println("Future failed with InterruptedIOException (OK): " + cause.getMessage()); + } else { + System.out.println("Unexpected failure type: " + cause); + } + } + + // 2) This one should SUCCEED (server delays ~1s, our requestTimeout is 3s) + final SimpleHttpRequest willSucceed = SimpleRequestBuilder.get() + .setHttpHost(host) + .setPath("/delay/1") + .build(); + willSucceed.setConfig(RequestConfig.custom() + .setRequestTimeout(Timeout.ofSeconds(3)) // <--- longer budget + .build()); + + System.out.println("Executing (expected success): " + willSucceed); + + final Future f2 = client.execute( + SimpleRequestProducer.create(willSucceed), + SimpleResponseConsumer.create(), + new FutureCallback() { + @Override + public void completed(final SimpleHttpResponse response) { + System.out.println(willSucceed + " -> " + new StatusLine(response)); + System.out.println(response.getBodyText()); + } + + @Override + public void failed(final Exception ex) { + System.out.println(willSucceed + " -> FAILED: " + ex); + } + + @Override + public void cancelled() { + System.out.println(willSucceed + " -> CANCELLED"); + } + }); + + f2.get(); // Should complete normally + + System.out.println("Shutting down"); + client.close(CloseMode.GRACEFUL); + } + } +} diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClassicClientCallTimeoutExample.java b/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClassicClientCallTimeoutExample.java new file mode 100644 index 0000000000..75a773316c --- /dev/null +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClassicClientCallTimeoutExample.java @@ -0,0 +1,91 @@ +/* + * ==================================================================== + * 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 + * . + * + */ +package org.apache.hc.client5.http.examples; + +import java.io.IOException; + +import org.apache.hc.client5.http.classic.methods.HttpGet; +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.HttpClients; +import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; +import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; +import org.apache.hc.core5.http.ClassicHttpResponse; +import org.apache.hc.core5.http.io.HttpClientResponseHandler; +import org.apache.hc.core5.util.Timeout; + +public class ClassicClientCallTimeoutExample { + + public static void main(final String[] args) throws Exception { + + // Non-deprecated: set connect/socket timeouts via ConnectionConfig + final PoolingHttpClientConnectionManager cm = + PoolingHttpClientConnectionManagerBuilder.create() + .setDefaultConnectionConfig( + ConnectionConfig.custom() + .setConnectTimeout(Timeout.ofSeconds(10)) + .setSocketTimeout(Timeout.ofSeconds(10)) + .build()) + .build(); + + try (final CloseableHttpClient client = HttpClients.custom() + .setConnectionManager(cm) + .build()) { + + // ---- Expected TIMEOUT (hard call deadline) ---- + final HttpGet slow = new HttpGet("https://httpbin.org/delay/5"); + slow.setConfig(RequestConfig.custom() + .setRequestTimeout(Timeout.ofSeconds(2)) // hard end-to-end cap + .setConnectionRequestTimeout(Timeout.ofSeconds(3)) // don't hang on pool lease + .build()); + + final HttpClientResponseHandler handler = (ClassicHttpResponse response) -> { + return response.getCode() + " " + response.getReasonPhrase(); + }; + + System.out.println("Executing (expected timeout): " + slow.getPath()); + try { + client.execute(slow, handler); // will throw by design + System.out.println("UNEXPECTED: completed"); + } catch (final IOException ex) { + System.out.println("As expected: " + ex.getClass().getSimpleName() + " - " + ex.getMessage()); + } + + // ---- Expected SUCCESS within budget (use HTTP to avoid TLS variance) ---- + final HttpGet fast = new HttpGet("http://httpbin.org/delay/1"); // HTTP on purpose + fast.setConfig(RequestConfig.custom() + .setRequestTimeout(Timeout.ofSeconds(8)) // generous end-to-end budget + .setConnectionRequestTimeout(Timeout.ofSeconds(2)) // quick fail if pool stuck + .build()); + + System.out.println("Executing (expected success): " + fast.getPath()); + final String ok = client.execute(fast, handler); + System.out.println("OK: " + ok); + } + } +}