Merge pull request #2953 from cryptomator/feature/error-dialog

Improved Error Dialog
This commit is contained in:
mindmonk
2023-06-16 15:55:43 +02:00
committed by GitHub
6 changed files with 353 additions and 25 deletions

View File

@@ -32,18 +32,15 @@ public class ErrorCode {
this.rootCauseSpecificFrames = rootCauseSpecificFrames;
}
// visible for testing
String methodCode() {
public String methodCode() {
return format(traceCode(rootCause, LATEST_FRAME));
}
// visible for testing
String rootCauseCode() {
public String rootCauseCode() {
return format(traceCode(rootCause, rootCauseSpecificFrames));
}
// visible for testing
String throwableCode() {
public String throwableCode() {
return format(traceCode(throwable, ALL_FRAMES));
}

View File

@@ -1,5 +1,7 @@
package org.cryptomator.ui.error;
import com.google.common.reflect.TypeToken;
import com.google.gson.Gson;
import org.cryptomator.common.Environment;
import org.cryptomator.common.ErrorCode;
import org.cryptomator.common.Nullable;
@@ -9,20 +11,34 @@ import javax.inject.Inject;
import javax.inject.Named;
import javafx.application.Application;
import javafx.application.Platform;
import javafx.beans.binding.BooleanExpression;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleObjectProperty;
import javafx.fxml.FXML;
import javafx.scene.Scene;
import javafx.scene.input.Clipboard;
import javafx.scene.input.ClipboardContent;
import javafx.stage.Stage;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.net.URI;
import java.net.URLEncoder;
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.nio.charset.StandardCharsets;
import java.util.Comparator;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.TimeUnit;
public class ErrorController implements FxController {
private static final String ERROR_CODES_URL = "https://gist.githubusercontent.com/cryptobot/accba9fb9555e7192271b85606f97230/raw/errorcodes.json";
private static final String SEARCH_URL_FORMAT = "https://github.com/cryptomator/cryptomator/discussions/categories/errors?discussions_q=category:Errors+%s";
private static final String REPORT_URL_FORMAT = "https://github.com/cryptomator/cryptomator/discussions/new?category=Errors&title=Error+%s&body=%s";
private static final String SEARCH_ERRORCODE_DELIM = " OR ";
@@ -46,16 +62,28 @@ public class ErrorController implements FxController {
private final Stage window;
private final Environment environment;
private BooleanProperty copiedDetails = new SimpleBooleanProperty();
private final BooleanProperty copiedDetails = new SimpleBooleanProperty();
private final ObjectProperty<ErrorDiscussion> matchingErrorDiscussion = new SimpleObjectProperty<>();
private final BooleanExpression errorSolutionFound = matchingErrorDiscussion.isNotNull();
private final BooleanProperty isLoadingHttpResponse = new SimpleBooleanProperty();
@Inject
ErrorController(Application application, @Named("stackTrace") String stackTrace, ErrorCode errorCode, @Nullable Scene previousScene, Stage window, Environment environment) {
ErrorController(Application application, @Named("stackTrace") String stackTrace, ErrorCode errorCode, @Nullable Scene previousScene, Stage window, Environment environment, ExecutorService executorService) {
this.application = application;
this.stackTrace = stackTrace;
this.errorCode = errorCode;
this.previousScene = previousScene;
this.window = window;
this.environment = environment;
isLoadingHttpResponse.set(true);
HttpClient httpClient = HttpClient.newBuilder().version(HttpClient.Version.HTTP_1_1).build();
HttpRequest httpRequest = HttpRequest.newBuilder()//
.uri(URI.create(ERROR_CODES_URL))//
.build();
httpClient.sendAsync(httpRequest, HttpResponse.BodyHandlers.ofInputStream())//
.thenAcceptAsync(this::loadHttpResponse, executorService)//
.whenCompleteAsync((r, e) -> isLoadingHttpResponse.set(false), Platform::runLater);
}
@FXML
@@ -70,6 +98,16 @@ public class ErrorController implements FxController {
window.close();
}
@FXML
public void showSolution() {
if (matchingErrorDiscussion.isNotNull().get()) {
var discussion = matchingErrorDiscussion.get();
if (discussion != null) {
application.getHostServices().showDocument(discussion.url);
}
}
}
@FXML
public void searchError() {
var searchTerm = URLEncoder.encode(getErrorCode().replace(ErrorCode.DELIM, SEARCH_ERRORCODE_DELIM), StandardCharsets.UTF_8);
@@ -95,13 +133,119 @@ public class ErrorController implements FxController {
Clipboard.getSystemClipboard().setContent(clipboardContent);
copiedDetails.set(true);
CompletableFuture.delayedExecutor(2, TimeUnit.SECONDS, Platform::runLater).execute(() -> {
copiedDetails.set(false);
});
CompletableFuture.delayedExecutor(2, TimeUnit.SECONDS, Platform::runLater).execute(() -> copiedDetails.set(false));
}
private void loadHttpResponse(HttpResponse<InputStream> response) {
if (response.statusCode() == 200) {
Map<String, ErrorDiscussion> errorDiscussionMap = new Gson().fromJson(//
new InputStreamReader(response.body(), StandardCharsets.UTF_8),//
new TypeToken<Map<String, ErrorDiscussion>>() {
}.getType());
if (errorDiscussionMap.values().stream().anyMatch(this::containsMethodCode)) {
Comparator<ErrorDiscussion> comp = this::compareByFullErrorCode;
Optional<ErrorDiscussion> value = errorDiscussionMap.values().stream().filter(this::containsMethodCode)//
.min(comp//
.thenComparing(this::compareByRootCauseCode)//
.thenComparing(this::compareIsAnswered)//
.thenComparing(this::compareUpvoteCount));
if (value.isPresent()) {
matchingErrorDiscussion.set(value.get());
}
}
}
}
/**
* Checks if an ErrorDiscussion object's title contains the error code's method code.
*
* @param errorDiscussion The ErrorDiscussion object to be checked.
* @return A boolean value indicating if the ErrorDiscussion object's title contains the error code's method code:
* - true if the title contains the method code,
* - false otherwise.
*/
public boolean containsMethodCode(ErrorDiscussion errorDiscussion) {
return errorDiscussion.title.contains(" " + errorCode.methodCode());
}
/**
* Compares two ErrorDiscussion objects based on their upvote counts and returns the result.
*
* @param ed1 The first ErrorDiscussion object.
* @param ed2 The second ErrorDiscussion object.
* @return An integer indicating which ErrorDiscussion object has a higher upvote count:
* - A positive value if ed2 has a higher upvote count than ed1,
* - A negative value if ed1 has a higher upvote count than ed2,
* - Or 0 if both upvote counts are equal.
*/
public int compareUpvoteCount(ErrorDiscussion ed1, ErrorDiscussion ed2) {
return Integer.compare(ed2.upvoteCount, ed1.upvoteCount);
}
/**
* Compares two ErrorDiscussion objects based on their answered status and returns the result.
*
* @param ed1 The first ErrorDiscussion object.
* @param ed2 The second ErrorDiscussion object.
* @return An integer indicating the answered status of the ErrorDiscussion objects:
* - A negative value (-1) if ed1 is considered answered and ed2 is considered unanswered,
* - A positive value (1) if ed1 is considered unanswered and ed2 is considered answered,
* - Or 0 if both ErrorDiscussion objects are considered answered or unanswered.
*/
public int compareIsAnswered(ErrorDiscussion ed1, ErrorDiscussion ed2) {
if (ed1.answer != null && ed2.answer == null) {
return -1;
} else if (ed1.answer == null && ed2.answer != null) {
return 1;
} else {
return 0;
}
}
/**
* Compares two ErrorDiscussion objects based on the presence of the full error code in their titles and returns the result.
*
* @param ed1 The first ErrorDiscussion object.
* @param ed2 The second ErrorDiscussion object.
* @return An integer indicating the comparison result based on the presence of the full error code in the titles:
* - A negative value (-1) if ed1 contains the full error code in the title and ed2 does not have a match,
* - A positive value (1) if ed1 does not have a match and ed2 contains the full error code in the title,
* - Or 0 if both ErrorDiscussion objects either contain the full error code or do not have a match in the titles.
*/
public int compareByFullErrorCode(ErrorDiscussion ed1, ErrorDiscussion ed2) {
if (ed1.title.contains(getErrorCode()) && !ed2.title.contains(getErrorCode())) {
return -1;
} else if (!ed1.title.contains(getErrorCode()) && ed2.title.contains(getErrorCode())) {
return 1;
} else {
return 0;
}
}
/**
* Compares two ErrorDiscussion objects based on the presence of the root cause code in their titles and returns the result.
*
* @param ed1 The first ErrorDiscussion object.
* @param ed2 The second ErrorDiscussion object.
* @return An integer indicating the comparison result based on the presence of the root cause code in the titles:
* - A negative value (-1) if ed1 contains the root cause code in the title and ed2 does not have a match,
* - A positive value (1) if ed1 does not have a match and ed2 contains the root cause code in the title,
* - Or 0 if both ErrorDiscussion objects either contain the root cause code or do not have a match in the titles.
*/
public int compareByRootCauseCode(ErrorDiscussion ed1, ErrorDiscussion ed2) {
String value = " " + errorCode.methodCode() + ErrorCode.DELIM + errorCode.rootCauseCode();
if (ed1.title.contains(value) && !ed2.title.contains(value)) {
return -1;
} else if (!ed1.title.contains(value) && ed2.title.contains(value)) {
return 1;
} else {
return 0;
}
}
/* Getter/Setter */
public boolean isPreviousScenePresent() {
return previousScene != null;
}
@@ -125,4 +269,21 @@ public class ErrorController implements FxController {
public boolean getCopiedDetails() {
return copiedDetails.get();
}
}
public BooleanExpression errorSolutionFoundProperty() {
return errorSolutionFound;
}
public boolean getErrorSolutionFound() {
return errorSolutionFound.get();
}
public BooleanProperty isLoadingHttpResponseProperty() {
return isLoadingHttpResponse;
}
public boolean getIsLoadingHttpResponse() {
return isLoadingHttpResponse.get();
}
}

View File

@@ -0,0 +1,13 @@
package org.cryptomator.ui.error;
public class ErrorDiscussion {
int upvoteCount;
String title;
String url;
Answer answer;
static class Answer {
}
}

View File

@@ -1,6 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<?import org.cryptomator.ui.controls.FontAwesome5IconView?>
<?import org.cryptomator.ui.controls.FontAwesome5Spinner?>
<?import org.cryptomator.ui.controls.FormattedLabel?>
<?import javafx.geometry.Insets?>
<?import javafx.scene.control.Button?>
@@ -31,20 +32,28 @@
</StackPane>
<VBox spacing="6" HBox.hgrow="ALWAYS">
<FormattedLabel styleClass="label-extra-large" format="%error.message" arg1="${controller.errorCode}"/>
<Label text="%error.description" wrapText="true"/>
<Hyperlink styleClass="hyperlink-underline" text="%error.hyperlink.lookup" onAction="#searchError" contentDisplay="LEFT">
<graphic>
<FontAwesome5IconView glyph="LINK" glyphSize="12"/>
</graphic>
</Hyperlink>
<Hyperlink styleClass="hyperlink-underline" text="%error.hyperlink.report" onAction="#reportError" contentDisplay="LEFT">
<graphic>
<FontAwesome5IconView glyph="LINK" glyphSize="12"/>
</graphic>
</Hyperlink>
<FontAwesome5Spinner glyphSize="24" visible="${controller.isLoadingHttpResponse}" managed="${controller.isLoadingHttpResponse}"/>
<VBox visible="${!controller.isLoadingHttpResponse}" managed="${!controller.isLoadingHttpResponse}">
<Label text="%error.existingSolutionDescription" wrapText="true" visible="${controller.errorSolutionFound}" managed="${controller.errorSolutionFound}"/>
<Hyperlink styleClass="hyperlink-underline" text="%error.hyperlink.solution" onAction="#showSolution" contentDisplay="LEFT" visible="${controller.errorSolutionFound}" managed="${controller.errorSolutionFound}">
<graphic>
<FontAwesome5IconView glyph="LINK" glyphSize="12"/>
</graphic>
</Hyperlink>
<Label text="%error.description" wrapText="true" visible="${!controller.errorSolutionFound}" managed="${!controller.errorSolutionFound}"/>
<Hyperlink styleClass="hyperlink-underline" text="%error.hyperlink.lookup" onAction="#searchError" contentDisplay="LEFT" visible="${!controller.errorSolutionFound}" managed="${!controller.errorSolutionFound}">
<graphic>
<FontAwesome5IconView glyph="LINK" glyphSize="12"/>
</graphic>
</Hyperlink>
<Hyperlink styleClass="hyperlink-underline" text="%error.hyperlink.report" onAction="#reportError" contentDisplay="LEFT" visible="${!controller.errorSolutionFound}" managed="${!controller.errorSolutionFound}">
<graphic>
<FontAwesome5IconView glyph="LINK" glyphSize="12"/>
</graphic>
</Hyperlink>
</VBox>
</VBox>
</HBox>
<VBox spacing="6" VBox.vgrow="ALWAYS">
<HBox>
<Label text="%error.technicalDetails"/>

View File

@@ -21,6 +21,9 @@ error.description=Cryptomator didn't expect this to happen. You can look up exis
error.hyperlink.lookup=Look up this error
error.hyperlink.report=Report this error
error.technicalDetails=Details:
error.existingSolutionDescription=Cryptomator didn't expect this to happen. But we found an existing solution for this error. Please take a look at the following link.
error.hyperlink.solution=Look up the solution
# Defaults
defaults.vault.vaultName=Vault

View File

@@ -0,0 +1,145 @@
package org.cryptomator.ui.error;
import org.cryptomator.common.Environment;
import org.cryptomator.common.ErrorCode;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.mockito.Mockito;
import javafx.application.Application;
import javafx.scene.Scene;
import javafx.stage.Stage;
import java.util.concurrent.ExecutorService;
class ErrorControllerTest {
Application application;
String stackTrace;
ErrorCode errorCode;
Scene previousScene;
Stage window;
Environment environment;
ExecutorService executorService;
ErrorController errorController;
@BeforeEach
public void beforeEach() {
application = Mockito.mock(Application.class);
stackTrace = "This is a stackTrace mock";
errorCode = Mockito.mock(ErrorCode.class);
previousScene = Mockito.mock(Scene.class);
window = Mockito.mock(Stage.class);
environment = Mockito.mock(Environment.class);
executorService = Mockito.mock(ExecutorService.class);
errorController = new ErrorController(application, stackTrace, errorCode, previousScene, window, environment, executorService);
}
private ErrorDiscussion createErrorDiscussion(String title, int upvoteCount, ErrorDiscussion.Answer answer) {
ErrorDiscussion ed = new ErrorDiscussion();
ed.title = title;
ed.upvoteCount = upvoteCount;
ed.answer = answer;
return ed;
}
@DisplayName("compare error discussions by upvote count")
@ParameterizedTest
@CsvSource(textBlock = """
10, <, 5
8, >, 15
10, =, 10
""")
public void testCompareUpvoteCount(int leftUpvoteCount, char expected, int rightUpvoteCount) {
int expectedResult = switch (expected) {
case '<' -> -1;
case '>' -> +1;
default -> 0;
};
var left = createErrorDiscussion("", leftUpvoteCount, null);
var right = createErrorDiscussion("", rightUpvoteCount, null);
int result = errorController.compareUpvoteCount(left, right);
Assertions.assertEquals(expectedResult, Integer.signum(result));
}
@DisplayName("compare error discussions by existence of an answer")
@ParameterizedTest
@CsvSource(textBlock = """
false, =, false
true, =, true
true, <, false
false, >, true
""")
public void testCompareIsAnswered(boolean leftIsAnswered, char expected, boolean rightIsAnswered) {
var answer = new ErrorDiscussion.Answer();
int expectedResult = switch (expected) {
case '<' -> -1;
case '>' -> +1;
default -> 0;
};
var left = createErrorDiscussion("", 0, leftIsAnswered ? answer : null);
var right = createErrorDiscussion("", 0, rightIsAnswered ? answer : null);
int result = errorController.compareIsAnswered(left, right);
Assertions.assertEquals(expectedResult, result);
}
@DisplayName("compare error codes by full error code")
@ParameterizedTest
@CsvSource(textBlock = """
Error 0000:0000:0000, =, Error 0000:0000:0000
Error 6HU1:12H1:HU7J, <, Error 0000:0000:0000
Error 0000:0000:0000, >, Error 6HU1:12H1:HU7J
""")
public void testCompareByFullErrorCode(String leftTitle, char expected, String rightTitle) {
Mockito.when(errorCode.toString()).thenReturn("6HU1:12H1:HU7J");
int expectedResult = switch (expected) {
case '<' -> -1;
case '>' -> +1;
default -> 0;
};
var left = createErrorDiscussion(leftTitle, 0, null);
var right = createErrorDiscussion(rightTitle, 0, null);
int result = errorController.compareByFullErrorCode(left, right);
Assertions.assertEquals(expectedResult, result);
}
@DisplayName("compare error codes by root cause")
@ParameterizedTest
@CsvSource(textBlock = """
Error 6HU1:12H1:0000, =, Error 6HU1:12H1:0000
Error 6HU1:12H1:0007, =, Error 6HU1:12H1:0042
Error 0000:0000:0000, =, Error 0000:0000:0000
Error 6HU1:12H1:0000, <, Error 0000:0000:0000
Error 6HU1:12H1:0000, <, Error 6HU1:0000:0000
Error 0000:0000:0000, >, Error 6HU1:12H1:0000
Error 6HU1:0000:0000, >, Error 6HU1:12H1:0000
""")
public void testCompareByRootCauseCode(String leftTitle, char expected, String rightTitle) {
Mockito.when(errorCode.methodCode()).thenReturn("6HU1");
Mockito.when(errorCode.rootCauseCode()).thenReturn("12H1");
int expectedResult = switch (expected) {
case '<' -> -1;
case '>' -> +1;
default -> 0;
};
var left = createErrorDiscussion(leftTitle, 0, null);
var right = createErrorDiscussion(rightTitle, 0, null);
int result = errorController.compareByRootCauseCode(left, right);
Assertions.assertEquals(expectedResult, result);
}
@DisplayName("check if the error code contains the method code")
@ParameterizedTest
@CsvSource(textBlock = """
Error 6HU1:0000:0000, true
Error 0000:0000:0000, false
""")
public void testContainsMethodCode(String title, boolean expectedResult) {
Mockito.when(errorCode.methodCode()).thenReturn("6HU1");
var ed = createErrorDiscussion(title, 0, null);
boolean result = errorController.containsMethodCode(ed);
Assertions.assertEquals(expectedResult, result);
}
}