Skip to content

Commit

Permalink
[grid] cancel pending request on client timeout
Browse files Browse the repository at this point in the history
  • Loading branch information
joerg1985 committed Oct 30, 2024
1 parent db343b9 commit 7175349
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 5 deletions.
21 changes: 16 additions & 5 deletions java/src/org/openqa/selenium/netty/server/SeleniumHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@

import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.SimpleChannelInboundHandler;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import org.openqa.selenium.internal.Require;
import org.openqa.selenium.remote.ErrorFilter;
import org.openqa.selenium.remote.http.HttpHandler;
Expand All @@ -31,18 +33,27 @@ class SeleniumHandler extends SimpleChannelInboundHandler<HttpRequest> {

private static final ExecutorService EXECUTOR = Executors.newCachedThreadPool();
private final HttpHandler seleniumHandler;
private Future<?> lastOne;

public SeleniumHandler(HttpHandler seleniumHandler) {
super(HttpRequest.class);
this.seleniumHandler = Require.nonNull("HTTP handler", seleniumHandler).with(new ErrorFilter());
this.lastOne = CompletableFuture.completedFuture(null);
}

@Override
protected void channelRead0(ChannelHandlerContext ctx, HttpRequest msg) {
EXECUTOR.submit(
() -> {
HttpResponse res = seleniumHandler.execute(msg);
ctx.writeAndFlush(res);
});
lastOne =
EXECUTOR.submit(
() -> {
HttpResponse res = seleniumHandler.execute(msg);
ctx.writeAndFlush(res);
});
}

@Override
public void channelInactive(ChannelHandlerContext ctx) throws Exception {
lastOne.cancel(true);
super.channelInactive(ctx);
}
}
43 changes: 43 additions & 0 deletions java/test/org/openqa/selenium/netty/server/NettyServerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,20 @@

import com.google.common.collect.ImmutableMap;
import java.net.URL;
import java.time.Duration;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.openqa.selenium.TimeoutException;
import org.openqa.selenium.grid.config.CompoundConfig;
import org.openqa.selenium.grid.config.Config;
import org.openqa.selenium.grid.config.MapConfig;
import org.openqa.selenium.grid.server.BaseServerOptions;
import org.openqa.selenium.grid.server.Server;
import org.openqa.selenium.net.PortProber;
import org.openqa.selenium.remote.http.ClientConfig;
import org.openqa.selenium.remote.http.HttpClient;
import org.openqa.selenium.remote.http.HttpRequest;
import org.openqa.selenium.remote.http.HttpResponse;
Expand Down Expand Up @@ -141,6 +147,43 @@ void shouldNotBindToHost() {
assertEquals("anyRandomHost", server.getUrl().getHost());
}

@Test
void doesInterruptPending() throws Exception {
CountDownLatch interrupted = new CountDownLatch(1);
Config cfg = new MapConfig(ImmutableMap.of());
BaseServerOptions options = new BaseServerOptions(cfg);

Server<?> server =
new NettyServer(
options,
req -> {
try {
Thread.sleep(800);
} catch (InterruptedException ex) {
interrupted.countDown();
}
return new HttpResponse();
})
.start();
ClientConfig config =
ClientConfig.defaultConfig()
.readTimeout(Duration.ofMillis(400))
.baseUri(server.getUrl().toURI());

// provoke a client timeout
Assertions.assertThrows(
TimeoutException.class,
() -> {
try (HttpClient client = HttpClient.Factory.createDefault().createClient(config)) {
HttpRequest request = new HttpRequest(DELETE, "/session");
request.setHeader("Accept", "*/*");
client.execute(request);
}
});

assertTrue(interrupted.await(1000, TimeUnit.MILLISECONDS), "The handling was interrupted");
}

private void outputHeaders(HttpResponse res) {
res.forEachHeader((name, value) -> System.out.printf("%s -> %s\n", name, value));
}
Expand Down

1 comment on commit 7175349

@joerg1985
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cancle the upstream request as soon as the client does timeout. The client forwarding the request does use the session timeout as read timeout, this might be much longer than the client read timeout.

Please sign in to comment.