Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve handling when unable to connect to tor #2399

Merged
merged 1 commit into from Feb 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions core/src/main/java/bisq/core/app/BisqSetup.java
Expand Up @@ -355,6 +355,10 @@ public StringProperty getBtcSplashSyncIconId() {
return walletAppSetup.getBtcSplashSyncIconId();
}

public BooleanProperty getUseTorForBTC() {
return walletAppSetup.getUseTorForBTC();
}

// P2P
public StringProperty getP2PNetworkInfo() {
return p2PNetworkSetup.getP2PNetworkInfo();
Expand Down Expand Up @@ -484,6 +488,11 @@ private void startP2pNetworkAndWallet() {
};

Timer startupTimeout = UserThread.runAfter(() -> {
if (p2PNetworkSetup.p2pNetworkFailed.get()) {
// Skip this timeout action if the p2p network setup failed
// since a p2p network error prompt will be shown containing the error message
return;
}
log.warn("startupTimeout called");
if (walletsManager.areWalletsEncrypted())
walletInitialized.addListener(walletInitializedListener);
Expand Down
5 changes: 4 additions & 1 deletion core/src/main/java/bisq/core/app/P2PNetworkSetup.java
Expand Up @@ -65,6 +65,8 @@ public class P2PNetworkSetup {
final StringProperty p2pNetworkWarnMsg = new SimpleStringProperty();
@Getter
final BooleanProperty updatedDataReceived = new SimpleBooleanProperty();
@Getter
final BooleanProperty p2pNetworkFailed = new SimpleBooleanProperty();

@Inject
public P2PNetworkSetup(PriceFeedService priceFeedService,
Expand Down Expand Up @@ -194,11 +196,12 @@ public void onUpdatedDataReceived() {

@Override
public void onSetupFailed(Throwable throwable) {
log.warn("onSetupFailed");
log.error("onSetupFailed");
p2pNetworkWarnMsg.set(Res.get("mainView.p2pNetworkWarnMsg.connectionToP2PFailed", throwable.getMessage()));
splashP2PNetworkAnimationVisible.set(false);
bootstrapWarning.set(Res.get("mainView.bootstrapWarning.bootstrappingToP2PFailed"));
p2pNetworkLabelId.set("splash-error-state-msg");
p2pNetworkFailed.set(true);
}

@Override
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/bisq/core/app/WalletAppSetup.java
Expand Up @@ -32,8 +32,10 @@
import org.fxmisc.easybind.EasyBind;
import org.fxmisc.easybind.monadic.MonadicBinding;

import javafx.beans.property.BooleanProperty;
import javafx.beans.property.DoubleProperty;
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleDoubleProperty;
import javafx.beans.property.SimpleObjectProperty;
import javafx.beans.property.SimpleStringProperty;
Expand Down Expand Up @@ -68,6 +70,8 @@ public class WalletAppSetup {
private final StringProperty btcInfo = new SimpleStringProperty(Res.get("mainView.footer.btcInfo.initializing"));
@Getter
private int numBtcPeers = 0;
@Getter
private final BooleanProperty useTorForBTC = new SimpleBooleanProperty();

@Inject
public WalletAppSetup(WalletsManager walletsManager,
Expand All @@ -80,6 +84,7 @@ public WalletAppSetup(WalletsManager walletsManager,
this.bisqEnvironment = bisqEnvironment;
this.preferences = preferences;
this.formatter = formatter;
this.useTorForBTC.set(preferences.getUseTorForBitcoinJ());
}

void init(@Nullable Consumer<String> chainFileLockedExceptionHandler,
Expand Down
27 changes: 19 additions & 8 deletions desktop/src/main/java/bisq/desktop/main/MainView.java
Expand Up @@ -495,6 +495,13 @@ private VBox createSplashScreen() {
splashP2PNetworkLabel.getStyleClass().add("sub-info");
splashP2PNetworkLabel.textProperty().bind(model.getP2PNetworkInfo());

Button showTorNetworkSettingsButton = new AutoTooltipButton(Res.get("settings.net.openTorSettingsButton"));
showTorNetworkSettingsButton.setVisible(false);
showTorNetworkSettingsButton.setManaged(false);
showTorNetworkSettingsButton.setOnAction(e -> {
model.getTorNetworkSettingsWindow().show();
});

splashP2PNetworkBusyAnimation = new BusyAnimation(false);

splashP2PNetworkErrorMsgListener = (ov, oldValue, newValue) -> {
Expand All @@ -504,21 +511,20 @@ private VBox createSplashScreen() {
splashP2PNetworkLabel.getStyleClass().add("error-text");
splashP2PNetworkBusyAnimation.setDisable(true);
splashP2PNetworkBusyAnimation.stop();
showTorNetworkSettingsButton.setVisible(true);
showTorNetworkSettingsButton.setManaged(true);
if (model.getUseTorForBTC().get()) {
// If using tor for BTC, hide the BTC status since tor is not working
btcSyncIndicator.setVisible(false);
btcSplashInfo.setVisible(false);
}
} else if (model.getSplashP2PNetworkAnimationVisible().get()) {
splashP2PNetworkBusyAnimation.setDisable(false);
splashP2PNetworkBusyAnimation.play();
}
};
model.getP2pNetworkWarnMsg().addListener(splashP2PNetworkErrorMsgListener);


Button showTorNetworkSettingsButton = new AutoTooltipButton(Res.get("settings.net.openTorSettingsButton"));
showTorNetworkSettingsButton.setVisible(false);
showTorNetworkSettingsButton.setManaged(false);
showTorNetworkSettingsButton.setOnAction(e -> {
model.getTorNetworkSettingsWindow().show();
});

ImageView splashP2PNetworkIcon = new ImageView();
splashP2PNetworkIcon.setId("image-connection-tor");
splashP2PNetworkIcon.setVisible(false);
Expand All @@ -528,6 +534,11 @@ private VBox createSplashScreen() {
Timer showTorNetworkSettingsTimer = UserThread.runAfter(() -> {
showTorNetworkSettingsButton.setVisible(true);
showTorNetworkSettingsButton.setManaged(true);
if (btcSyncIndicator.progressProperty().getValue() <= 0) {
// If no progress has been made, hide the BTC status since tor is not working
btcSyncIndicator.setVisible(false);
btcSplashInfo.setVisible(false);
}
}, SHOW_TOR_SETTINGS_DELAY_SEC);

splashP2PNetworkIconIdListener = (ov, oldValue, newValue) -> {
Expand Down
4 changes: 4 additions & 0 deletions desktop/src/main/java/bisq/desktop/main/MainViewModel.java
Expand Up @@ -510,6 +510,10 @@ StringProperty getBtcSplashSyncIconId() {
return bisqSetup.getBtcSplashSyncIconId();
}

BooleanProperty getUseTorForBTC() {
return bisqSetup.getUseTorForBTC();
}

// P2P
StringProperty getP2PNetworkInfo() {
return bisqSetup.getP2PNetworkInfo();
Expand Down
Expand Up @@ -22,7 +22,6 @@ public interface SetupListener {

void onHiddenServicePublished();

@SuppressWarnings("unused")
void onSetupFailed(Throwable throwable);

void onRequestCustomBridges();
Expand Down
13 changes: 6 additions & 7 deletions p2p/src/main/java/bisq/network/p2p/network/TorNetworkNode.java
Expand Up @@ -288,13 +288,13 @@ public void run() {
log.error("Tor node creation failed: " + (e.getCause() != null ? e.getCause().toString() : e.toString()));
restartTor(e.getMessage());
} catch (IOException e) {
log.error("Could not connect to running Tor: "
+ e.getMessage());

// Seems a bit harsh, but since we cannot connect to Tor, we cannot do nothing.
log.error("Could not connect to running Tor: " + e.getMessage());
// Since we cannot connect to Tor, we cannot do nothing.
// Furthermore, we have no hidden services started yet, so there is no graceful
// shutdown needed either
System.exit(1);
UserThread.execute(() -> {
setupListeners.stream().forEach(s -> s.onSetupFailed(new RuntimeException(e.getMessage())));
});
} catch (Throwable ignore) {
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@freimair Since I removed restartTor(throwable.getMessage()); from onFailure below since I didn't want it to be called after handling IOException above, perhaps I should add it to this ignored exception?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can force onSuccess by return null; after triggering the onSetupFailed and let the onFailure be?

And yes, this class cries for being refactored :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact we are already doing that. Let me review this again. I should be able to add restartTor back to onFailure.


Expand All @@ -306,8 +306,7 @@ public void onSuccess(Void ignore) {

public void onFailure(@NotNull Throwable throwable) {
UserThread.execute(() -> {
log.error("Hidden service creation failed" + throwable);
restartTor(throwable.getMessage());
log.error("Hidden service creation failed: " + throwable);
});
}
});
Expand Down