From 243c74b0cbeff712121a9e811ec435cae051c79c Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 21 Apr 2020 12:38:36 +0200 Subject: [PATCH] Show stack trace in UI for vaults in error state --- .../org/cryptomator/common/vaults/Vault.java | 27 +++++++++++- .../common/vaults/VaultComponent.java | 6 +++ .../common/vaults/VaultListManager.java | 42 ++++++++++--------- .../common/vaults/VaultModule.java | 10 +++++ .../cryptomator/ui/common/ErrorModule.java | 3 +- .../ui/mainwindow/MainWindowModule.java | 14 +++---- .../VaultDetailUnknownErrorController.java | 40 ++++++++++++++++++ .../ui/mainwindow/VaultListController.java | 11 ++++- .../src/main/resources/fxml/vault_detail.fxml | 1 + .../fxml/vault_detail_unknownerror.fxml | 15 +++++++ 10 files changed, 138 insertions(+), 31 deletions(-) create mode 100644 main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailUnknownErrorController.java create mode 100644 main/ui/src/main/resources/fxml/vault_detail_unknownerror.fxml diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java index 6e35ffab2..e9e0fbd6d 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -27,6 +27,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.inject.Inject; +import javax.inject.Named; import javax.inject.Provider; import java.io.IOException; import java.nio.file.NoSuchFileException; @@ -51,6 +52,7 @@ public class Vault { private final StringBinding defaultMountFlags; private final AtomicReference cryptoFileSystem; private final ObjectProperty state; + private final ObjectProperty lastKnownException; private final VaultStats stats; private final StringBinding displayableName; private final StringBinding displayablePath; @@ -59,18 +61,20 @@ public class Vault { private final BooleanBinding unlocked; private final BooleanBinding missing; private final BooleanBinding needsMigration; + private final BooleanBinding unknownError; private final StringBinding accessPoint; private final BooleanBinding accessPointPresent; private volatile Volume volume; @Inject - Vault(VaultSettings vaultSettings, Provider volumeProvider, @DefaultMountFlags StringBinding defaultMountFlags, AtomicReference cryptoFileSystem, ObjectProperty state, VaultStats stats) { + Vault(VaultSettings vaultSettings, Provider volumeProvider, @DefaultMountFlags StringBinding defaultMountFlags, AtomicReference cryptoFileSystem, ObjectProperty state, @Named("lastKnownException") ObjectProperty lastKnownException, VaultStats stats) { this.vaultSettings = vaultSettings; this.volumeProvider = volumeProvider; this.defaultMountFlags = defaultMountFlags; this.cryptoFileSystem = cryptoFileSystem; this.state = state; + this.lastKnownException = lastKnownException; this.stats = stats; this.displayableName = Bindings.createStringBinding(this::getDisplayableName, vaultSettings.path()); this.displayablePath = Bindings.createStringBinding(this::getDisplayablePath, vaultSettings.path()); @@ -79,6 +83,7 @@ public class Vault { this.unlocked = Bindings.createBooleanBinding(this::isUnlocked, state); this.missing = Bindings.createBooleanBinding(this::isMissing, state); this.needsMigration = Bindings.createBooleanBinding(this::isNeedsMigration, state); + this.unknownError = Bindings.createBooleanBinding(this::isUnknownError, state); this.accessPoint = Bindings.createStringBinding(this::getAccessPoint, state); this.accessPointPresent = this.accessPoint.isNotEmpty(); } @@ -149,6 +154,18 @@ public class Vault { state.setValue(value); } + public ObjectProperty lastKnownExceptionProperty() { + return lastKnownException; + } + + public Exception getLastKnownException() { + return lastKnownException.get(); + } + + public void setLastKnownException(Exception e) { + lastKnownException.setValue(e); + } + public BooleanBinding lockedProperty() { return locked; } @@ -189,6 +206,14 @@ public class Vault { return state.get() == VaultState.NEEDS_MIGRATION; } + public BooleanBinding unknownErrorProperty() { + return unknownError; + } + + public boolean isUnknownError() { + return state.get() == VaultState.ERROR; + } + public StringBinding displayableNameProperty() { return displayableName; } diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultComponent.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultComponent.java index 58abd2d37..98a9f951d 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultComponent.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultComponent.java @@ -10,6 +10,9 @@ import org.cryptomator.common.settings.VaultSettings; import dagger.Subcomponent; +import javax.annotation.Nullable; +import javax.inject.Named; + @PerVault @Subcomponent(modules = {VaultModule.class}) public interface VaultComponent { @@ -25,6 +28,9 @@ public interface VaultComponent { @BindsInstance Builder initialVaultState(VaultState vaultState); + @BindsInstance + Builder initialErrorCause(@Nullable @Named("lastKnownException") Exception initialErrorCause); + VaultComponent build(); } diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java index 51a933ad4..09bd60c77 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java @@ -31,9 +31,9 @@ import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; @Singleton public class VaultListManager { - + private static final Logger LOG = LoggerFactory.getLogger(VaultListManager.class); - + private final VaultComponent.Builder vaultComponentBuilder; private final ObservableList vaultList; @@ -41,7 +41,7 @@ public class VaultListManager { public VaultListManager(VaultComponent.Builder vaultComponentBuilder, Settings settings) { this.vaultComponentBuilder = vaultComponentBuilder; this.vaultList = FXCollections.observableArrayList(Vault::observables); - + addAll(settings.getDirectories()); vaultList.addListener(new VaultListChangeListener(settings.getDirectories())); } @@ -65,12 +65,12 @@ public class VaultListManager { return newVault; } } - + private void addAll(Collection vaultSettings) { Collection vaults = vaultSettings.stream().map(this::create).collect(Collectors.toList()); vaultList.addAll(vaults); } - + private Optional get(Path vaultPath) { return vaultList.stream().filter(v -> { try { @@ -82,23 +82,25 @@ public class VaultListManager { } private Vault create(VaultSettings vaultSettings) { - VaultState vaultState = determineVaultState(vaultSettings.path().get()); - VaultComponent comp = vaultComponentBuilder.vaultSettings(vaultSettings).initialVaultState(vaultState).build(); - return comp.vault(); + VaultComponent.Builder compBuilder = vaultComponentBuilder.vaultSettings(vaultSettings); + try { + VaultState vaultState = determineVaultState(vaultSettings.path().get()); + compBuilder.initialVaultState(vaultState); + } catch (IOException e) { + LOG.warn("Failed to determine vault state for " + vaultSettings.path().get(), e); + compBuilder.initialVaultState(VaultState.ERROR); + compBuilder.initialErrorCause(e); + } + return compBuilder.build().vault(); } - public static VaultState determineVaultState(Path pathToVault) { - try { - if (!CryptoFileSystemProvider.containsVault(pathToVault, MASTERKEY_FILENAME)) { - return VaultState.MISSING; - } else if (Migrators.get().needsMigration(pathToVault, MASTERKEY_FILENAME)) { - return VaultState.NEEDS_MIGRATION; - } else { - return VaultState.LOCKED; - } - } catch (IOException e) { - LOG.warn("Could not determine vault state of " + pathToVault + " due to unexpected exception.", e); - return VaultState.ERROR; + public static VaultState determineVaultState(Path pathToVault) throws IOException { + if (!CryptoFileSystemProvider.containsVault(pathToVault, MASTERKEY_FILENAME)) { + return VaultState.MISSING; + } else if (Migrators.get().needsMigration(pathToVault, MASTERKEY_FILENAME)) { + return VaultState.NEEDS_MIGRATION; + } else { + return VaultState.LOCKED; } } diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java index f9c6b9177..1afd9d88b 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java @@ -23,6 +23,8 @@ import org.cryptomator.cryptofs.CryptoFileSystem; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.annotation.Nullable; +import javax.inject.Named; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -46,6 +48,14 @@ public class VaultModule { return new SimpleObjectProperty<>(initialState); } + @Provides + @Named("lastKnownException") + @PerVault + public ObjectProperty provideLastKnownException(@Named("lastKnownException") @Nullable Exception initialErrorCause) { + return new SimpleObjectProperty<>(initialErrorCause); + } + + @Provides public Volume provideVolume(Settings settings, WebDavVolume webDavVolume, FuseVolume fuseVolume, DokanyVolume dokanyVolume) { VolumeImpl preferredImpl = settings.preferredVolumeImpl().get(); diff --git a/main/ui/src/main/java/org/cryptomator/ui/common/ErrorModule.java b/main/ui/src/main/java/org/cryptomator/ui/common/ErrorModule.java index 9b272a585..27a51c00b 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/common/ErrorModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/common/ErrorModule.java @@ -21,10 +21,11 @@ abstract class ErrorModule { static FXMLLoaderFactory provideFxmlLoaderFactory(Map, Provider> factories, DefaultSceneFactory sceneFactory, ResourceBundle resourceBundle) { return new FXMLLoaderFactory(factories, sceneFactory, resourceBundle); } - + @Provides @Named("stackTrace") static String provideStackTrace(Throwable cause) { + // TODO deduplicate VaultDetailUnknownErrorController.java ByteArrayOutputStream baos = new ByteArrayOutputStream(); cause.printStackTrace(new PrintStream(baos)); return baos.toString(StandardCharsets.UTF_8); diff --git a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowModule.java b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowModule.java index 5043fa485..04a047424 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowModule.java @@ -6,13 +6,9 @@ import dagger.Provides; import dagger.multibindings.IntoMap; import javafx.scene.Scene; import javafx.scene.image.Image; -import javafx.scene.input.KeyCode; -import javafx.scene.input.KeyCodeCombination; -import javafx.scene.input.KeyCombination; import javafx.stage.Stage; import javafx.stage.StageStyle; import org.cryptomator.ui.addvaultwizard.AddVaultWizardComponent; -import org.cryptomator.ui.common.DefaultSceneFactory; import org.cryptomator.ui.common.FXMLLoaderFactory; import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxControllerKey; @@ -87,7 +83,7 @@ abstract class MainWindowModule { @IntoMap @FxControllerKey(VaultDetailController.class) abstract FxController bindVaultDetailController(VaultDetailController controller); - + @Binds @IntoMap @FxControllerKey(WelcomeController.class) @@ -102,7 +98,7 @@ abstract class MainWindowModule { @IntoMap @FxControllerKey(VaultDetailUnlockedController.class) abstract FxController bindVaultDetailUnlockedController(VaultDetailUnlockedController controller); - + @Binds @IntoMap @FxControllerKey(VaultDetailMissingVaultController.class) @@ -113,11 +109,15 @@ abstract class MainWindowModule { @FxControllerKey(VaultDetailNeedsMigrationController.class) abstract FxController bindVaultDetailNeedsMigrationController(VaultDetailNeedsMigrationController controller); + @Binds + @IntoMap + @FxControllerKey(VaultDetailUnknownErrorController.class) + abstract FxController bindVaultDetailUnknownErrorController(VaultDetailUnknownErrorController controller); + @Binds @IntoMap @FxControllerKey(VaultListCellController.class) abstract FxController bindVaultListCellController(VaultListCellController controller); - } diff --git a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailUnknownErrorController.java b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailUnknownErrorController.java new file mode 100644 index 000000000..4c23ba4ae --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailUnknownErrorController.java @@ -0,0 +1,40 @@ +package org.cryptomator.ui.mainwindow; + +import javafx.beans.binding.Binding; +import javafx.beans.property.ObjectProperty; +import org.cryptomator.common.vaults.Vault; +import org.cryptomator.ui.common.FxController; +import org.fxmisc.easybind.EasyBind; + +import javax.inject.Inject; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.nio.charset.StandardCharsets; + +@MainWindowScoped +public class VaultDetailUnknownErrorController implements FxController { + + private final Binding stackTrace; + + @Inject + public VaultDetailUnknownErrorController(ObjectProperty vault) { + this.stackTrace = EasyBind.select(vault).selectObject(Vault::lastKnownExceptionProperty).map(this::provideStackTrace).orElse(""); + } + + private String provideStackTrace(Throwable cause) { + // TODO deduplicate ErrorModule.java + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + cause.printStackTrace(new PrintStream(baos)); + return baos.toString(StandardCharsets.UTF_8); + } + + /* Getter/Setter */ + + public Binding stackTraceProperty() { + return stackTrace; + } + + public String getStackTrace() { + return stackTrace.getValue(); + } +} diff --git a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListController.java b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListController.java index 167138e0f..625a10e65 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListController.java @@ -18,6 +18,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.inject.Inject; +import java.io.IOException; @MainWindowScoped public class VaultListController implements FxController { @@ -68,8 +69,14 @@ public class VaultListController implements FxController { case LOCKED: case NEEDS_MIGRATION: case MISSING: - VaultState determinedState = VaultListManager.determineVaultState(newValue.getPath()); - newValue.setState(determinedState); + try { + VaultState determinedState = VaultListManager.determineVaultState(newValue.getPath()); + newValue.setState(determinedState); + } catch (IOException e) { + LOG.warn("Failed to determine vault state for " + newValue.getPath(), e); + newValue.setState(VaultState.ERROR); + newValue.setLastKnownException(e); + } break; case ERROR: case UNLOCKED: diff --git a/main/ui/src/main/resources/fxml/vault_detail.fxml b/main/ui/src/main/resources/fxml/vault_detail.fxml index 349db1f4c..e171e0887 100644 --- a/main/ui/src/main/resources/fxml/vault_detail.fxml +++ b/main/ui/src/main/resources/fxml/vault_detail.fxml @@ -52,5 +52,6 @@ + diff --git a/main/ui/src/main/resources/fxml/vault_detail_unknownerror.fxml b/main/ui/src/main/resources/fxml/vault_detail_unknownerror.fxml new file mode 100644 index 000000000..4c33a67f0 --- /dev/null +++ b/main/ui/src/main/resources/fxml/vault_detail_unknownerror.fxml @@ -0,0 +1,15 @@ + + + + + +