Skip to content

Commit 0683bb8

Browse files
barancevbarancev
committed
[java] Implementing request timeouts in both HttpClient-s
1 parent 9514499 commit 0683bb8

File tree

3 files changed

+86
-44
lines changed

3 files changed

+86
-44
lines changed

‎java/client/src/org/openqa/selenium/remote/http/netty/NettyHttpHandler.java

+6-2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
import java.io.UncheckedIOException;
3131
import java.util.concurrent.ExecutionException;
3232
import java.util.concurrent.Future;
33+
import java.util.concurrent.TimeUnit;
34+
import java.util.concurrent.TimeoutException;
3335

3436
public class NettyHttpHandler extends RemoteCall {
3537

@@ -51,14 +53,16 @@ private HttpResponse makeCall(HttpRequest request) {
5153
Require.nonNull("Request", request);
5254

5355
Future<Response> whenResponse = client.executeRequest(
54-
NettyMessages.toNettyRequest(getConfig().baseUri(), request));
56+
NettyMessages.toNettyRequest(getConfig().baseUri(), request));
5557

5658
try {
57-
Response response = whenResponse.get();
59+
Response response = whenResponse.get(getConfig().readTimeout().toMillis(), TimeUnit.MILLISECONDS);
5860
return NettyMessages.toSeleniumResponse(response);
5961
} catch (InterruptedException e) {
6062
Thread.currentThread().interrupt();
6163
throw new RuntimeException("NettyHttpHandler request interrupted", e);
64+
} catch (TimeoutException e) {
65+
throw new org.openqa.selenium.TimeoutException(e);
6266
} catch (ExecutionException e) {
6367
Throwable cause = e.getCause();
6468
if (cause instanceof UncheckedIOException) {

‎java/client/src/org/openqa/selenium/remote/http/reactor/ReactorClient.java

+33-27
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import io.netty.buffer.ByteBufAllocator;
2525
import io.netty.channel.ChannelOption;
2626
import io.netty.channel.unix.DomainSocketAddress;
27+
28+
import org.openqa.selenium.TimeoutException;
2729
import org.openqa.selenium.internal.Require;
2830
import org.openqa.selenium.remote.http.AddSeleniumUserAgent;
2931
import org.openqa.selenium.remote.http.ClientConfig;
@@ -72,8 +74,8 @@ public class ReactorClient implements HttpClient {
7274

7375
private static final Map<HttpMethod, io.netty.handler.codec.http.HttpMethod> METHOD_MAP =
7476
ImmutableMap.of(HttpMethod.DELETE, io.netty.handler.codec.http.HttpMethod.DELETE,
75-
HttpMethod.GET, io.netty.handler.codec.http.HttpMethod.GET,
76-
HttpMethod.POST, io.netty.handler.codec.http.HttpMethod.POST);
77+
HttpMethod.GET, io.netty.handler.codec.http.HttpMethod.GET,
78+
HttpMethod.POST, io.netty.handler.codec.http.HttpMethod.POST);
7779

7880
private static final int MAX_CHUNK_SIZE = 1024 * 512; // 500k
7981

@@ -93,9 +95,9 @@ private reactor.netty.http.client.HttpClient createClient() {
9395
switch (config.baseUri().getScheme()) {
9496
case "http":
9597
case "https":
96-
int port = config.baseUri().getPort() == -1 ?
97-
SCHEME_TO_PORT.get(config.baseUri().getScheme()) :
98-
config.baseUri().getPort();
98+
int port = config.baseUri().getPort() == -1
99+
? SCHEME_TO_PORT.get(config.baseUri().getScheme())
100+
: config.baseUri().getPort();
99101
SocketAddress inetAddr = new InetSocketAddress(config.baseUri().getHost(), port);
100102
client = client.remoteAddress(() -> inetAddr)
101103
.tcpConfiguration(
@@ -136,28 +138,32 @@ public HttpResponse execute(HttpRequest request) {
136138
Joiner.on('&').appendTo(uri, queryPairs);
137139
}
138140

139-
Tuple2<InputStream, HttpResponse> result = httpClient
140-
.headers(h -> {
141-
request.getHeaderNames().forEach(
142-
name -> request.getHeaders(name).forEach(value -> h.set(name, value)));
143-
if (request.getHeader("User-Agent") == null) {
144-
h.set("User-Agent", AddSeleniumUserAgent.USER_AGENT);
145-
}
146-
})
147-
.request(METHOD_MAP.get(request.getMethod()))
148-
.uri(uri.toString())
149-
.send((r, out) -> out.send(fromInputStream(request.getContent().get())))
150-
.responseSingle((res, buf) -> {
151-
HttpResponse toReturn = new HttpResponse();
152-
toReturn.setStatus(res.status().code());
153-
res.responseHeaders().entries().forEach(
154-
entry -> toReturn.addHeader(entry.getKey(), entry.getValue()));
155-
return buf.asInputStream()
156-
.switchIfEmpty(Mono.just(new ByteArrayInputStream("".getBytes(UTF_8))))
157-
.zipWith(Mono.just(toReturn));
158-
}).block();
159-
result.getT2().setContent(result::getT1);
160-
return result.getT2();
141+
try {
142+
Tuple2<InputStream, HttpResponse> result = httpClient
143+
.headers(h -> {
144+
request.getHeaderNames().forEach(
145+
name -> request.getHeaders(name).forEach(value -> h.set(name, value)));
146+
if (request.getHeader("User-Agent") == null) {
147+
h.set("User-Agent", AddSeleniumUserAgent.USER_AGENT);
148+
}
149+
})
150+
.request(METHOD_MAP.get(request.getMethod()))
151+
.uri(uri.toString())
152+
.send((r, out) -> out.send(fromInputStream(request.getContent().get())))
153+
.responseSingle((res, buf) -> {
154+
HttpResponse toReturn = new HttpResponse();
155+
toReturn.setStatus(res.status().code());
156+
res.responseHeaders().entries().forEach(
157+
entry -> toReturn.addHeader(entry.getKey(), entry.getValue()));
158+
return buf.asInputStream()
159+
.switchIfEmpty(Mono.just(new ByteArrayInputStream("".getBytes(UTF_8))))
160+
.zipWith(Mono.just(toReturn));
161+
}).block(config.readTimeout());
162+
result.getT2().setContent(result::getT1);
163+
return result.getT2();
164+
} catch (IllegalStateException ex) {
165+
throw new TimeoutException(ex);
166+
}
161167
}
162168

163169
private Flux<ByteBuf> fromInputStream(InputStream is) {

‎java/client/test/org/openqa/selenium/remote/internal/HttpClientTestBase.java

+47-15
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,11 @@
2222
import org.junit.Test;
2323
import org.openqa.selenium.BuildInfo;
2424
import org.openqa.selenium.Platform;
25+
import org.openqa.selenium.TimeoutException;
2526
import org.openqa.selenium.environment.webserver.AppServer;
2627
import org.openqa.selenium.environment.webserver.NettyAppServer;
2728
import org.openqa.selenium.json.Json;
29+
import org.openqa.selenium.remote.http.ClientConfig;
2830
import org.openqa.selenium.remote.http.Contents;
2931
import org.openqa.selenium.remote.http.HttpClient;
3032
import org.openqa.selenium.remote.http.HttpHandler;
@@ -33,6 +35,7 @@
3335

3436
import java.net.URI;
3537
import java.net.URL;
38+
import java.time.Duration;
3639
import java.util.Arrays;
3740
import java.util.List;
3841
import java.util.Map;
@@ -42,6 +45,7 @@
4245
import static java.util.Collections.singletonList;
4346
import static java.util.stream.Collectors.toList;
4447
import static org.assertj.core.api.Assertions.assertThat;
48+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
4549
import static org.openqa.selenium.json.Json.MAP_TYPE;
4650
import static org.openqa.selenium.net.Urls.fromUri;
4751
import static org.openqa.selenium.remote.http.Contents.string;
@@ -77,8 +81,8 @@ public void responseShouldKeepMultipleHeadersSeparate() {
7781
HttpResponse response = getResponseWithHeaders(headers);
7882

7983
List<String> values = StreamSupport
80-
.stream(response.getHeaders("Cheese").spliterator(), false)
81-
.collect(toList());
84+
.stream(response.getHeaders("Cheese").spliterator(), false)
85+
.collect(toList());
8286

8387
assertThat(values).contains("Cheddar");
8488
assertThat(values).contains("Brie, Gouda");
@@ -143,8 +147,8 @@ public void shouldAllowUrlsWithSchemesToBeUsed() throws Exception {
143147

144148
URI uri = URI.create(server.whereIs("/"));
145149
HttpRequest request = new HttpRequest(
146-
GET,
147-
String.format("http://%s:%s/hello", uri.getHost(), uri.getPort()));
150+
GET,
151+
String.format("http://%s:%s/hello", uri.getHost(), uri.getPort()));
148152

149153
HttpResponse response = client.execute(request);
150154

@@ -157,27 +161,43 @@ public void shouldAllowUrlsWithSchemesToBeUsed() throws Exception {
157161
@Test
158162
public void shouldIncludeAUserAgentHeader() {
159163
HttpResponse response = executeWithinServer(
160-
new HttpRequest(GET, "/foo"),
161-
req -> new HttpResponse().setContent(Contents.utf8String(req.getHeader("user-agent"))));
164+
new HttpRequest(GET, "/foo"),
165+
req -> new HttpResponse().setContent(Contents.utf8String(req.getHeader("user-agent"))));
162166

163167
String label = new BuildInfo().getReleaseLabel();
164168
Platform platform = Platform.getCurrent();
165169
Platform family = platform.family() == null ? platform : platform.family();
166170

167171
assertThat(string(response)).isEqualTo(String.format(
168-
"selenium/%s (java %s)",
169-
label,
170-
family.toString().toLowerCase()));
172+
"selenium/%s (java %s)",
173+
label,
174+
family.toString().toLowerCase()));
171175
}
172176

173-
private HttpResponse getResponseWithHeaders(final Multimap<String, String> headers) {
174-
return executeWithinServer(
177+
@Test
178+
public void shouldAllowConfigurationOfRequestTimeout() {
179+
assertThatExceptionOfType(TimeoutException.class)
180+
.isThrownBy(() -> executeWithinServer(
175181
new HttpRequest(GET, "/foo"),
176182
req -> {
177-
HttpResponse resp = new HttpResponse();
178-
headers.forEach(resp::addHeader);
179-
return resp;
180-
});
183+
try {
184+
Thread.sleep(3000);
185+
} catch (InterruptedException e) {
186+
e.printStackTrace();
187+
}
188+
return new HttpResponse().setContent(Contents.utf8String(req.getHeader("user-agent")));
189+
},
190+
ClientConfig.defaultConfig().readTimeout(Duration.ofSeconds(1))));
191+
}
192+
193+
private HttpResponse getResponseWithHeaders(final Multimap<String, String> headers) {
194+
return executeWithinServer(
195+
new HttpRequest(GET, "/foo"),
196+
req -> {
197+
HttpResponse resp = new HttpResponse();
198+
headers.forEach(resp::addHeader);
199+
return resp;
200+
});
181201
}
182202

183203
private HttpResponse getQueryParameterResponse(HttpRequest request) {
@@ -203,4 +223,16 @@ private HttpResponse executeWithinServer(HttpRequest request, HttpHandler handle
203223
server.stop();
204224
}
205225
}
226+
227+
private HttpResponse executeWithinServer(HttpRequest request, HttpHandler handler, ClientConfig config) {
228+
AppServer server = new NettyAppServer(handler);
229+
server.start();
230+
231+
try {
232+
HttpClient client = createFactory().createClient(config.baseUri(URI.create(server.whereIs("/"))));
233+
return client.execute(request);
234+
} finally {
235+
server.stop();
236+
}
237+
}
206238
}

0 commit comments

Comments
 (0)