Skip to content

Commit

Permalink
We should actually keep RequestOptions#setTimeout to configure simult…
Browse files Browse the repository at this point in the history
…aneously the connect/idle timeouts
  • Loading branch information
vietj committed Nov 9, 2023
1 parent 27b9a77 commit 433d282
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 22 deletions.
6 changes: 6 additions & 0 deletions src/main/asciidoc/http.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,12 @@ You can set a connect timeout to prevent your application from unresponsive busy
The connect timeout option is not related to the TCP {@link io.vertx.core.http.HttpClientOptions#setConnectTimeout(int)} option, when a request is made against a pooled HTTP client, the timeout applies to the duration to obtain a connection from the pool to serve the request,
the timeout might fire because the server does not respond in time or the pool is too busy to serve a request.

You can configure both timeout using {@link io.vertx.core.http.RequestOptions#setTimeout(long)}

[source,$lang]
----
{@link examples.HTTPExamples#clientTimeout}
----

==== Writing HTTP/2 frames

Expand Down
10 changes: 10 additions & 0 deletions src/main/java/examples/HTTPExamples.java
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,16 @@ public void clientConnectTimeout(HttpClient client, int port, String host, Strin
.compose(request -> request.send().compose(HttpClientResponse::body));
}

public void clientTimeout(HttpClient client, int port, String host, String uri, int timeoutMS) {
Future<Buffer> fut = client
.request(new RequestOptions()
.setHost(host)
.setPort(port)
.setURI(uri)
.setTimeout(timeoutMS))
.compose(request -> request.send().compose(HttpClientResponse::body));
}

public void useRequestAsStream(HttpClientRequest request) {

request.setChunked(true);
Expand Down
26 changes: 16 additions & 10 deletions src/main/java/io/vertx/core/http/RequestOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -76,19 +76,19 @@ public class RequestOptions {
public static final boolean DEFAULT_FOLLOW_REDIRECTS = false;

/**
* The default request timeout = {@code 0} (disabled)
* The default request timeout = {@code -1L} (disabled)
*/
public static final long DEFAULT_TIMEOUT = 0;
public static final long DEFAULT_TIMEOUT = -1L;

/**
* The default connect timeout = {@code 0} (disabled)
* The default connect timeout = {@code -1L} (disabled)
*/
public static final long DEFAULT_CONNECT_TIMEOUT = 0;
public static final long DEFAULT_CONNECT_TIMEOUT = -1L;

/**
* The default idle timeout = {@code 0} (disabled)
* The default idle timeout = {@code -1L} (disabled)
*/
public static final long DEFAULT_IDLE_TIMEOUT = 0;
public static final long DEFAULT_IDLE_TIMEOUT = -1L;

private ProxyOptions proxyOptions;
private SocketAddress server;
Expand Down Expand Up @@ -346,17 +346,23 @@ public RequestOptions setFollowRedirects(Boolean followRedirects) {
/**
* @see #setTimeout(long)
*/
@Deprecated
public long getTimeout() {
return timeout;
}

/**
* Equivalent to setting the same timeout value with {@link #setConnectTimeout(long)} and {@link #setIdleTimeout(long)}.
* Sets both connect and idle timeouts for the request
*
* @deprecated instead use {@link #setConnectTimeout(long)} or/and {@link #setIdleTimeout(long)}
* <ul>
* <li><i>connect timeout</i>: if the request is not obtained from the client within the timeout period, the {@code Future<HttpClientRequest>}
* obtained from the client is failed with a {@link java.util.concurrent.TimeoutException}.</li>
* <li><i>idle timeout</i>: if the request does not return any data within the timeout period, the request/response is closed and the
* related futures are failed with a {@link java.util.concurrent.TimeoutException}, e.g. {@code Future<HttpClientResponse>}
* or {@code Future<Buffer>} response body.</li>
* </ul>
*
* The connect and idle timeouts can be set separately using {@link #setConnectTimeout(long)} and {@link #setIdleTimeout(long)}
*/
@Deprecated
public RequestOptions setTimeout(long timeout) {
this.timeout = timeout;
return this;
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/io/vertx/core/http/impl/HttpClientBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ public Endpoint<HttpClientConnection> create(ContextInternal ctx, Runnable dispo
ar -> {
if (ar.succeeded()) {
Http1xClientConnection conn = (Http1xClientConnection) ar.result();
long timeout = connectOptions.getTimeout();
if (timeout == 0L) {
long timeout = Math.max(connectOptions.getTimeout(), 0L);
if (connectOptions.getIdleTimeout() >= 0L) {
timeout = connectOptions.getIdleTimeout();
}
conn.toWebSocket(ctx,
Expand Down
21 changes: 11 additions & 10 deletions src/main/java/io/vertx/core/http/impl/HttpClientImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -310,16 +310,17 @@ private void doRequest(RequestOptions request, PromiseInternal<HttpClientRequest
} else {
key = new EndpointKey(useSSL, proxyOptions, server, peerAddress);
}
long connectTimeout = request.getConnectTimeout();
long idleTimeout = request.getIdleTimeout();
long timeout = request.getTimeout();
if (timeout > 0L) {
if (connectTimeout == 0L) {
connectTimeout = timeout;
}
if (idleTimeout == 0L) {
idleTimeout = timeout;
}
long connectTimeout = 0L;
long idleTimeout = 0L;
if (request.getTimeout() >= 0L) {
connectTimeout = request.getTimeout();
idleTimeout = request.getTimeout();
}
if (request.getConnectTimeout() >= 0L) {
connectTimeout = request.getConnectTimeout();
}
if (request.getIdleTimeout() >= 0L) {
idleTimeout = request.getIdleTimeout();
}
doRequest(method, server, host, port, useSSL, requestURI, headers, request.getTraceOperation(), connectTimeout, idleTimeout, followRedirects, proxyOptions, key, promise);
}
Expand Down

0 comments on commit 433d282

Please sign in to comment.