From 1501704793e552837a35eda7f8875d45dcc02835 Mon Sep 17 00:00:00 2001 From: Andrei Solntsev Date: Wed, 21 Feb 2024 21:17:59 +0200 Subject: [PATCH] #1687 throw an exception if Grid response contains an error message --- .../grid/DownloadedFilesResponse.java | 2 +- .../selenide/grid/FileContentResponse.java | 2 +- .../java/org/selenide/grid/GridClient.java | 18 ++++++--- .../java/org/selenide/grid/GridResponse.java | 9 +++++ .../org/selenide/grid/GridClientTest.java | 38 +++++++++---------- 5 files changed, 43 insertions(+), 26 deletions(-) create mode 100644 modules/grid/src/main/java/org/selenide/grid/GridResponse.java diff --git a/modules/grid/src/main/java/org/selenide/grid/DownloadedFilesResponse.java b/modules/grid/src/main/java/org/selenide/grid/DownloadedFilesResponse.java index c2934ecc45..34c0a83494 100644 --- a/modules/grid/src/main/java/org/selenide/grid/DownloadedFilesResponse.java +++ b/modules/grid/src/main/java/org/selenide/grid/DownloadedFilesResponse.java @@ -11,5 +11,5 @@ record DownloadedFiles( @Nullable String error, @Nullable String message, @Nullable String stacktrace -) { +) implements GridResponse { } diff --git a/modules/grid/src/main/java/org/selenide/grid/FileContentResponse.java b/modules/grid/src/main/java/org/selenide/grid/FileContentResponse.java index ac2a0737b6..7c071b2ccd 100644 --- a/modules/grid/src/main/java/org/selenide/grid/FileContentResponse.java +++ b/modules/grid/src/main/java/org/selenide/grid/FileContentResponse.java @@ -11,5 +11,5 @@ record FileContent( @Nullable String error, @Nullable String message, @Nullable String stacktrace -) { +) implements GridResponse { } diff --git a/modules/grid/src/main/java/org/selenide/grid/GridClient.java b/modules/grid/src/main/java/org/selenide/grid/GridClient.java index 6f4b80acda..b194d38ded 100644 --- a/modules/grid/src/main/java/org/selenide/grid/GridClient.java +++ b/modules/grid/src/main/java/org/selenide/grid/GridClient.java @@ -24,6 +24,7 @@ import java.util.function.Supplier; import static java.time.Duration.ofSeconds; +import static org.apache.commons.lang3.StringUtils.isNotBlank; import static org.openqa.selenium.json.Json.JSON_UTF_8; import static org.openqa.selenium.remote.http.HttpClient.Factory.createDefault; import static org.openqa.selenium.remote.http.HttpMethod.DELETE; @@ -64,7 +65,7 @@ public List downloads() { String uri = "%s/session/%s/se/files".formatted(baseUrl, sessionId); HttpRequest request = new HttpRequest(GET, uri); HttpResponse response = client.execute(request); - List fileNames = parseDownloadedFiles(Contents.string(response)).names(); + List fileNames = verifyNoErrors(parseDownloadedFiles(Contents.string(response))).names(); log.debug("Retrieved files from {}: {}", uri, fileNames); return fileNames; } @@ -78,6 +79,13 @@ DownloadedFiles parseDownloadedFiles(String responseJson) throws JsonProcessingE return downloadedFiles.value(); } + private T verifyNoErrors(T response) { + if (isNotBlank(response.error())) { + throw new RuntimeException("%s: %s".formatted(response.error(), response.message())); + } + return response; + } + @CheckReturnValue @Nonnull public File download(String fileName, File targetFolder) { @@ -87,8 +95,8 @@ public File download(String fileName, File targetFolder) { .addHeader("Content-Type", JSON_UTF_8) .setContent(toJson(new FileRequest(fileName))); HttpResponse response = client.execute(request); - FileContentResponse fileContent = parseDownloadedFile(Contents.string(response)); - Zip.unzip(fileContent.value().contents(), targetFolder); + FileContent fileContent = verifyNoErrors(parseDownloadedFile(Contents.string(response))); + Zip.unzip(fileContent.contents(), targetFolder); File file = new File(targetFolder, fileName); log.debug("Downloaded file from {} to {}", uri, file.getAbsolutePath()); return file; @@ -110,8 +118,8 @@ private Supplier toJson(Object content) { }; } - FileContentResponse parseDownloadedFile(String responseJson) throws JsonProcessingException { - return json.readValue(responseJson, FileContentResponse.class); + FileContent parseDownloadedFile(String responseJson) throws JsonProcessingException { + return json.readValue(responseJson, FileContentResponse.class).value(); } public void deleteDownloadedFiles() { diff --git a/modules/grid/src/main/java/org/selenide/grid/GridResponse.java b/modules/grid/src/main/java/org/selenide/grid/GridResponse.java new file mode 100644 index 0000000000..0e8cb5e90b --- /dev/null +++ b/modules/grid/src/main/java/org/selenide/grid/GridResponse.java @@ -0,0 +1,9 @@ +package org.selenide.grid; + +import javax.annotation.Nullable; + +interface GridResponse { + @Nullable String error(); + @Nullable String message(); + @Nullable String stacktrace(); +} diff --git a/modules/grid/src/test/java/org/selenide/grid/GridClientTest.java b/modules/grid/src/test/java/org/selenide/grid/GridClientTest.java index 0d55f0d247..fb06406253 100644 --- a/modules/grid/src/test/java/org/selenide/grid/GridClientTest.java +++ b/modules/grid/src/test/java/org/selenide/grid/GridClientTest.java @@ -8,7 +8,7 @@ import static org.assertj.core.api.Assertions.assertThat; class GridClientTest { - private final GridClient client = new GridClient("", ""); + private final GridClient client = new GridClient("https://my.hub.com", "sid12345"); @Test void parsesDownloadedFilesResponse() throws JsonProcessingException { @@ -54,12 +54,12 @@ void parsesDownloadedFileContents() throws JsonProcessingException { "contents": "%s" } }""".formatted(base64zip); - FileContentResponse response = client.parseDownloadedFile(responseJson); - assertThat(response.value().error()).isNull(); - assertThat(response.value().message()).isNull(); - assertThat(response.value().stacktrace()).isNull(); - assertThat(response.value().filename()).isEqualTo("hello_world.txt"); - assertThat(response.value().contents()).startsWith("UEsDBBQACAg").endsWith("AAAA=="); + FileContent response = client.parseDownloadedFile(responseJson); + assertThat(response.error()).isNull(); + assertThat(response.message()).isNull(); + assertThat(response.stacktrace()).isNull(); + assertThat(response.filename()).isEqualTo("hello_world.txt"); + assertThat(response.contents()).startsWith("UEsDBBQACAg").endsWith("AAAA=="); } @Test @@ -72,12 +72,12 @@ void errorResponse() throws JsonProcessingException { "stacktrace": "" } }"""; - FileContentResponse response = client.parseDownloadedFile(responseJson); - assertThat(response.value().error()).isEqualTo("unknown error"); - assertThat(response.value().message()).isEqualTo("Content-Type header is missing"); - assertThat(response.value().stacktrace()).isEqualTo(""); - assertThat(response.value().filename()).isNull(); - assertThat(response.value().contents()).isNull(); + FileContent response = client.parseDownloadedFile(responseJson); + assertThat(response.error()).isEqualTo("unknown error"); + assertThat(response.message()).isEqualTo("Content-Type header is missing"); + assertThat(response.stacktrace()).isEqualTo(""); + assertThat(response.filename()).isNull(); + assertThat(response.contents()).isNull(); } @Test @@ -90,11 +90,11 @@ void errorResponseWithStacktrace() throws JsonProcessingException { "error": "unknown error" } }"""; - FileContentResponse response = client.parseDownloadedFile(responseJson); - assertThat(response.value().error()).isEqualTo("unknown error"); - assertThat(response.value().message()).startsWith("Please specify file to download"); - assertThat(response.value().stacktrace()).startsWith("org.openqa.selenium.WebDriverException"); - assertThat(response.value().filename()).isNull(); - assertThat(response.value().contents()).isNull(); + FileContent response = client.parseDownloadedFile(responseJson); + assertThat(response.error()).isEqualTo("unknown error"); + assertThat(response.message()).startsWith("Please specify file to download"); + assertThat(response.stacktrace()).startsWith("org.openqa.selenium.WebDriverException"); + assertThat(response.filename()).isNull(); + assertThat(response.contents()).isNull(); } }