[cdp][java] Continue requests without modification for know errors in…

… NetworkInterceptor This is an alternative solution to SeleniumHQ#13836. Related to SeleniumHQ#13774
joebandenburg · Apr 19, 2024 · eee03a3 · eee03a3
1 parent 85c0ad1
commit eee03a3
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 9 deletions.
diff --git a/java/src/org/openqa/selenium/devtools/RequestFailedException.java b/java/src/org/openqa/selenium/devtools/RequestFailedException.java
@@ -0,0 +1,4 @@
+package org.openqa.selenium.devtools;
+
+public class RequestFailedException extends RuntimeException {
+}
diff --git a/java/src/org/openqa/selenium/devtools/idealized/Network.java b/java/src/org/openqa/selenium/devtools/idealized/Network.java
@@ -45,6 +45,7 @@
 import org.openqa.selenium.devtools.DevToolsException;
 import org.openqa.selenium.devtools.Event;
 import org.openqa.selenium.devtools.NetworkInterceptor;
+import org.openqa.selenium.devtools.RequestFailedException;
 import org.openqa.selenium.internal.Either;
 import org.openqa.selenium.internal.Require;
 import org.openqa.selenium.remote.http.Contents;
@@ -200,6 +201,17 @@ public void prepareToInterceptTraffic() {
         pausedRequest -> {
           try {
             String id = getRequestId(pausedRequest);
+
+            if (hasErrorResponse(pausedRequest)) {
+              CompletableFuture<HttpResponse> future = pendingResponses.remove(id);
+              if (future == null) {
+                devTools.send(continueWithoutModification(pausedRequest));
+              } else {
+                future.completeExceptionally(new RequestFailedException());
+              }
+              return;
+            }
+
             Either<HttpRequest, HttpResponse> message = createSeMessages(pausedRequest);
 
             if (message.isRight()) {
@@ -237,6 +249,9 @@ public void prepareToInterceptTraffic() {
                             pendingResponses.remove(id);
                             return STOP_PROCESSING;
                           } catch (ExecutionException e) {
+                            if (e.getCause() instanceof RequestFailedException) {
+                              throw (RequestFailedException) e.getCause();
+                            }
                             if (fetchEnabled.get()) {
                               LOG.log(WARNING, e, () -> "Unable to process request");
                             }
@@ -254,6 +269,8 @@ public void prepareToInterceptTraffic() {
             }
 
             devTools.send(fulfillRequest(pausedRequest, forBrowser));
+          } catch (RequestFailedException e) {
+            devTools.send(continueWithoutModification(pausedRequest));
           } catch (TimeoutException e) {
             if (fetchEnabled.get()) {
               throw e;
@@ -348,6 +365,8 @@ protected abstract Command<Void> continueWithAuth(
 
   protected abstract Either<HttpRequest, HttpResponse> createSeMessages(REQUESTPAUSED pausedReq);
 
+  protected abstract boolean hasErrorResponse(REQUESTPAUSED pausedReq);
+
   protected abstract Command<Void> continueWithoutModification(REQUESTPAUSED pausedReq);
 
   protected abstract Command<Void> continueRequest(REQUESTPAUSED pausedReq, HttpRequest req);

diff --git a/java/src/org/openqa/selenium/devtools/v121/v121Network.java b/java/src/org/openqa/selenium/devtools/v121/v121Network.java
@@ -114,8 +114,7 @@ public Event<RequestPaused> requestPausedEvent() {
 
   @Override
   public Either<HttpRequest, HttpResponse> createSeMessages(RequestPaused pausedReq) {
-    if (pausedReq.getResponseStatusCode().isPresent()
-        || pausedReq.getResponseErrorReason().isPresent()) {
+    if (pausedReq.getResponseStatusCode().isPresent()) {
       String body;
       boolean bodyIsBase64Encoded;
 
@@ -161,6 +160,11 @@ public Either<HttpRequest, HttpResponse> createSeMessages(RequestPaused pausedRe
     return Either.left(req);
   }
 
+  @Override
+  protected boolean hasErrorResponse(RequestPaused pausedReq) {
+    return pausedReq.getResponseErrorReason().isPresent();
+  }
+
   @Override
   protected String getRequestId(RequestPaused pausedReq) {
     return pausedReq.getRequestId().toString();

diff --git a/java/src/org/openqa/selenium/devtools/v122/v122Network.java b/java/src/org/openqa/selenium/devtools/v122/v122Network.java
@@ -114,8 +114,7 @@ public Event<RequestPaused> requestPausedEvent() {
 
   @Override
   public Either<HttpRequest, HttpResponse> createSeMessages(RequestPaused pausedReq) {
-    if (pausedReq.getResponseStatusCode().isPresent()
-        || pausedReq.getResponseErrorReason().isPresent()) {
+    if (pausedReq.getResponseStatusCode().isPresent()) {
       String body;
       boolean bodyIsBase64Encoded;
 
@@ -161,6 +160,11 @@ public Either<HttpRequest, HttpResponse> createSeMessages(RequestPaused pausedRe
     return Either.left(req);
   }
 
+  @Override
+  protected boolean hasErrorResponse(RequestPaused pausedReq) {
+    return pausedReq.getResponseErrorReason().isPresent();
+  }
+
   @Override
   protected String getRequestId(RequestPaused pausedReq) {
     return pausedReq.getRequestId().toString();

diff --git a/java/src/org/openqa/selenium/devtools/v123/v123Network.java b/java/src/org/openqa/selenium/devtools/v123/v123Network.java
@@ -114,8 +114,7 @@ public Event<RequestPaused> requestPausedEvent() {
 
   @Override
   public Either<HttpRequest, HttpResponse> createSeMessages(RequestPaused pausedReq) {
-    if (pausedReq.getResponseStatusCode().isPresent()
-        || pausedReq.getResponseErrorReason().isPresent()) {
+    if (pausedReq.getResponseStatusCode().isPresent()) {
       String body;
       boolean bodyIsBase64Encoded;
 
@@ -161,6 +160,11 @@ public Either<HttpRequest, HttpResponse> createSeMessages(RequestPaused pausedRe
     return Either.left(req);
   }
 
+  @Override
+  protected boolean hasErrorResponse(RequestPaused pausedReq) {
+    return pausedReq.getResponseErrorReason().isPresent();
+  }
+
   @Override
   protected String getRequestId(RequestPaused pausedReq) {
     return pausedReq.getRequestId().toString();

diff --git a/java/src/org/openqa/selenium/devtools/v85/V85Network.java b/java/src/org/openqa/selenium/devtools/v85/V85Network.java
@@ -124,8 +124,7 @@ public Event<RequestPaused> requestPausedEvent() {
 
   @Override
   public Either<HttpRequest, HttpResponse> createSeMessages(RequestPaused pausedReq) {
-    if (pausedReq.getResponseStatusCode().isPresent()
-        || pausedReq.getResponseErrorReason().isPresent()) {
+    if (pausedReq.getResponseStatusCode().isPresent()) {
       String body;
       boolean bodyIsBase64Encoded;
 
@@ -171,6 +170,11 @@ public Either<HttpRequest, HttpResponse> createSeMessages(RequestPaused pausedRe
     return Either.left(req);
   }
 
+  @Override
+  protected boolean hasErrorResponse(RequestPaused pausedReq) {
+    return pausedReq.getResponseErrorReason().isPresent();
+  }
+
   @Override
   protected String getRequestId(RequestPaused pausedReq) {
     return pausedReq.getRequestId().toString();

diff --git a/java/test/org/openqa/selenium/devtools/NetworkInterceptorTest.java b/java/test/org/openqa/selenium/devtools/NetworkInterceptorTest.java
@@ -20,7 +20,8 @@
 import static com.google.common.net.MediaType.XHTML_UTF_8;
 import static java.net.HttpURLConnection.HTTP_MOVED_TEMP;
 import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;
+import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
 import static org.assertj.core.api.Assumptions.assumeThat;
 import static org.openqa.selenium.remote.http.Contents.utf8String;
 import static org.openqa.selenium.testing.Safely.safelyCall;
@@ -35,7 +36,9 @@
 import org.junit.jupiter.api.Test;
 import org.openqa.selenium.By;
 import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.environment.webserver.NettyAppServer;
+import org.openqa.selenium.net.PortProber;
 import org.openqa.selenium.remote.http.Contents;
 import org.openqa.selenium.remote.http.Filter;
 import org.openqa.selenium.remote.http.HttpMethod;
@@ -248,4 +251,31 @@ void shouldHandleRedirects() {
       assertThat(body).contains("Hello, World!");
     }
   }
+
+  @Test
+  @NoDriverBeforeTest
+  void shouldThrowExceptionThroughFilterIfRequestResultInAnKnownError() {
+    Filter filter = next -> req -> {
+      try {
+        return next.execute(req);
+      } catch (RequestFailedException e) {
+        return new HttpResponse().setStatus(200).setContent(Contents.utf8String("Hello, World!"));
+      }
+    };
+    try (NetworkInterceptor ignored = new NetworkInterceptor(driver, filter)) {
+      driver.get("http://localhost:" + PortProber.findFreePort());
+      String body = driver.findElement(By.tagName("body")).getText();
+      assertThat(body).contains("Hello, World!");
+    }
+  }
+
+  @Test
+  @NoDriverBeforeTest
+  void shouldContinueWithRequestIfExceptionThrowFromFilterIfRequestResultInAnKnownError() {
+    Filter filter = next -> next;
+    try (NetworkInterceptor ignored = new NetworkInterceptor(driver, filter)) {
+      assertThatExceptionOfType(WebDriverException.class)
+        .isThrownBy(() -> driver.get("http://localhost:" + PortProber.findFreePort()));
+    }
+  }
 }