Skip to content

Commit b58f975

Browse files
diemoldiemol
committed
[grid] Bubbling up session creation exception to client
1 parent ad30bcd commit b58f975

File tree

10 files changed

+123
-102
lines changed

10 files changed

+123
-102
lines changed

‎java/server/src/org/openqa/selenium/grid/docker/DockerSessionFactory.java

+19-14
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,20 @@
1717

1818
package org.openqa.selenium.grid.docker;
1919

20+
import static java.util.Optional.ofNullable;
21+
import static org.openqa.selenium.docker.ContainerConfig.image;
22+
import static org.openqa.selenium.remote.Dialect.W3C;
23+
import static org.openqa.selenium.remote.http.Contents.string;
24+
import static org.openqa.selenium.remote.http.HttpMethod.GET;
25+
import static org.openqa.selenium.remote.tracing.Tags.EXCEPTION;
26+
2027
import org.openqa.selenium.Capabilities;
2128
import org.openqa.selenium.Dimension;
2229
import org.openqa.selenium.ImmutableCapabilities;
30+
import org.openqa.selenium.RetrySessionRequestException;
2331
import org.openqa.selenium.SessionNotCreatedException;
2432
import org.openqa.selenium.TimeoutException;
33+
import org.openqa.selenium.WebDriverException;
2534
import org.openqa.selenium.docker.Container;
2635
import org.openqa.selenium.docker.ContainerInfo;
2736
import org.openqa.selenium.docker.Docker;
@@ -30,6 +39,7 @@
3039
import org.openqa.selenium.grid.data.CreateSessionRequest;
3140
import org.openqa.selenium.grid.node.ActiveSession;
3241
import org.openqa.selenium.grid.node.SessionFactory;
42+
import org.openqa.selenium.internal.Either;
3343
import org.openqa.selenium.internal.Require;
3444
import org.openqa.selenium.json.Json;
3545
import org.openqa.selenium.net.PortProber;
@@ -71,13 +81,6 @@
7181
import java.util.logging.Level;
7282
import java.util.logging.Logger;
7383

74-
import static java.util.Optional.ofNullable;
75-
import static org.openqa.selenium.docker.ContainerConfig.image;
76-
import static org.openqa.selenium.remote.Dialect.W3C;
77-
import static org.openqa.selenium.remote.http.Contents.string;
78-
import static org.openqa.selenium.remote.http.HttpMethod.GET;
79-
import static org.openqa.selenium.remote.tracing.Tags.EXCEPTION;
80-
8184
public class DockerSessionFactory implements SessionFactory {
8285

8386
private static final Logger LOG = Logger.getLogger(DockerSessionFactory.class.getName());
@@ -122,7 +125,7 @@ public boolean test(Capabilities capabilities) {
122125
}
123126

124127
@Override
125-
public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
128+
public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sessionRequest) {
126129
LOG.info("Starting session for " + sessionRequest.getCapabilities());
127130
int port = PortProber.findFreePort();
128131
URL remoteAddress = getUrl(port);
@@ -158,9 +161,10 @@ public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
158161
span.addEvent(AttributeKey.EXCEPTION_EVENT.getKey(), attributeMap);
159162

160163
container.stop(Duration.ofMinutes(1));
161-
LOG.warning(String.format(
162-
"Unable to connect to docker server (container id: %s)", container.getId()));
163-
return Optional.empty();
164+
String message = String.format(
165+
"Unable to connect to docker server (container id: %s)", container.getId());
166+
LOG.warning(message);
167+
return Either.left(new RetrySessionRequestException(message));
164168
}
165169
LOG.info(String.format("Server is ready (container id: %s)", container.getId()));
166170

@@ -187,8 +191,9 @@ public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
187191
span.addEvent(AttributeKey.EXCEPTION_EVENT.getKey(), attributeMap);
188192

189193
container.stop(Duration.ofMinutes(1));
190-
LOG.log(Level.WARNING, "Unable to create session: " + e.getMessage(), e);
191-
return Optional.empty();
194+
String message = "Unable to create session: " + e.getMessage();
195+
LOG.log(Level.WARNING, message, e);
196+
return Either.left(new SessionNotCreatedException(message));
192197
}
193198

194199
SessionId id = new SessionId(response.getSessionId());
@@ -221,7 +226,7 @@ public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
221226
id,
222227
capabilities,
223228
container.getId()));
224-
return Optional.of(new DockerSession(
229+
return Either.right(new DockerSession(
225230
container,
226231
videoContainer,
227232
tracer,

‎java/server/src/org/openqa/selenium/grid/node/SessionFactory.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,15 @@
1818
package org.openqa.selenium.grid.node;
1919

2020
import org.openqa.selenium.Capabilities;
21+
import org.openqa.selenium.WebDriverException;
2122
import org.openqa.selenium.grid.data.CreateSessionRequest;
23+
import org.openqa.selenium.internal.Either;
2224

23-
import java.util.Optional;
2425
import java.util.function.Function;
2526
import java.util.function.Predicate;
2627

2728
public interface SessionFactory extends
28-
Function<CreateSessionRequest, Optional<ActiveSession>>,
29-
Predicate<Capabilities> {
29+
Function<CreateSessionRequest, Either<WebDriverException, ActiveSession>>,
30+
Predicate<Capabilities> {
31+
3032
}

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

+17-11
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,21 @@
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+
2024
import org.openqa.selenium.Capabilities;
2125
import org.openqa.selenium.ImmutableCapabilities;
2226
import org.openqa.selenium.PersistentCapabilities;
27+
import org.openqa.selenium.SessionNotCreatedException;
28+
import org.openqa.selenium.WebDriverException;
2329
import org.openqa.selenium.chromium.ChromiumDevToolsLocator;
2430
import org.openqa.selenium.grid.data.CreateSessionRequest;
2531
import org.openqa.selenium.grid.node.ActiveSession;
2632
import org.openqa.selenium.grid.node.ProtocolConvertingSession;
2733
import org.openqa.selenium.grid.node.SessionFactory;
34+
import org.openqa.selenium.internal.Either;
2835
import org.openqa.selenium.internal.Require;
2936
import org.openqa.selenium.remote.Command;
3037
import org.openqa.selenium.remote.Dialect;
@@ -50,10 +57,6 @@
5057
import java.util.Set;
5158
import java.util.function.Predicate;
5259

53-
import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES;
54-
import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES_EVENT;
55-
import static org.openqa.selenium.remote.tracing.Tags.EXCEPTION;
56-
5760
public class DriverServiceSessionFactory implements SessionFactory {
5861

5962
private final Tracer tracer;
@@ -83,13 +86,14 @@ public boolean test(Capabilities capabilities) {
8386
}
8487

8588
@Override
86-
public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
89+
public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sessionRequest) {
8790
if (sessionRequest.getDownstreamDialects().isEmpty()) {
88-
return Optional.empty();
91+
return Either.left(new SessionNotCreatedException("No downstream dialects were found."));
8992
}
9093

9194
if (!test(sessionRequest.getCapabilities())) {
92-
return Optional.empty();
95+
return Either.left(new SessionNotCreatedException("New session request capabilities do not "
96+
+ "match the stereotype."));
9397
}
9498

9599
try (Span span = tracer.getCurrentContext().createSpan("driver_service_factory.apply")) {
@@ -137,7 +141,7 @@ public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
137141
}
138142

139143
span.addEvent("Driver service created session", attributeMap);
140-
return Optional.of(
144+
return Either.right(
141145
new ProtocolConvertingSession(
142146
tracer,
143147
client,
@@ -158,14 +162,16 @@ public void stop() {
158162
span.setAttribute("error", true);
159163
span.setStatus(Status.CANCELLED);
160164
EXCEPTION.accept(attributeMap, e);
165+
String errorMessage = "Error while creating session with the driver service. "
166+
+ "Stopping driver service: " + e.getMessage();
161167
attributeMap.put(AttributeKey.EXCEPTION_MESSAGE.getKey(),
162-
EventAttribute.setValue("Error while creating session with the driver service. Stopping driver service: " + e.getMessage()));
168+
EventAttribute.setValue(errorMessage));
163169
span.addEvent(AttributeKey.EXCEPTION_EVENT.getKey(), attributeMap);
164170
service.stop();
165-
return Optional.empty();
171+
return Either.left(new SessionNotCreatedException(errorMessage));
166172
}
167173
} catch (Exception e) {
168-
return Optional.empty();
174+
return Either.left(new SessionNotCreatedException(e.getMessage()));
169175
}
170176
}
171177

‎java/server/src/org/openqa/selenium/grid/node/local/LocalNode.java

-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
import org.openqa.selenium.NoSuchSessionException;
4242
import org.openqa.selenium.PersistentCapabilities;
4343
import org.openqa.selenium.RetrySessionRequestException;
44-
import org.openqa.selenium.SessionNotCreatedException;
4544
import org.openqa.selenium.WebDriverException;
4645
import org.openqa.selenium.concurrent.Regularly;
4746
import org.openqa.selenium.events.EventBus;

‎java/server/src/org/openqa/selenium/grid/node/local/SessionSlot.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import org.openqa.selenium.remote.http.HttpResponse;
3838

3939
import java.io.UncheckedIOException;
40-
import java.util.Optional;
4140
import java.util.ServiceLoader;
4241
import java.util.UUID;
4342
import java.util.concurrent.atomic.AtomicBoolean;
@@ -136,17 +135,18 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
136135
}
137136

138137
if (!test(sessionRequest.getCapabilities())) {
139-
return Either.left(new SessionNotCreatedException("New session request capabilities does match the stereotype."));
138+
return Either.left(new SessionNotCreatedException("New session request capabilities do not "
139+
+ "match the stereotype."));
140140
}
141141

142142
try {
143-
Optional<ActiveSession> possibleSession = factory.apply(sessionRequest);
144-
if(possibleSession.isPresent()) {
145-
ActiveSession session = possibleSession.get();
143+
Either<WebDriverException, ActiveSession> possibleSession = factory.apply(sessionRequest);
144+
if (possibleSession.isRight()) {
145+
ActiveSession session = possibleSession.right();
146146
currentSession = session;
147147
return Either.right(session);
148148
} else {
149-
return Either.left(new SessionNotCreatedException("Error while creating session with the driver service."));
149+
return Either.left(possibleSession.left());
150150
}
151151
} catch (Exception e) {
152152
LOG.log(Level.WARNING, "Unable to create session", e);

‎java/server/test/org/openqa/selenium/grid/graphql/GraphqlHandlerTest.java

+17-15
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,26 @@
1717

1818
package org.openqa.selenium.grid.graphql;
1919

20+
import static java.util.Collections.singletonList;
21+
import static java.util.Collections.singletonMap;
22+
import static org.assertj.core.api.Assertions.assertThat;
23+
import static org.assertj.core.api.Assertions.fail;
24+
import static org.assertj.core.api.InstanceOfAssertFactories.LIST;
25+
import static org.assertj.core.api.InstanceOfAssertFactories.MAP;
26+
import static org.openqa.selenium.json.Json.MAP_TYPE;
27+
import static org.openqa.selenium.remote.http.Contents.asJson;
28+
import static org.openqa.selenium.remote.http.Contents.utf8String;
29+
import static org.openqa.selenium.remote.http.HttpMethod.GET;
30+
import static org.openqa.selenium.remote.http.HttpMethod.POST;
31+
2032
import com.google.common.collect.ImmutableMap;
33+
2134
import org.junit.Before;
2235
import org.junit.Test;
2336
import org.openqa.selenium.Capabilities;
2437
import org.openqa.selenium.ImmutableCapabilities;
2538
import org.openqa.selenium.SessionNotCreatedException;
39+
import org.openqa.selenium.WebDriverException;
2640
import org.openqa.selenium.events.EventBus;
2741
import org.openqa.selenium.events.local.GuavaEventBus;
2842
import org.openqa.selenium.grid.data.CreateSessionRequest;
@@ -64,22 +78,9 @@
6478
import java.time.Instant;
6579
import java.util.Collections;
6680
import java.util.Map;
67-
import java.util.Optional;
6881
import java.util.Set;
6982
import java.util.UUID;
7083

71-
import static java.util.Collections.singletonList;
72-
import static java.util.Collections.singletonMap;
73-
import static org.assertj.core.api.Assertions.assertThat;
74-
import static org.assertj.core.api.Assertions.fail;
75-
import static org.assertj.core.api.InstanceOfAssertFactories.LIST;
76-
import static org.assertj.core.api.InstanceOfAssertFactories.MAP;
77-
import static org.openqa.selenium.json.Json.MAP_TYPE;
78-
import static org.openqa.selenium.remote.http.Contents.asJson;
79-
import static org.openqa.selenium.remote.http.Contents.utf8String;
80-
import static org.openqa.selenium.remote.http.HttpMethod.GET;
81-
import static org.openqa.selenium.remote.http.HttpMethod.POST;
82-
8384
public class GraphqlHandlerTest {
8485

8586
private static final Json JSON = new Json();
@@ -232,8 +233,9 @@ public void shouldBeAbleToGetUrlsOfAllNodes() throws URISyntaxException {
232233
Node node = LocalNode.builder(tracer, events, new URI(nodeUri), publicUri, registrationSecret)
233234
.add(stereotype, new SessionFactory() {
234235
@Override
235-
public Optional<ActiveSession> apply(CreateSessionRequest createSessionRequest) {
236-
return Optional.empty();
236+
public Either<WebDriverException, ActiveSession> apply(
237+
CreateSessionRequest createSessionRequest) {
238+
return Either.left(new SessionNotCreatedException("Factory for testing"));
237239
}
238240

239241
@Override

‎java/server/test/org/openqa/selenium/grid/node/NodeTest.java

+15-14
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,27 @@
1717

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

20+
import static java.time.Duration.ofSeconds;
21+
import static java.util.concurrent.TimeUnit.SECONDS;
22+
import static org.assertj.core.api.Assertions.assertThat;
23+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
24+
import static org.assertj.core.api.InstanceOfAssertFactories.MAP;
25+
import static org.openqa.selenium.json.Json.MAP_TYPE;
26+
import static org.openqa.selenium.remote.http.Contents.string;
27+
import static org.openqa.selenium.remote.http.HttpMethod.GET;
28+
import static org.openqa.selenium.remote.http.HttpMethod.POST;
29+
2030
import com.google.common.collect.ImmutableMap;
2131
import com.google.common.collect.ImmutableSet;
32+
2233
import org.junit.Before;
2334
import org.junit.Test;
2435
import org.openqa.selenium.Capabilities;
2536
import org.openqa.selenium.ImmutableCapabilities;
2637
import org.openqa.selenium.NoSuchSessionException;
2738
import org.openqa.selenium.RetrySessionRequestException;
2839
import org.openqa.selenium.SessionNotCreatedException;
40+
import org.openqa.selenium.WebDriverException;
2941
import org.openqa.selenium.events.EventBus;
3042
import org.openqa.selenium.events.local.GuavaEventBus;
3143
import org.openqa.selenium.grid.data.CreateSessionRequest;
@@ -57,7 +69,6 @@
5769
import org.openqa.selenium.remote.tracing.Tracer;
5870
import org.openqa.selenium.support.ui.FluentWait;
5971
import org.openqa.selenium.support.ui.Wait;
60-
import org.openqa.selenium.WebDriverException;
6172

6273
import java.io.ByteArrayInputStream;
6374
import java.io.File;
@@ -73,22 +84,11 @@
7384
import java.time.ZoneId;
7485
import java.util.Collections;
7586
import java.util.Map;
76-
import java.util.Optional;
7787
import java.util.UUID;
7888
import java.util.concurrent.CountDownLatch;
7989
import java.util.concurrent.atomic.AtomicBoolean;
8090
import java.util.concurrent.atomic.AtomicReference;
8191

82-
import static java.time.Duration.ofSeconds;
83-
import static java.util.concurrent.TimeUnit.SECONDS;
84-
import static org.assertj.core.api.Assertions.assertThat;
85-
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
86-
import static org.assertj.core.api.InstanceOfAssertFactories.MAP;
87-
import static org.openqa.selenium.json.Json.MAP_TYPE;
88-
import static org.openqa.selenium.remote.http.Contents.string;
89-
import static org.openqa.selenium.remote.http.HttpMethod.GET;
90-
import static org.openqa.selenium.remote.http.HttpMethod.POST;
91-
9292
public class NodeTest {
9393

9494
private Tracer tracer;
@@ -175,8 +175,9 @@ public void shouldRetryIfNoMatchingSlotIsAvailable() {
175175
Node local = LocalNode.builder(tracer, bus, uri, uri, registrationSecret)
176176
.add(caps, new SessionFactory() {
177177
@Override
178-
public Optional<ActiveSession> apply(CreateSessionRequest createSessionRequest) {
179-
return Optional.empty();
178+
public Either<WebDriverException, ActiveSession> apply(
179+
CreateSessionRequest createSessionRequest) {
180+
return Either.left(new SessionNotCreatedException("HelperFactory for testing"));
180181
}
181182

182183
@Override

0 commit comments

Comments
 (0)