Skip to content

Commit 42f054e

Browse files
diemoldiemol
committed
[java] Retry filter not used by default
The objective of the filter was to support retries when the Grid components fail to communicate, such as during start time. However, other mechanisms exist for that now. Retries are used for now only when communicating with the browser driver only. We will add it in other places as feedback from the community comes in. Users can still enable it in their RemoteWebDriver by creating a custom config. Fixes #10437
1 parent 919a988 commit 42f054e

File tree

4 files changed

+44
-54
lines changed

4 files changed

+44
-54
lines changed

‎java/src/org/openqa/selenium/grid/node/config/DriverServiceSessionFactory.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@
1717

1818
package org.openqa.selenium.grid.node.config;
1919

20-
import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES;
21-
import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES_EVENT;
22-
import static org.openqa.selenium.remote.tracing.Tags.EXCEPTION;
23-
2420
import org.openqa.selenium.Capabilities;
2521
import org.openqa.selenium.ImmutableCapabilities;
2622
import org.openqa.selenium.PersistentCapabilities;
@@ -42,6 +38,7 @@
4238
import org.openqa.selenium.remote.ProtocolHandshake;
4339
import org.openqa.selenium.remote.Response;
4440
import org.openqa.selenium.remote.SessionId;
41+
import org.openqa.selenium.remote.http.ClientConfig;
4542
import org.openqa.selenium.remote.http.HttpClient;
4643
import org.openqa.selenium.remote.service.DriverService;
4744
import org.openqa.selenium.remote.tracing.AttributeKey;
@@ -62,6 +59,10 @@
6259
import java.util.function.Predicate;
6360
import java.util.stream.Stream;
6461

62+
import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES;
63+
import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES_EVENT;
64+
import static org.openqa.selenium.remote.tracing.Tags.EXCEPTION;
65+
6566
public class DriverServiceSessionFactory implements SessionFactory {
6667

6768
private final Tracer tracer;
@@ -124,7 +125,8 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
124125
URL serviceURL = service.getUrl();
125126
attributeMap.put(AttributeKey.DRIVER_URL.getKey(),
126127
EventAttribute.setValue(serviceURL.toString()));
127-
HttpClient client = clientFactory.createClient(serviceURL);
128+
ClientConfig clientConfig = ClientConfig.defaultConfig().baseUrl(serviceURL).withRetries();
129+
HttpClient client = clientFactory.createClient(clientConfig);
128130

129131
Command command = new Command(null, DriverCommand.NEW_SESSION(capabilities));
130132

‎java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,6 @@
1717

1818
package org.openqa.selenium.grid.node.relay;
1919

20-
import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES;
21-
import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES_EVENT;
22-
import static org.openqa.selenium.remote.tracing.AttributeKey.DOWNSTREAM_DIALECT;
23-
import static org.openqa.selenium.remote.tracing.AttributeKey.DRIVER_RESPONSE;
24-
import static org.openqa.selenium.remote.tracing.AttributeKey.DRIVER_URL;
25-
import static org.openqa.selenium.remote.tracing.AttributeKey.EXCEPTION_EVENT;
26-
import static org.openqa.selenium.remote.tracing.AttributeKey.EXCEPTION_MESSAGE;
27-
import static org.openqa.selenium.remote.tracing.AttributeKey.LOGGER_CLASS;
28-
import static org.openqa.selenium.remote.tracing.AttributeKey.UPSTREAM_DIALECT;
29-
import static org.openqa.selenium.remote.tracing.EventAttribute.setValue;
30-
import static org.openqa.selenium.remote.tracing.Tags.EXCEPTION;
31-
3220
import org.openqa.selenium.Capabilities;
3321
import org.openqa.selenium.ImmutableCapabilities;
3422
import org.openqa.selenium.SessionNotCreatedException;
@@ -46,8 +34,6 @@
4634
import org.openqa.selenium.remote.ProtocolHandshake;
4735
import org.openqa.selenium.remote.Response;
4836
import org.openqa.selenium.remote.SessionId;
49-
import org.openqa.selenium.remote.http.AddSeleniumUserAgent;
50-
import org.openqa.selenium.remote.http.ClientConfig;
5137
import org.openqa.selenium.remote.http.Contents;
5238
import org.openqa.selenium.remote.http.HttpClient;
5339
import org.openqa.selenium.remote.http.HttpMethod;
@@ -69,6 +55,18 @@
6955
import java.util.logging.Level;
7056
import java.util.logging.Logger;
7157

58+
import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES;
59+
import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES_EVENT;
60+
import static org.openqa.selenium.remote.tracing.AttributeKey.DOWNSTREAM_DIALECT;
61+
import static org.openqa.selenium.remote.tracing.AttributeKey.DRIVER_RESPONSE;
62+
import static org.openqa.selenium.remote.tracing.AttributeKey.DRIVER_URL;
63+
import static org.openqa.selenium.remote.tracing.AttributeKey.EXCEPTION_EVENT;
64+
import static org.openqa.selenium.remote.tracing.AttributeKey.EXCEPTION_MESSAGE;
65+
import static org.openqa.selenium.remote.tracing.AttributeKey.LOGGER_CLASS;
66+
import static org.openqa.selenium.remote.tracing.AttributeKey.UPSTREAM_DIALECT;
67+
import static org.openqa.selenium.remote.tracing.EventAttribute.setValue;
68+
import static org.openqa.selenium.remote.tracing.Tags.EXCEPTION;
69+
7270
public class RelaySessionFactory implements SessionFactory {
7371

7472
private static final Logger LOG = Logger.getLogger(RelaySessionFactory.class.getName());
@@ -201,10 +199,7 @@ public boolean isServiceUp() {
201199
return true;
202200
}
203201
try {
204-
ClientConfig config = ClientConfig.defaultConfig()
205-
.baseUri(serviceStatusUrl.toURI())
206-
.filter(new AddSeleniumUserAgent());
207-
HttpClient client = clientFactory.createClient(config);
202+
HttpClient client = clientFactory.createClient(serviceStatusUrl);
208203
HttpResponse response = client
209204
.execute(new HttpRequest(HttpMethod.GET, serviceStatusUrl.toString()));
210205
LOG.log(Debug.getDebugLogLevel(), Contents.string(response));

‎java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,6 @@
1717

1818
package org.openqa.selenium.grid.node.remote;
1919

20-
import static java.net.HttpURLConnection.HTTP_OK;
21-
import static org.openqa.selenium.grid.data.Availability.DOWN;
22-
import static org.openqa.selenium.grid.data.Availability.DRAINING;
23-
import static org.openqa.selenium.grid.data.Availability.UP;
24-
import static org.openqa.selenium.remote.http.Contents.asJson;
25-
import static org.openqa.selenium.remote.http.Contents.reader;
26-
import static org.openqa.selenium.remote.http.HttpMethod.DELETE;
27-
import static org.openqa.selenium.remote.http.HttpMethod.GET;
28-
import static org.openqa.selenium.remote.http.HttpMethod.POST;
29-
3020
import com.google.common.collect.ImmutableMap;
3121
import com.google.common.collect.ImmutableSet;
3222

@@ -50,8 +40,6 @@
5040
import org.openqa.selenium.json.Json;
5141
import org.openqa.selenium.json.JsonInput;
5242
import org.openqa.selenium.remote.SessionId;
53-
import org.openqa.selenium.remote.http.AddSeleniumUserAgent;
54-
import org.openqa.selenium.remote.http.ClientConfig;
5543
import org.openqa.selenium.remote.http.Filter;
5644
import org.openqa.selenium.remote.http.HttpClient;
5745
import org.openqa.selenium.remote.http.HttpHandler;
@@ -70,6 +58,17 @@
7058
import java.util.Optional;
7159
import java.util.Set;
7260

61+
import static java.net.HttpURLConnection.HTTP_OK;
62+
import static org.openqa.selenium.grid.data.Availability.DOWN;
63+
import static org.openqa.selenium.grid.data.Availability.DRAINING;
64+
import static org.openqa.selenium.grid.data.Availability.UP;
65+
import static org.openqa.selenium.net.Urls.fromUri;
66+
import static org.openqa.selenium.remote.http.Contents.asJson;
67+
import static org.openqa.selenium.remote.http.Contents.reader;
68+
import static org.openqa.selenium.remote.http.HttpMethod.DELETE;
69+
import static org.openqa.selenium.remote.http.HttpMethod.GET;
70+
import static org.openqa.selenium.remote.http.HttpMethod.POST;
71+
7372
public class RemoteNode extends Node {
7473

7574
public static final Json JSON = new Json();
@@ -90,14 +89,8 @@ public RemoteNode(
9089
this.externalUri = Require.nonNull("External URI", externalUri);
9190
this.capabilities = ImmutableSet.copyOf(capabilities);
9291

93-
HttpClient.Factory httpClientFactory = Require.nonNull("HTTP client factory", clientFactory);
94-
// A client config without RetryRequest(), not needed to monitor the Nodes because the
95-
// health checks implicitly do the retries.
96-
ClientConfig config = ClientConfig
97-
.defaultConfig()
98-
.baseUri(externalUri)
99-
.filter(new AddSeleniumUserAgent());
100-
this.client = httpClientFactory.createClient(config);
92+
this.client = Require.nonNull("HTTP client factory", clientFactory)
93+
.createClient(fromUri(externalUri));
10194

10295
this.healthCheck = new RemoteCheck();
10396

@@ -260,11 +253,12 @@ public void drain() {
260253
}
261254
}
262255

256+
@SuppressWarnings("unused")
263257
private Map<String, Object> toJson() {
264258
return ImmutableMap.of(
265-
"id", getId(),
266-
"uri", externalUri,
267-
"capabilities", capabilities);
259+
"id", getId(),
260+
"uri", externalUri,
261+
"capabilities", capabilities);
268262
}
269263

270264
private class RemoteCheck implements HealthCheck {

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@
3131
public class ClientConfig {
3232

3333
private static final Filter RETRY_FILTER = new RetryRequest();
34-
private static final Filter SELENIUM_USER_AGENT_FILTER = new AddSeleniumUserAgent();
35-
private static final Filter DEFAULT_FILTERS = SELENIUM_USER_AGENT_FILTER.andThen(RETRY_FILTER);
34+
private static final Filter DEFAULT_FILTER = new AddSeleniumUserAgent();
3635
private final URI baseUri;
3736
private final Duration connectionTimeout;
3837
private final Duration readTimeout;
@@ -60,7 +59,7 @@ public static ClientConfig defaultConfig() {
6059
null,
6160
Duration.ofSeconds(10),
6261
Duration.ofMinutes(3),
63-
DEFAULT_FILTERS,
62+
DEFAULT_FILTER,
6463
null,
6564
null);
6665
}
@@ -113,23 +112,23 @@ public Duration readTimeout() {
113112
return readTimeout;
114113
}
115114

116-
public ClientConfig filter(Filter filter) {
115+
public ClientConfig withFilter(Filter filter) {
116+
Require.nonNull("Filter", filter);
117117
return new ClientConfig(
118118
baseUri,
119119
connectionTimeout,
120120
readTimeout,
121-
Require.nonNull("Filter", filter),
121+
filter.andThen(DEFAULT_FILTER),
122122
proxy,
123123
credentials);
124124
}
125125

126-
public ClientConfig withFilter(Filter filter) {
127-
Require.nonNull("Filter", filter);
126+
public ClientConfig withRetries() {
128127
return new ClientConfig(
129128
baseUri,
130129
connectionTimeout,
131130
readTimeout,
132-
filter.andThen(DEFAULT_FILTERS),
131+
filters.andThen(RETRY_FILTER),
133132
proxy,
134133
credentials);
135134
}

0 commit comments

Comments
 (0)