From 63a64deb563f91adcefffe6e1bcf88e747935562 Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Tue, 12 Aug 2025 15:34:24 +0700 Subject: [PATCH 1/3] [grid] Improve readTimeout in handle session between Router and Node Signed-off-by: Viet Nguyen Duc --- .../selenium/grid/router/HandleSession.java | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/router/HandleSession.java b/java/src/org/openqa/selenium/grid/router/HandleSession.java index b199f0a51f356..8bfbb08591022 100644 --- a/java/src/org/openqa/selenium/grid/router/HandleSession.java +++ b/java/src/org/openqa/selenium/grid/router/HandleSession.java @@ -21,13 +21,17 @@ import static org.openqa.selenium.remote.RemoteTags.SESSION_ID; import static org.openqa.selenium.remote.RemoteTags.SESSION_ID_EVENT; import static org.openqa.selenium.remote.http.Contents.asJson; +import static org.openqa.selenium.remote.http.Contents.reader; +import static org.openqa.selenium.remote.http.HttpMethod.GET; import static org.openqa.selenium.remote.tracing.Tags.EXCEPTION; import static org.openqa.selenium.remote.tracing.Tags.HTTP_REQUEST; import static org.openqa.selenium.remote.tracing.Tags.HTTP_REQUEST_EVENT; import static org.openqa.selenium.remote.tracing.Tags.HTTP_RESPONSE; import java.io.Closeable; +import java.io.Reader; import java.net.URI; +import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Iterator; @@ -43,9 +47,12 @@ import org.openqa.selenium.NoSuchSessionException; import org.openqa.selenium.concurrent.ExecutorServices; import org.openqa.selenium.concurrent.GuardedRunnable; +import org.openqa.selenium.grid.data.NodeStatus; import org.openqa.selenium.grid.sessionmap.SessionMap; import org.openqa.selenium.grid.web.ReverseProxyHandler; import org.openqa.selenium.internal.Require; +import org.openqa.selenium.json.Json; +import org.openqa.selenium.json.JsonInput; import org.openqa.selenium.remote.ErrorCodec; import org.openqa.selenium.remote.SessionId; import org.openqa.selenium.remote.http.ClientConfig; @@ -229,8 +236,7 @@ private Callable loadSessionId( return entry; } - ClientConfig config = - ClientConfig.defaultConfig().baseUri(sessionUri).withRetries(); + ClientConfig config = fetchNodeSessionTimeout(sessionUri); HttpClient httpClient = httpClientFactory.createClient(config); return new CacheEntry(httpClient, 1); @@ -246,6 +252,49 @@ private Callable loadSessionId( }); } + private ClientConfig fetchNodeSessionTimeout(URI uri) { + ClientConfig config = ClientConfig.defaultConfig().baseUri(uri).withRetries(); + Duration sessionTimeout = config.readTimeout(); + try (HttpClient httpClient = httpClientFactory.createClient(config)) { + HttpRequest statusRequest = new HttpRequest(GET, "/status"); + HttpResponse res = httpClient.execute(statusRequest); + Reader reader = reader(res); + Json JSON = new Json(); + JsonInput in = JSON.newInput(reader); + in.beginObject(); + // Skip everything until we find "value" + while (in.hasNext()) { + if ("value".equals(in.nextName())) { + in.beginObject(); + while (in.hasNext()) { + if ("node".equals(in.nextName())) { + NodeStatus nodeStatus = in.read(NodeStatus.class); + sessionTimeout = nodeStatus.getSessionTimeout(); + LOG.fine( + "Fetched session timeout from node status (read timeout: " + + sessionTimeout.toSeconds() + + " seconds) for " + + uri); + } else { + in.skipValue(); + } + } + in.endObject(); + } else { + in.skipValue(); + } + } + } catch (Exception e) { + LOG.fine( + "Use default from ClientConfig (read timeout: " + + config.readTimeout().toSeconds() + + " seconds) for " + + uri); + } + config = config.readTimeout(sessionTimeout); + return config; + } + @Override public void close() { ExecutorServices.shutdownGracefully( From f299fae9f9f639bcdec8828c52f279a1652653d7 Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Sun, 17 Aug 2025 23:18:09 +0700 Subject: [PATCH 2/3] Fix review comment Signed-off-by: Viet Nguyen Duc --- .../org/openqa/selenium/grid/node/Node.java | 4 +- .../selenium/grid/router/HandleSession.java | 44 +++++-------------- 2 files changed, 14 insertions(+), 34 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/node/Node.java b/java/src/org/openqa/selenium/grid/node/Node.java index d2b0d5223802b..6a8627fd23ffe 100644 --- a/java/src/org/openqa/selenium/grid/node/Node.java +++ b/java/src/org/openqa/selenium/grid/node/Node.java @@ -199,7 +199,9 @@ protected Node( .to(() -> new Drain(this, json)) .with(spanDecorator("node.drain").andThen(requiresSecret)), get("/se/grid/node/status") - .to(() -> req -> new HttpResponse().setContent(asJson(getStatus()))) + .to( + () -> + req -> new HttpResponse().setContent(asJson(Map.of("value", getStatus())))) .with(spanDecorator("node.node_status")), get("/status").to(() -> new StatusHandler(this)).with(spanDecorator("node.status"))); } diff --git a/java/src/org/openqa/selenium/grid/router/HandleSession.java b/java/src/org/openqa/selenium/grid/router/HandleSession.java index b086e781ee80b..da9e38b4d440d 100644 --- a/java/src/org/openqa/selenium/grid/router/HandleSession.java +++ b/java/src/org/openqa/selenium/grid/router/HandleSession.java @@ -21,7 +21,6 @@ import static org.openqa.selenium.remote.RemoteTags.SESSION_ID; import static org.openqa.selenium.remote.RemoteTags.SESSION_ID_EVENT; import static org.openqa.selenium.remote.http.Contents.asJson; -import static org.openqa.selenium.remote.http.Contents.reader; import static org.openqa.selenium.remote.http.HttpMethod.GET; import static org.openqa.selenium.remote.tracing.Tags.EXCEPTION; import static org.openqa.selenium.remote.tracing.Tags.HTTP_REQUEST; @@ -29,7 +28,6 @@ import static org.openqa.selenium.remote.tracing.Tags.HTTP_RESPONSE; import java.io.Closeable; -import java.io.Reader; import java.net.URI; import java.time.Duration; import java.time.Instant; @@ -50,9 +48,8 @@ import org.openqa.selenium.grid.data.NodeStatus; import org.openqa.selenium.grid.sessionmap.SessionMap; import org.openqa.selenium.grid.web.ReverseProxyHandler; +import org.openqa.selenium.grid.web.Values; import org.openqa.selenium.internal.Require; -import org.openqa.selenium.json.Json; -import org.openqa.selenium.json.JsonInput; import org.openqa.selenium.remote.ErrorCodec; import org.openqa.selenium.remote.SessionId; import org.openqa.selenium.remote.http.ClientConfig; @@ -237,7 +234,7 @@ private Callable loadSessionId( return entry; } - ClientConfig config = fetchNodeSessionTimeout(sessionUri); + ClientConfig config = fetchNodeSessionTimeout(sessionUri).withRetries(); HttpClient httpClient = httpClientFactory.createClient(config); return new CacheEntry(httpClient, 1); @@ -254,37 +251,18 @@ private Callable loadSessionId( } private ClientConfig fetchNodeSessionTimeout(URI uri) { - ClientConfig config = ClientConfig.defaultConfig().baseUri(uri).withRetries(); + ClientConfig config = ClientConfig.defaultConfig().baseUri(uri); Duration sessionTimeout = config.readTimeout(); try (HttpClient httpClient = httpClientFactory.createClient(config)) { - HttpRequest statusRequest = new HttpRequest(GET, "/status"); + HttpRequest statusRequest = new HttpRequest(GET, "/se/grid/node/status"); HttpResponse res = httpClient.execute(statusRequest); - Reader reader = reader(res); - Json JSON = new Json(); - JsonInput in = JSON.newInput(reader); - in.beginObject(); - // Skip everything until we find "value" - while (in.hasNext()) { - if ("value".equals(in.nextName())) { - in.beginObject(); - while (in.hasNext()) { - if ("node".equals(in.nextName())) { - NodeStatus nodeStatus = in.read(NodeStatus.class); - sessionTimeout = nodeStatus.getSessionTimeout(); - LOG.fine( - "Fetched session timeout from node status (read timeout: " - + sessionTimeout.toSeconds() - + " seconds) for " - + uri); - } else { - in.skipValue(); - } - } - in.endObject(); - } else { - in.skipValue(); - } - } + NodeStatus nodeStatus = Values.get(res, NodeStatus.class); + sessionTimeout = nodeStatus.getSessionTimeout(); + LOG.fine( + "Fetched session timeout from node status (read timeout: " + + sessionTimeout.toSeconds() + + " seconds) for " + + uri); } catch (Exception e) { LOG.fine( "Use default from ClientConfig (read timeout: " From 41644c6bc6f10b2c01bc3eda486473e07b2df733 Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Tue, 19 Aug 2025 08:18:45 +0700 Subject: [PATCH 3/3] Check null might return from Values.get() and remove catch block Signed-off-by: Viet Nguyen Duc --- .../selenium/grid/router/HandleSession.java | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/router/HandleSession.java b/java/src/org/openqa/selenium/grid/router/HandleSession.java index da9e38b4d440d..d557a890f974e 100644 --- a/java/src/org/openqa/selenium/grid/router/HandleSession.java +++ b/java/src/org/openqa/selenium/grid/router/HandleSession.java @@ -257,19 +257,11 @@ private ClientConfig fetchNodeSessionTimeout(URI uri) { HttpRequest statusRequest = new HttpRequest(GET, "/se/grid/node/status"); HttpResponse res = httpClient.execute(statusRequest); NodeStatus nodeStatus = Values.get(res, NodeStatus.class); - sessionTimeout = nodeStatus.getSessionTimeout(); - LOG.fine( - "Fetched session timeout from node status (read timeout: " - + sessionTimeout.toSeconds() - + " seconds) for " - + uri); - } catch (Exception e) { - LOG.fine( - "Use default from ClientConfig (read timeout: " - + config.readTimeout().toSeconds() - + " seconds) for " - + uri); + if (nodeStatus != null) { + sessionTimeout = nodeStatus.getSessionTimeout(); + } } + LOG.fine("Set read timeout: " + sessionTimeout.toSeconds() + " seconds for " + uri); config = config.readTimeout(sessionTimeout); return config; }