Skip to content

Commit 4749d81

Browse files
committed
Refactor ReactorClientHttpRequestFactory timeouts
Closes gh-33782
1 parent 044da79 commit 4749d81

File tree

3 files changed

+106
-34
lines changed

3 files changed

+106
-34
lines changed

spring-web/src/main/java/org/springframework/http/client/ReactorClientHttpRequest.java

+42-10
Original file line numberDiff line numberDiff line change
@@ -53,19 +53,50 @@ final class ReactorClientHttpRequest extends AbstractStreamingClientHttpRequest
5353

5454
private final URI uri;
5555

56+
@Nullable
5657
private final Duration exchangeTimeout;
5758

58-
private final Duration readTimeout;
5959

60+
/**
61+
* Create an instance.
62+
* @param httpClient the client to perform the request with
63+
* @param method the HTTP method
64+
* @param uri the URI for the request
65+
* @since 6.2
66+
*/
67+
public ReactorClientHttpRequest(HttpClient httpClient, HttpMethod method, URI uri) {
68+
this.httpClient = httpClient;
69+
this.method = method;
70+
this.uri = uri;
71+
this.exchangeTimeout = null;
72+
}
6073

74+
/**
75+
* Package private constructor for use until exchangeTimeout is removed.
76+
*/
77+
ReactorClientHttpRequest(HttpClient httpClient, HttpMethod method, URI uri, @Nullable Duration exchangeTimeout) {
78+
this.httpClient = httpClient;
79+
this.method = method;
80+
this.uri = uri;
81+
this.exchangeTimeout = exchangeTimeout;
82+
}
83+
84+
/**
85+
* Original constructor with timeout values.
86+
* @deprecated without a replacement; readTimeout is now applied to the
87+
* underlying client via {@link HttpClient#responseTimeout(Duration)}, and the
88+
* value passed here is not used; exchangeTimeout is deprecated and superseded
89+
* by Reactor Netty timeout configuration, but applied if set.
90+
*/
91+
@Deprecated(since = "6.2", forRemoval = true)
6192
public ReactorClientHttpRequest(
62-
HttpClient httpClient, URI uri, HttpMethod method, Duration exchangeTimeout, Duration readTimeout) {
93+
HttpClient httpClient, URI uri, HttpMethod method,
94+
@Nullable Duration exchangeTimeout, @Nullable Duration readTimeout) {
6395

6496
this.httpClient = httpClient;
6597
this.method = method;
6698
this.uri = uri;
6799
this.exchangeTimeout = exchangeTimeout;
68-
this.readTimeout = readTimeout;
69100
}
70101

71102

@@ -89,18 +120,19 @@ protected ClientHttpResponse executeInternal(HttpHeaders headers, @Nullable Body
89120
sender = (this.uri.isAbsolute() ? sender.uri(this.uri) : sender.uri(this.uri.toString()));
90121

91122
try {
92-
ReactorClientHttpResponse result =
123+
Mono<ReactorClientHttpResponse> mono =
93124
sender.send((request, outbound) -> send(headers, body, request, outbound))
94-
.responseConnection((response, conn) ->
95-
Mono.just(new ReactorClientHttpResponse(response, conn, this.readTimeout)))
96-
.next()
97-
.block(this.exchangeTimeout);
125+
.responseConnection((response, conn) -> Mono.just(new ReactorClientHttpResponse(response, conn)))
126+
.next();
127+
128+
ReactorClientHttpResponse clientResponse =
129+
(this.exchangeTimeout != null ? mono.block(this.exchangeTimeout) : mono.block());
98130

99-
if (result == null) {
131+
if (clientResponse == null) {
100132
throw new IOException("HTTP exchange resulted in no result");
101133
}
102134

103-
return result;
135+
return clientResponse;
104136
}
105137
catch (RuntimeException ex) {
106138
throw convertException(ex);

spring-web/src/main/java/org/springframework/http/client/ReactorClientHttpRequestFactory.java

+41-18
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ public class ReactorClientHttpRequestFactory implements ClientHttpRequestFactory
4848

4949
private static final Log logger = LogFactory.getLog(ReactorClientHttpRequestFactory.class);
5050

51-
private static final Function<HttpClient, HttpClient> defaultInitializer = client -> client.compress(true);
51+
private static final Function<HttpClient, HttpClient> defaultInitializer =
52+
client -> client.compress(true).responseTimeout(Duration.ofSeconds(10));
5253

5354

5455
@Nullable
@@ -60,9 +61,11 @@ public class ReactorClientHttpRequestFactory implements ClientHttpRequestFactory
6061
@Nullable
6162
private Integer connectTimeout;
6263

63-
private Duration readTimeout = Duration.ofSeconds(10);
64+
@Nullable
65+
private Duration readTimeout;
6466

65-
private Duration exchangeTimeout = Duration.ofSeconds(5);
67+
@Nullable
68+
private Duration exchangeTimeout;
6669

6770
@Nullable
6871
private volatile HttpClient httpClient;
@@ -120,12 +123,15 @@ private HttpClient createHttpClient(ReactorResourceFactory factory, Function<Htt
120123
if (this.connectTimeout != null) {
121124
client = client.option(ChannelOption.CONNECT_TIMEOUT_MILLIS, this.connectTimeout);
122125
}
126+
if (this.readTimeout != null) {
127+
client = client.responseTimeout(this.readTimeout);
128+
}
123129
return client;
124130
}
125131

126132

127133
/**
128-
* Set the underlying connect timeout value on the underlying client.
134+
* Set the connect timeout value on the underlying client.
129135
* Effectively, a shortcut for
130136
* {@code httpClient.option(CONNECT_TIMEOUT_MILLIS, timeout)}.
131137
* <p>By default, set to 30 seconds.
@@ -152,35 +158,53 @@ public void setConnectTimeout(Duration connectTimeout) {
152158
}
153159

154160
/**
155-
* Set the underlying read timeout in milliseconds.
156-
* <p>Default is 10 seconds.
161+
* Set the read timeout value on the underlying client.
162+
* Effectively, a shortcut for {@link HttpClient#responseTimeout(Duration)}.
163+
* <p>By default, set to 10 seconds.
164+
* @param timeout the read timeout value in millis; must be > 0.
157165
*/
158-
public void setReadTimeout(long readTimeout) {
159-
Assert.isTrue(readTimeout > 0, "Timeout must be a positive value");
160-
this.readTimeout = Duration.ofMillis(readTimeout);
166+
public void setReadTimeout(Duration timeout) {
167+
Assert.notNull(timeout, "ReadTimeout must not be null");
168+
Assert.isTrue(timeout.toMillis() > 0, "Timeout must be a positive value");
169+
this.readTimeout = timeout;
170+
HttpClient httpClient = this.httpClient;
171+
if (httpClient != null) {
172+
this.httpClient = httpClient.responseTimeout(timeout);
173+
}
161174
}
162175

163176
/**
164-
* Variant of {@link #setConnectTimeout(int)} with a {@link Duration} value.
177+
* Variant of {@link #setReadTimeout(Duration)} with a long value.
165178
*/
166-
public void setReadTimeout(Duration readTimeout) {
167-
Assert.notNull(readTimeout, "ReadTimeout must not be null");
168-
setReadTimeout((int) readTimeout.toMillis());
179+
public void setReadTimeout(long readTimeout) {
180+
setReadTimeout(Duration.ofMillis(readTimeout));
169181
}
170182

171183
/**
172184
* Set the timeout for the HTTP exchange in milliseconds.
173-
* <p>Default is 5 seconds.
185+
* <p>By default, as of 6.2 this is no longer set.
186+
* @see #setConnectTimeout(int)
187+
* @see #setReadTimeout(Duration)
188+
* @see <a href="https://projectreactor.io/docs/netty/release/reference/index.html#timeout-configuration">Timeout Configuration</a>
189+
* @deprecated as of 6.2 and no longer set by default (previously 5 seconds)
190+
* in favor of using Reactor Netty HttpClient timeout configuration.
174191
*/
192+
@Deprecated(since = "6.2", forRemoval = true)
175193
public void setExchangeTimeout(long exchangeTimeout) {
176194
Assert.isTrue(exchangeTimeout > 0, "Timeout must be a positive value");
177195
this.exchangeTimeout = Duration.ofMillis(exchangeTimeout);
178196
}
179197

180198
/**
181-
* Set the timeout for the HTTP exchange.
182-
* <p>Default is 5 seconds.
199+
* Variant of {@link #setExchangeTimeout(long)} with a Duration value.
200+
* <p>By default, as of 6.2 this is no longer set.
201+
* @see #setConnectTimeout(int)
202+
* @see #setReadTimeout(Duration)
203+
* @see <a href="https://projectreactor.io/docs/netty/release/reference/index.html#timeout-configuration">Timeout Configuration</a>
204+
* @deprecated as of 6.2 and no longer set by default (previously 5 seconds)
205+
* in favor of using Reactor Netty HttpClient timeout configuration.
183206
*/
207+
@Deprecated(since = "6.2", forRemoval = true)
184208
public void setExchangeTimeout(Duration exchangeTimeout) {
185209
Assert.notNull(exchangeTimeout, "ExchangeTimeout must not be null");
186210
setExchangeTimeout((int) exchangeTimeout.toMillis());
@@ -195,8 +219,7 @@ public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IO
195219
"Expected HttpClient or ResourceFactory and mapper");
196220
client = createHttpClient(this.resourceFactory, this.mapper);
197221
}
198-
return new ReactorClientHttpRequest(
199-
client, uri, httpMethod, this.exchangeTimeout, this.readTimeout);
222+
return new ReactorClientHttpRequest(client, httpMethod, uri, this.exchangeTimeout);
200223
}
201224

202225

spring-web/src/main/java/org/springframework/http/client/ReactorClientHttpResponse.java

+23-6
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import io.netty.buffer.ByteBuf;
2424
import org.reactivestreams.FlowAdapters;
2525
import reactor.netty.Connection;
26+
import reactor.netty.http.client.HttpClient;
2627
import reactor.netty.http.client.HttpClientResponse;
2728

2829
import org.springframework.http.HttpHeaders;
@@ -46,22 +47,38 @@ final class ReactorClientHttpResponse implements ClientHttpResponse {
4647

4748
private final HttpHeaders headers;
4849

49-
private final Duration readTimeout;
50-
5150
@Nullable
5251
private volatile InputStream body;
5352

5453

55-
public ReactorClientHttpResponse(
56-
HttpClientResponse response, Connection connection, Duration readTimeout) {
57-
54+
/**
55+
* Create a response instance.
56+
* @param response the Reactor Netty response
57+
* @param connection the connection for the exchange
58+
* @since 6.2
59+
*/
60+
public ReactorClientHttpResponse(HttpClientResponse response, Connection connection) {
5861
this.response = response;
5962
this.connection = connection;
60-
this.readTimeout = readTimeout;
6163
this.headers = HttpHeaders.readOnlyHttpHeaders(
6264
new Netty4HeadersAdapter(response.responseHeaders()));
6365
}
6466

67+
/**
68+
* Original constructor.
69+
* @deprecated without a replacement; readTimeout is now applied to the
70+
* underlying client via {@link HttpClient#responseTimeout(Duration)}, and the
71+
* value passed here is not used.
72+
*/
73+
@Deprecated(since = "6.2", forRemoval = true)
74+
public ReactorClientHttpResponse(
75+
HttpClientResponse response, Connection connection, @Nullable Duration readTimeout) {
76+
77+
this.response = response;
78+
this.connection = connection;
79+
this.headers = HttpHeaders.readOnlyHttpHeaders(new Netty4HeadersAdapter(response.responseHeaders()));
80+
}
81+
6582

6683
@Override
6784
public HttpStatusCode getStatusCode() {

0 commit comments

Comments
 (0)