Skip to content

Commit f841c48

Browse files
pujaganipujagani
authored
Add retry http request filter for transient errors. Integrate filter with ClientConfig.
1 parent aceb6e2 commit f841c48

File tree

5 files changed

+270
-2
lines changed

5 files changed

+270
-2
lines changed

‎java/src/org/openqa/selenium/remote/http/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,6 @@ java_export(
1717
"//java/src/org/openqa/selenium:core",
1818
"//java/src/org/openqa/selenium/json",
1919
artifact("com.google.guava:guava"),
20+
artifact("net.jodah:failsafe")
2021
],
2122
)

‎java/src/org/openqa/selenium/remote/http/ClientConfig.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030

3131
public class ClientConfig {
3232

33-
private static final AddSeleniumUserAgent DEFAULT_FILTER = new AddSeleniumUserAgent();
33+
private static final Filter DEFAULT_FILTER = new AddSeleniumUserAgent().andThen(new RetryRequest());
3434
private final URI baseUri;
3535
private final Duration connectionTimeout;
3636
private final Duration readTimeout;
@@ -58,7 +58,7 @@ public static ClientConfig defaultConfig() {
5858
null,
5959
Duration.ofSeconds(10),
6060
Duration.ofMinutes(3),
61-
new AddSeleniumUserAgent(),
61+
DEFAULT_FILTER,
6262
null,
6363
null);
6464
}

‎java/src/org/openqa/selenium/remote/http/RetryRequest.java

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Licensed to the Software Freedom Conservancy (SFC) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The SFC licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.openqa.selenium.remote.http;
19+
20+
import net.jodah.failsafe.Failsafe;
21+
import net.jodah.failsafe.RetryPolicy;
22+
import org.openqa.selenium.TimeoutException;
23+
24+
import java.net.ConnectException;
25+
import java.time.Duration;
26+
import java.time.temporal.ChronoUnit;
27+
import java.util.logging.Level;
28+
import java.util.logging.Logger;
29+
30+
import static com.google.common.net.HttpHeaders.CONTENT_LENGTH;
31+
import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
32+
import static java.net.HttpURLConnection.HTTP_UNAVAILABLE;
33+
34+
public class RetryRequest implements Filter {
35+
36+
private static final Logger LOG = Logger.getLogger(RetryRequest.class.getName());
37+
38+
// Retry on connection error.
39+
private static final RetryPolicy<HttpResponse> connectionFailurePolicy =
40+
new RetryPolicy<HttpResponse>()
41+
.handleIf(failure -> failure.getCause() instanceof ConnectException)
42+
.withBackoff(1, 4, ChronoUnit.SECONDS)
43+
.withMaxRetries(3)
44+
.withMaxDuration(Duration.ofSeconds(10))
45+
.onRetry(e -> LOG.log(
46+
Level.WARNING,
47+
"Connection failure #{0}. Retrying.",
48+
e.getAttemptCount()));
49+
50+
// Retry on read timeout.
51+
private static final RetryPolicy<HttpResponse> readTimeoutPolicy =
52+
new RetryPolicy<HttpResponse>()
53+
.handle(TimeoutException.class)
54+
.withBackoff(1, 4, ChronoUnit.SECONDS)
55+
.withMaxRetries(3)
56+
.withMaxDuration(Duration.ofSeconds(10))
57+
.onRetry(e -> LOG.log(
58+
Level.WARNING,
59+
"Read timeout #{0}. Retrying.",
60+
e.getAttemptCount()));
61+
62+
// Retry if server is unavailable or an internal server error occurs without response body.
63+
private static final RetryPolicy<HttpResponse> serverErrorPolicy =
64+
new RetryPolicy<HttpResponse>()
65+
.handleResultIf(response -> response.getStatus() == HTTP_INTERNAL_ERROR &&
66+
Integer.parseInt(response.getHeader(CONTENT_LENGTH)) == 0)
67+
.handleResultIf(response -> response.getStatus() == HTTP_UNAVAILABLE)
68+
.withBackoff(1, 2, ChronoUnit.SECONDS)
69+
.withMaxRetries(2)
70+
.withMaxDuration(Duration.ofSeconds(3))
71+
.onRetry(e -> LOG.log(
72+
Level.WARNING,
73+
"Failure due to server error #{0}. Retrying.",
74+
e.getAttemptCount()));
75+
76+
@Override
77+
public HttpHandler apply(HttpHandler next) {
78+
return req -> Failsafe.with(
79+
connectionFailurePolicy,
80+
readTimeoutPolicy,
81+
serverErrorPolicy)
82+
.get(() -> next.execute(req));
83+
}
84+
}

‎java/test/org/openqa/selenium/remote/http/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@ java_test_suite(
1111
],
1212
deps = [
1313
"//java:auto-service",
14+
"//java/src/org/openqa/selenium:core",
1415
"//java/src/org/openqa/selenium/remote/http",
1516
"//java/src/org/openqa/selenium/remote/http/netty",
17+
"//java/test/org/openqa/selenium/environment",
1618
"//java/test/org/openqa/selenium/testing:annotations",
1719
artifact("org.assertj:assertj-core"),
1820
artifact("com.google.guava:guava"),

‎java/test/org/openqa/selenium/remote/http/RetryRequestTest.java

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
// Licensed to the Software Freedom Conservancy (SFC) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The SFC licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.openqa.selenium.remote.http;
19+
20+
import com.google.common.collect.ImmutableMap;
21+
import org.junit.Before;
22+
import org.junit.Test;
23+
import org.openqa.selenium.TimeoutException;
24+
import org.openqa.selenium.environment.webserver.AppServer;
25+
import org.openqa.selenium.environment.webserver.NettyAppServer;
26+
import org.openqa.selenium.remote.http.netty.NettyClient;
27+
28+
import java.io.UncheckedIOException;
29+
import java.net.MalformedURLException;
30+
import java.net.URI;
31+
import java.time.Duration;
32+
import java.util.concurrent.atomic.AtomicInteger;
33+
34+
import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
35+
import static java.net.HttpURLConnection.HTTP_OK;
36+
import static java.net.HttpURLConnection.HTTP_UNAVAILABLE;
37+
import static org.assertj.core.api.Assertions.assertThat;
38+
import static org.openqa.selenium.remote.http.Contents.asJson;
39+
import static org.openqa.selenium.remote.http.HttpMethod.GET;
40+
41+
public class RetryRequestTest {
42+
43+
private HttpClient client;
44+
private static final String REQUEST_PATH = "http://%s:%s/hello";
45+
46+
@Before
47+
public void setUp() throws MalformedURLException {
48+
ClientConfig config = ClientConfig.defaultConfig()
49+
.baseUrl(URI.create("http://localhost:2345").toURL())
50+
.readTimeout(Duration.ofMillis(1000));
51+
client = new NettyClient.Factory().createClient(config);
52+
}
53+
54+
@Test
55+
public void shouldBeAbleToHandleARequest() {
56+
AtomicInteger count = new AtomicInteger(0);
57+
AppServer server = new NettyAppServer(req -> {
58+
count.incrementAndGet();
59+
return new HttpResponse();
60+
});
61+
server.start();
62+
63+
URI uri = URI.create(server.whereIs("/"));
64+
HttpRequest request = new HttpRequest(
65+
GET,
66+
String.format(REQUEST_PATH, uri.getHost(), uri.getPort()));
67+
HttpResponse response = client.execute(request);
68+
69+
assertThat(response).extracting(HttpResponse::getStatus).isEqualTo(HTTP_OK);
70+
71+
assertThat(count.get()).isEqualTo(1);
72+
server.stop();
73+
}
74+
75+
@Test
76+
public void shouldBeAbleToRetryARequestOnInternalServerError() {
77+
AtomicInteger count = new AtomicInteger(0);
78+
AppServer server = new NettyAppServer(req -> {
79+
count.incrementAndGet();
80+
return new HttpResponse().setStatus(500);
81+
});
82+
server.start();
83+
84+
URI uri = URI.create(server.whereIs("/"));
85+
HttpRequest request = new HttpRequest(
86+
GET,
87+
String.format(REQUEST_PATH, uri.getHost(), uri.getPort()));
88+
HttpResponse response = client.execute(request);
89+
90+
assertThat(response).extracting(HttpResponse::getStatus).isEqualTo(HTTP_INTERNAL_ERROR);
91+
assertThat(count.get()).isEqualTo(3);
92+
93+
server.stop();
94+
}
95+
96+
@Test
97+
public void shouldNotRetryRequestOnInternalServerErrorWithContent() {
98+
AtomicInteger count = new AtomicInteger(0);
99+
AppServer server = new NettyAppServer(req -> {
100+
count.incrementAndGet();
101+
return new HttpResponse()
102+
.setStatus(500)
103+
.setContent(asJson(ImmutableMap.of("error", "non-transient")));
104+
});
105+
server.start();
106+
107+
URI uri = URI.create(server.whereIs("/"));
108+
HttpRequest request = new HttpRequest(
109+
GET,
110+
String.format(REQUEST_PATH, uri.getHost(), uri.getPort()));
111+
HttpResponse response = client.execute(request);
112+
113+
assertThat(response).extracting(HttpResponse::getStatus).isEqualTo(HTTP_INTERNAL_ERROR);
114+
assertThat(count.get()).isEqualTo(1);
115+
116+
server.stop();
117+
}
118+
119+
@Test
120+
public void shouldRetryRequestOnServerUnavailableError() {
121+
AtomicInteger count = new AtomicInteger(0);
122+
AppServer server = new NettyAppServer(req -> {
123+
count.incrementAndGet();
124+
return new HttpResponse()
125+
.setStatus(503)
126+
.setContent(asJson(ImmutableMap.of("error", "server down")));
127+
});
128+
server.start();
129+
130+
URI uri = URI.create(server.whereIs("/"));
131+
HttpRequest request = new HttpRequest(
132+
GET,
133+
String.format(REQUEST_PATH, uri.getHost(), uri.getPort()));
134+
HttpResponse response = client.execute(request);
135+
136+
assertThat(response).extracting(HttpResponse::getStatus).isEqualTo(HTTP_UNAVAILABLE);
137+
assertThat(count.get()).isEqualTo(3);
138+
139+
server.stop();
140+
}
141+
142+
@Test
143+
public void shouldBeAbleToRetryARequestOnTimeout() {
144+
AtomicInteger count = new AtomicInteger(0);
145+
AppServer server = new NettyAppServer(req -> {
146+
count.incrementAndGet();
147+
try {
148+
Thread.sleep(2000);
149+
} catch (InterruptedException e) {
150+
e.printStackTrace();
151+
}
152+
return new HttpResponse();
153+
});
154+
server.start();
155+
156+
URI uri = URI.create(server.whereIs("/"));
157+
HttpRequest request = new HttpRequest(
158+
GET,
159+
String.format(REQUEST_PATH, uri.getHost(), uri.getPort()));
160+
161+
try {
162+
client.execute(request);
163+
} catch (TimeoutException e) {
164+
assertThat(count.get()).isEqualTo(4);
165+
} finally {
166+
server.stop();
167+
}
168+
}
169+
170+
@Test(expected = UncheckedIOException.class)
171+
public void shouldBeAbleToRetryARequestOnConnectionFailure() {
172+
AppServer server = new NettyAppServer(req -> new HttpResponse());
173+
174+
URI uri = URI.create(server.whereIs("/"));
175+
HttpRequest request = new HttpRequest(
176+
GET,
177+
String.format(REQUEST_PATH, uri.getHost(), uri.getPort()));
178+
179+
client.execute(request);
180+
}
181+
}

0 commit comments

Comments
 (0)