From 06b823f4c1ee2da9ebe70a2c3fe07039a7e709c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Mon, 4 Jul 2022 14:59:01 +0200 Subject: [PATCH] connect: catch panics during init --- nym-connect/src-tauri/Cargo.toml | 2 +- nym-connect/src-tauri/src/config/mod.rs | 24 ++++++++++---- nym-connect/src-tauri/src/error.rs | 2 ++ nym-connect/src-tauri/src/main.rs | 1 + .../src/operations/connection/connect.rs | 2 +- nym-connect/src-tauri/src/state.rs | 32 +++++++++++++------ 6 files changed, 46 insertions(+), 17 deletions(-) diff --git a/nym-connect/src-tauri/Cargo.toml b/nym-connect/src-tauri/Cargo.toml index e45da2ce06..56edeae17a 100644 --- a/nym-connect/src-tauri/Cargo.toml +++ b/nym-connect/src-tauri/Cargo.toml @@ -30,7 +30,7 @@ serde_json = "1.0" tauri = { version = "=1.0.0-rc.8", features = ["ayatana-tray", "shell-open", "system-tray"] } tendermint-rpc = "0.23.0" thiserror = "1.0" -tokio = { version = "1.19.1", features = ["sync"] } +tokio = { version = "1.19.1", features = ["sync", "time"] } url = "2.2" log = "0.4" pretty_env_logger = "0.4.0" diff --git a/nym-connect/src-tauri/src/config/mod.rs b/nym-connect/src-tauri/src/config/mod.rs index 008e8a2086..cda081175f 100644 --- a/nym-connect/src-tauri/src/config/mod.rs +++ b/nym-connect/src-tauri/src/config/mod.rs @@ -76,10 +76,22 @@ impl Config { self.socks5.get_base_mut() } - pub async fn init(service_provider: &str, chosen_gateway_id: &str) { + pub async fn init(service_provider: &str, chosen_gateway_id: &str) -> Result<(), BackendError> { info!("Initialising..."); - init_socks5(service_provider, chosen_gateway_id).await; + + let service_provider = service_provider.to_owned(); + let chosen_gateway_id = chosen_gateway_id.to_owned(); + + // The client initialization was originally not written for this use case, so there are + // lots of ways it can panic. Until we have proper error handling in the init code for the + // clients we'll catch any panics here. + std::panic::catch_unwind(move || { + futures::executor::block_on(init_socks5(service_provider, chosen_gateway_id)); + }) + .map_err(|_| BackendError::InitializationPanic)?; + info!("Configuration saved 🚀"); + Ok(()) } pub fn config_file_location(id: &str) -> PathBuf { @@ -87,11 +99,11 @@ impl Config { } } -pub async fn init_socks5(provider_address: &str, chosen_gateway_id: &str) { +pub async fn init_socks5(provider_address: String, chosen_gateway_id: String) { log::info!("Initialising client..."); // Append the gateway id to the name id that we store the config under - let id = append_config_id(chosen_gateway_id); + let id = append_config_id(&chosen_gateway_id); log::debug!( "Attempting to use config file location: {}", @@ -112,7 +124,7 @@ pub async fn init_socks5(provider_address: &str, chosen_gateway_id: &str) { let register_gateway = !already_init || user_wants_force_register; log::trace!("Creating config for id: {}", id); - let mut config = Config::new(id.as_str(), provider_address); + let mut config = Config::new(id.as_str(), &provider_address); // As far as I'm aware, these two are not used, they are only set because the socks5 init code // requires them for initialising the bandwidth controller. @@ -126,7 +138,7 @@ pub async fn init_socks5(provider_address: &str, chosen_gateway_id: &str) { let gateway = setup_gateway( &id, register_gateway, - Some(chosen_gateway_id), + Some(&chosen_gateway_id), config.get_socks5(), ) .await; diff --git a/nym-connect/src-tauri/src/error.rs b/nym-connect/src-tauri/src/error.rs index e14bd47264..9a36ea4837 100644 --- a/nym-connect/src-tauri/src/error.rs +++ b/nym-connect/src-tauri/src/error.rs @@ -19,6 +19,8 @@ pub enum BackendError { #[from] source: reqwest::Error, }, + #[error("Initialization failed with a panic")] + InitializationPanic, } impl Serialize for BackendError { diff --git a/nym-connect/src-tauri/src/main.rs b/nym-connect/src-tauri/src/main.rs index db1e06fe55..e13b324dd4 100644 --- a/nym-connect/src-tauri/src/main.rs +++ b/nym-connect/src-tauri/src/main.rs @@ -68,5 +68,6 @@ fn setup_logging() { .filter_module("sled", log::LevelFilter::Warn) .filter_module("tokio_tungstenite", log::LevelFilter::Warn) .filter_module("tungstenite", log::LevelFilter::Warn) + .filter_module("want", log::LevelFilter::Warn) .init(); } diff --git a/nym-connect/src-tauri/src/operations/connection/connect.rs b/nym-connect/src-tauri/src/operations/connection/connect.rs index ca52f01424..2677fadc3a 100644 --- a/nym-connect/src-tauri/src/operations/connection/connect.rs +++ b/nym-connect/src-tauri/src/operations/connection/connect.rs @@ -13,7 +13,7 @@ pub async fn start_connecting( log::trace!("Start connecting with:"); log::trace!(" service_provider: {:?}", guard.get_service_provider()); log::trace!(" gateway: {:?}", guard.get_gateway()); - guard.start_connecting(&window).await + guard.start_connecting(&window).await? }; // Setup task for checking status diff --git a/nym-connect/src-tauri/src/state.rs b/nym-connect/src-tauri/src/state.rs index 6794036e99..091c467b55 100644 --- a/nym-connect/src-tauri/src/state.rs +++ b/nym-connect/src-tauri/src/state.rs @@ -1,11 +1,13 @@ +use std::time::Duration; + use futures::SinkExt; -use log::info; use tauri::Manager; use nym_socks5::client::{Socks5ControlMessage, Socks5ControlMessageSender}; use crate::{ config::append_config_id, + error::BackendError, models::{ AppEventConnectionStatusChangedPayload, ConnectionStatusKind, APP_EVENT_CONNECTION_STATUS_CHANGED, @@ -61,7 +63,7 @@ impl State { self.gateway = Some(gateway); } - pub async fn init_config(&self) { + pub async fn init_config(&self) -> Result<(), BackendError> { let service_provider = self .service_provider .as_ref() @@ -70,16 +72,28 @@ impl State { .gateway .as_ref() .expect("Attempting to init without gateway"); - crate::config::Config::init(service_provider, gateway).await; + crate::config::Config::init(service_provider, gateway).await } - pub async fn start_connecting(&mut self, window: &tauri::Window) -> StatusReceiver { - info!("Connecting"); + pub async fn start_connecting( + &mut self, + window: &tauri::Window, + ) -> Result { + log::info!("Connecting"); self.set_state(ConnectionStatusKind::Connecting, window); self.status = ConnectionStatusKind::Connecting; // Setup configuration by writing to file - self.init_config().await; + if let Err(err) = self.init_config().await { + log::warn!("Failed to initialize: {}", err); + + // Wait a little to give the user some rudimentary feedback that the click actually + // registered. + tokio::time::sleep(Duration::from_secs(1)).await; + self.set_state(ConnectionStatusKind::Disconnected, window); + self.status = ConnectionStatusKind::Disconnected; + return Err(err); + } // Kick off the main task and get the channel for controlling it let id = append_config_id( @@ -94,11 +108,11 @@ impl State { self.status = ConnectionStatusKind::Connected; self.set_state(ConnectionStatusKind::Connected, window); - status_receiver + Ok(status_receiver) } pub async fn start_disconnecting(&mut self, window: &tauri::Window) { - info!("Disconnecting"); + log::info!("Disconnecting"); self.set_state(ConnectionStatusKind::Disconnecting, window); self.status = ConnectionStatusKind::Disconnecting; @@ -109,7 +123,7 @@ impl State { } pub async fn mark_disconnected(&mut self, window: &tauri::Window) { - info!("Disconnected"); + log::info!("Disconnected"); self.status = ConnectionStatusKind::Disconnected; self.set_state(ConnectionStatusKind::Disconnected, window); }