From eee03a317266b7c92e6ef7de34c961abe6d1ac50 Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Thu, 18 Apr 2024 13:23:26 +0530 Subject: [PATCH] [cdp][java] Continue requests without modification for know errors in NetworkInterceptor This is an alternative solution to #13836. Related to #13774 --- .../devtools/RequestFailedException.java | 4 +++ .../selenium/devtools/idealized/Network.java | 19 +++++++++++ .../selenium/devtools/v121/v121Network.java | 8 +++-- .../selenium/devtools/v122/v122Network.java | 8 +++-- .../selenium/devtools/v123/v123Network.java | 8 +++-- .../selenium/devtools/v85/V85Network.java | 8 +++-- .../devtools/NetworkInterceptorTest.java | 32 ++++++++++++++++++- 7 files changed, 78 insertions(+), 9 deletions(-) create mode 100644 java/src/org/openqa/selenium/devtools/RequestFailedException.java diff --git a/java/src/org/openqa/selenium/devtools/RequestFailedException.java b/java/src/org/openqa/selenium/devtools/RequestFailedException.java new file mode 100644 index 00000000000000..91b02880cc3426 --- /dev/null +++ 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 index 0725b19b63e5f3..2d39b5b796471b 100644 --- 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 future = pendingResponses.remove(id); + if (future == null) { + devTools.send(continueWithoutModification(pausedRequest)); + } else { + future.completeExceptionally(new RequestFailedException()); + } + return; + } + Either 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 continueWithAuth( protected abstract Either createSeMessages(REQUESTPAUSED pausedReq); + protected abstract boolean hasErrorResponse(REQUESTPAUSED pausedReq); + protected abstract Command continueWithoutModification(REQUESTPAUSED pausedReq); protected abstract Command 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 index 0addc563c81202..d098d429306117 100644 --- 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 requestPausedEvent() { @Override public Either createSeMessages(RequestPaused pausedReq) { - if (pausedReq.getResponseStatusCode().isPresent() - || pausedReq.getResponseErrorReason().isPresent()) { + if (pausedReq.getResponseStatusCode().isPresent()) { String body; boolean bodyIsBase64Encoded; @@ -161,6 +160,11 @@ public Either 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 index 6d5b128d9ac3c8..ea368fd7dab0b7 100644 --- 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 requestPausedEvent() { @Override public Either createSeMessages(RequestPaused pausedReq) { - if (pausedReq.getResponseStatusCode().isPresent() - || pausedReq.getResponseErrorReason().isPresent()) { + if (pausedReq.getResponseStatusCode().isPresent()) { String body; boolean bodyIsBase64Encoded; @@ -161,6 +160,11 @@ public Either 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 index c0686a3c98fabe..df8438cbbb7dc6 100644 --- 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 requestPausedEvent() { @Override public Either createSeMessages(RequestPaused pausedReq) { - if (pausedReq.getResponseStatusCode().isPresent() - || pausedReq.getResponseErrorReason().isPresent()) { + if (pausedReq.getResponseStatusCode().isPresent()) { String body; boolean bodyIsBase64Encoded; @@ -161,6 +160,11 @@ public Either 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 index 1c484924433fb2..da12dbe42802d4 100644 --- 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 requestPausedEvent() { @Override public Either createSeMessages(RequestPaused pausedReq) { - if (pausedReq.getResponseStatusCode().isPresent() - || pausedReq.getResponseErrorReason().isPresent()) { + if (pausedReq.getResponseStatusCode().isPresent()) { String body; boolean bodyIsBase64Encoded; @@ -171,6 +170,11 @@ public Either 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 index d0531526886986..e66f2d8c536e2b 100644 --- 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())); + } + } }