Skip to content

Commit

Permalink
#1687 throw an exception if Grid response contains an error message
Browse files Browse the repository at this point in the history
  • Loading branch information
asolntsev committed Feb 22, 2024
1 parent 12ffae0 commit 1501704
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ record DownloadedFiles(
@Nullable String error,
@Nullable String message,
@Nullable String stacktrace
) {
) implements GridResponse {
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ record FileContent(
@Nullable String error,
@Nullable String message,
@Nullable String stacktrace
) {
) implements GridResponse {
}
18 changes: 13 additions & 5 deletions modules/grid/src/main/java/org/selenide/grid/GridClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -64,7 +65,7 @@ public List<String> downloads() {
String uri = "%s/session/%s/se/files".formatted(baseUrl, sessionId);
HttpRequest request = new HttpRequest(GET, uri);
HttpResponse response = client.execute(request);
List<String> fileNames = parseDownloadedFiles(Contents.string(response)).names();
List<String> fileNames = verifyNoErrors(parseDownloadedFiles(Contents.string(response))).names();
log.debug("Retrieved files from {}: {}", uri, fileNames);
return fileNames;
}
Expand All @@ -78,6 +79,13 @@ DownloadedFiles parseDownloadedFiles(String responseJson) throws JsonProcessingE
return downloadedFiles.value();
}

private <T extends GridResponse> 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) {
Expand All @@ -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;
Expand All @@ -110,8 +118,8 @@ private Supplier<InputStream> 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() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package org.selenide.grid;

import javax.annotation.Nullable;

interface GridResponse {
@Nullable String error();
@Nullable String message();
@Nullable String stacktrace();
}
38 changes: 19 additions & 19 deletions modules/grid/src/test/java/org/selenide/grid/GridClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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();
}
}

0 comments on commit 1501704

Please sign in to comment.