From 513baba3fdc49a5028287ca4a9800e79e4130a79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Tue, 5 Jul 2022 11:56:20 +0200 Subject: [PATCH] nym-connect: sort out error handling --- nym-connect/Cargo.lock | 1 + nym-connect/src-tauri/Cargo.toml | 9 +- nym-connect/src-tauri/src/build.rs | 2 +- nym-connect/src-tauri/src/config/mod.rs | 46 ++++---- nym-connect/src-tauri/src/error.rs | 39 ++++++- nym-connect/src-tauri/src/models/mod.rs | 13 +++ .../src/operations/connection/connect.rs | 33 +++--- .../src/operations/connection/disconnect.rs | 9 +- .../src-tauri/src/operations/directory/mod.rs | 4 +- nym-connect/src-tauri/src/state.rs | 105 ++++++++++++------ nym-connect/src-tauri/src/tasks.rs | 33 +++--- 11 files changed, 183 insertions(+), 111 deletions(-) diff --git a/nym-connect/Cargo.lock b/nym-connect/Cargo.lock index 39072331f0..19a6901d2e 100644 --- a/nym-connect/Cargo.lock +++ b/nym-connect/Cargo.lock @@ -3388,6 +3388,7 @@ dependencies = [ "reqwest", "serde", "serde_json", + "tap", "tauri", "tauri-build", "tauri-codegen", diff --git a/nym-connect/src-tauri/Cargo.toml b/nym-connect/src-tauri/Cargo.toml index 56edeae17a..8b4173a120 100644 --- a/nym-connect/src-tauri/Cargo.toml +++ b/nym-connect/src-tauri/Cargo.toml @@ -23,19 +23,20 @@ tauri-macros = "=1.0.0-rc.5" bip39 = "1.0" dirs = "4.0" eyre = "0.6.5" +fix-path-env = { git = "https://github.com/tauri-apps/fix-path-env-rs", branch = "release"} futures = "0.3" +log = "0.4" +pretty_env_logger = "0.4.0" rand = "0.7" +reqwest = { version = "0.11", features = ["json"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" +tap = "1.0.1" 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", "time"] } url = "2.2" -log = "0.4" -pretty_env_logger = "0.4.0" -fix-path-env = { git = "https://github.com/tauri-apps/fix-path-env-rs", branch = "release"} -reqwest = { version = "0.11", features = ["json"] } client-core = { path = "../../clients/client-core" } config = { path = "../../common/config" } diff --git a/nym-connect/src-tauri/src/build.rs b/nym-connect/src-tauri/src/build.rs index d860e1e6a7..261851f6b6 100644 --- a/nym-connect/src-tauri/src/build.rs +++ b/nym-connect/src-tauri/src/build.rs @@ -1,3 +1,3 @@ fn main() { - tauri_build::build() + tauri_build::build(); } diff --git a/nym-connect/src-tauri/src/config/mod.rs b/nym-connect/src-tauri/src/config/mod.rs index 8a88b65037..d1965349e5 100644 --- a/nym-connect/src-tauri/src/config/mod.rs +++ b/nym-connect/src-tauri/src/config/mod.rs @@ -3,13 +3,17 @@ use std::path::PathBuf; use client_core::config::GatewayEndpoint; use log::info; use std::sync::Arc; +use tap::TapFallible; use tokio::sync::RwLock; use client_core::config::Config as BaseConfig; use config::NymConfig; use nym_socks5::client::config::Config as Socks5Config; -use crate::{error::BackendError, state::State}; +use crate::{ + error::{BackendError, Result}, + state::State, +}; pub static SOCKS5_CONFIG_ID: &str = "nym-connect"; @@ -17,30 +21,22 @@ const DEFAULT_ETH_ENDPOINT: &str = "https://rinkeby.infura.io/v3/000000000000000 const DEFAULT_ETH_PRIVATE_KEY: &str = "0000000000000000000000000000000000000000000000000000000000000001"; -pub fn append_config_id(gateway_id: &str) -> String { +pub fn socks5_config_id_appended_with(gateway_id: &str) -> Result { use std::fmt::Write as _; let mut id = SOCKS5_CONFIG_ID.to_string(); - write!(id, "-{}", gateway_id).expect("Failed to set config id"); - id + write!(id, "-{}", gateway_id)?; + Ok(id) } #[tauri::command] -pub async fn get_config_id( - state: tauri::State<'_, Arc>>, -) -> Result { - let guard = state.read().await; - // TODO: return error instead - let gateway_id = guard - .get_gateway() - .as_ref() - .expect("The config id can not be determined before setting the gateway"); - Ok(append_config_id(gateway_id)) +pub async fn get_config_id(state: tauri::State<'_, Arc>>) -> Result { + state.read().await.get_config_id() } #[tauri::command] pub async fn get_config_file_location( state: tauri::State<'_, Arc>>, -) -> Result { +) -> Result { let id = get_config_id(state).await?; Ok(Config::config_file_location(&id) .to_string_lossy() @@ -76,7 +72,7 @@ impl Config { self.socks5.get_base_mut() } - pub async fn init(service_provider: &str, chosen_gateway_id: &str) -> Result<(), BackendError> { + pub async fn init(service_provider: &str, chosen_gateway_id: &str) -> Result<()> { info!("Initialising..."); let service_provider = service_provider.to_owned(); @@ -88,10 +84,12 @@ impl Config { std::thread::spawn(move || { tokio::runtime::Runtime::new() .expect("Failed to create tokio runtime") - .block_on(async move { init_socks5(service_provider, chosen_gateway_id).await }) + .block_on( + async move { init_socks5_config(service_provider, chosen_gateway_id).await }, + ) }) .join() - .map_err(|_| BackendError::InitializationPanic)?; + .map_err(|_| BackendError::InitializationPanic)??; info!("Configuration saved 🚀"); Ok(()) @@ -102,11 +100,11 @@ impl Config { } } -pub async fn init_socks5(provider_address: String, chosen_gateway_id: String) { +pub async fn init_socks5_config(provider_address: String, chosen_gateway_id: String) -> Result<()> { 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 = socks5_config_id_appended_with(&chosen_gateway_id)?; log::debug!( "Attempting to use config file location: {}", @@ -148,10 +146,9 @@ pub async fn init_socks5(provider_address: String, chosen_gateway_id: String) { config.get_base_mut().with_gateway_endpoint(gateway); let config_save_location = config.get_socks5().get_config_file_save_location(); - config - .get_socks5() - .save_to_file(None) - .expect("Failed to save the config file"); + config.get_socks5().save_to_file(None).tap_err(|_| { + log::warn!("Failed to save the config file"); + })?; log::info!("Saved configuration file to {:?}", config_save_location); log::info!("Gateway id: {}", config.get_base().get_gateway_id()); @@ -172,6 +169,7 @@ pub async fn init_socks5(provider_address: String, chosen_gateway_id: String) { info!("Client configuration completed."); client_core::init::show_address(config.get_base()); + Ok(()) } // TODO: deduplicate with same functions in other client diff --git a/nym-connect/src-tauri/src/error.rs b/nym-connect/src-tauri/src/error.rs index 9a36ea4837..ad129a7c5a 100644 --- a/nym-connect/src-tauri/src/error.rs +++ b/nym-connect/src-tauri/src/error.rs @@ -4,30 +4,57 @@ use thiserror::Error; #[allow(unused)] #[derive(Error, Debug)] pub enum BackendError { + #[error("{source}")] + ReqwestError { + #[from] + source: reqwest::Error, + }, + #[error("I/O error: {source}")] + IoError { + #[from] + source: std::io::Error, + }, + #[error("String formatting error: {source}")] + FmtError { + #[from] + source: std::fmt::Error, + }, + #[error("Tauri error: {source}")] + TauriError { + #[from] + source: tauri::Error, + }, + #[error("State error")] StateError, #[error("Could not connect")] CouldNotConnect, #[error("Could not disconnect")] CouldNotDisconnect, + #[error("Could not send disconnect signal to the SOCKS5 client")] + CoundNotSendDisconnectSignal, #[error("No service provider set")] NoServiceProviderSet, #[error("No gateway provider set")] NoGatewaySet, - #[error("{source}")] - ReqwestError { - #[from] - source: reqwest::Error, - }, #[error("Initialization failed with a panic")] InitializationPanic, + #[error("Could not get config id before gateway is set")] + CouldNotGetIdWithoutGateway, + #[error("Could initialize without gateway set")] + CouldNotInitWithoutGateway, + #[error("Could initialize without service provider set")] + CouldNotInitWithoutServiceProvider, } impl Serialize for BackendError { - fn serialize(&self, serializer: S) -> Result + fn serialize(&self, serializer: S) -> std::result::Result where S: Serializer, { serializer.collect_str(self) } } + +// Local crate level Result alias +pub(crate) type Result = std::result::Result; diff --git a/nym-connect/src-tauri/src/models/mod.rs b/nym-connect/src-tauri/src/models/mod.rs index 5ecdbf5c50..ded6a479fc 100644 --- a/nym-connect/src-tauri/src/models/mod.rs +++ b/nym-connect/src-tauri/src/models/mod.rs @@ -1,3 +1,5 @@ +use core::fmt; + use serde::{Deserialize, Serialize}; #[cfg_attr(test, derive(ts_rs::TS))] @@ -23,6 +25,17 @@ pub enum ConnectionStatusKind { Connecting, } +impl fmt::Display for ConnectionStatusKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match *self { + ConnectionStatusKind::Disconnected => write!(f, "Disconnected"), + ConnectionStatusKind::Disconnecting => write!(f, "Disconnecting"), + ConnectionStatusKind::Connected => write!(f, "Connected"), + ConnectionStatusKind::Connecting => write!(f, "Connecting"), + } + } +} + pub const APP_EVENT_CONNECTION_STATUS_CHANGED: &str = "app:connection-status-changed"; #[cfg_attr(test, derive(ts_rs::TS))] diff --git a/nym-connect/src-tauri/src/operations/connection/connect.rs b/nym-connect/src-tauri/src/operations/connection/connect.rs index 2677fadc3a..032221d456 100644 --- a/nym-connect/src-tauri/src/operations/connection/connect.rs +++ b/nym-connect/src-tauri/src/operations/connection/connect.rs @@ -1,4 +1,8 @@ -use crate::{error::BackendError, models::ConnectResult, tasks::start_disconnect_listener, State}; +use crate::{ + error::{BackendError, Result}, + models::ConnectResult, + tasks, State, +}; use std::sync::Arc; use tokio::sync::RwLock; @@ -6,30 +10,23 @@ use tokio::sync::RwLock; pub async fn start_connecting( state: tauri::State<'_, Arc>>, window: tauri::Window, -) -> Result { +) -> Result { let status_receiver = { - let mut guard = state.write().await; - - log::trace!("Start connecting with:"); - log::trace!(" service_provider: {:?}", guard.get_service_provider()); - log::trace!(" gateway: {:?}", guard.get_gateway()); - guard.start_connecting(&window).await? + let mut state_w = state.write().await; + state_w.start_connecting(&window).await? }; // Setup task for checking status let state = state.inner().clone(); - start_disconnect_listener(state, window, status_receiver); + tasks::start_disconnect_listener(state, window, status_receiver); Ok(ConnectResult { - // WIP(JON): fixme - address: "Test".to_string(), + address: "PLACEHOLDER".to_string(), }) } #[tauri::command] -pub async fn get_service_provider( - state: tauri::State<'_, Arc>>, -) -> Result { +pub async fn get_service_provider(state: tauri::State<'_, Arc>>) -> Result { let guard = state.read().await; guard .get_service_provider() @@ -41,7 +38,7 @@ pub async fn get_service_provider( pub async fn set_service_provider( service_provider: String, state: tauri::State<'_, Arc>>, -) -> Result<(), BackendError> { +) -> Result<()> { log::trace!("Setting service_provider: {service_provider}"); let mut guard = state.write().await; guard.set_service_provider(service_provider); @@ -49,9 +46,7 @@ pub async fn set_service_provider( } #[tauri::command] -pub async fn get_gateway( - state: tauri::State<'_, Arc>>, -) -> Result { +pub async fn get_gateway(state: tauri::State<'_, Arc>>) -> Result { let guard = state.read().await; guard .get_gateway() @@ -63,7 +58,7 @@ pub async fn get_gateway( pub async fn set_gateway( gateway: String, state: tauri::State<'_, Arc>>, -) -> Result<(), BackendError> { +) -> Result<()> { log::trace!("Setting gateway: {gateway}"); let mut guard = state.write().await; guard.set_gateway(gateway); diff --git a/nym-connect/src-tauri/src/operations/connection/disconnect.rs b/nym-connect/src-tauri/src/operations/connection/disconnect.rs index 24826b12c4..2361213e40 100644 --- a/nym-connect/src-tauri/src/operations/connection/disconnect.rs +++ b/nym-connect/src-tauri/src/operations/connection/disconnect.rs @@ -1,4 +1,4 @@ -use crate::error::BackendError; +use crate::error::Result; use crate::models::ConnectResult; use crate::State; use std::sync::Arc; @@ -8,13 +8,12 @@ use tokio::sync::RwLock; pub async fn start_disconnecting( state: tauri::State<'_, Arc>>, window: tauri::Window, -) -> Result { +) -> Result { let mut guard = state.write().await; - guard.start_disconnecting(&window).await; + guard.start_disconnecting(&window).await?; Ok(ConnectResult { - // WIP(JON): fixme - address: "Test".to_string(), + address: "PLACEHOLDER".to_string(), }) } diff --git a/nym-connect/src-tauri/src/operations/directory/mod.rs b/nym-connect/src-tauri/src/operations/directory/mod.rs index 1088c37f8b..aaac2276d9 100644 --- a/nym-connect/src-tauri/src/operations/directory/mod.rs +++ b/nym-connect/src-tauri/src/operations/directory/mod.rs @@ -1,11 +1,11 @@ -use crate::error::BackendError; +use crate::error::Result; use crate::models::DirectoryService; static SERVICE_PROVIDER_WELLKNOWN_URL: &str = "https://nymtech.net/.wellknown/connect/service-providers.json"; #[tauri::command] -pub async fn get_services() -> Result, BackendError> { +pub async fn get_services() -> Result> { let res = reqwest::get(SERVICE_PROVIDER_WELLKNOWN_URL) .await? .json::>() diff --git a/nym-connect/src-tauri/src/state.rs b/nym-connect/src-tauri/src/state.rs index 091c467b55..36913f5192 100644 --- a/nym-connect/src-tauri/src/state.rs +++ b/nym-connect/src-tauri/src/state.rs @@ -1,24 +1,32 @@ use std::time::Duration; use futures::SinkExt; +use tap::TapFallible; use tauri::Manager; use nym_socks5::client::{Socks5ControlMessage, Socks5ControlMessageSender}; use crate::{ - config::append_config_id, - error::BackendError, + config::{self, socks5_config_id_appended_with}, + error::{BackendError, Result}, models::{ AppEventConnectionStatusChangedPayload, ConnectionStatusKind, APP_EVENT_CONNECTION_STATUS_CHANGED, }, - tasks::{start_nym_socks5_client, StatusReceiver}, + tasks::{self, StatusReceiver}, }; pub struct State { + /// The current connection status status: ConnectionStatusKind, + + /// The service provider service_provider: Option, + + /// The gateway used. Note that this is also used to create the configuration id gateway: Option, + + /// Channel that is used to send command messages to the SOCKS5 client, such as to disconnect socks5_client_sender: Option, } @@ -38,13 +46,15 @@ impl State { } fn set_state(&mut self, status: ConnectionStatusKind, window: &tauri::Window) { + log::info!("{status}"); self.status = status.clone(); window .emit_all( APP_EVENT_CONNECTION_STATUS_CHANGED, AppEventConnectionStatusChangedPayload { status }, ) - .unwrap(); + .tap_err(|err| log::warn!("{err}")) + .ok(); } pub fn get_service_provider(&self) -> &Option { @@ -63,25 +73,21 @@ impl State { self.gateway = Some(gateway); } - pub async fn init_config(&self) -> Result<(), BackendError> { - let service_provider = self - .service_provider + /// The effective config id is the static config id appended with the id of the gateway + pub fn get_config_id(&self) -> Result { + self.get_gateway() .as_ref() - .expect("Attempting to init without service provider"); - let gateway = self - .gateway - .as_ref() - .expect("Attempting to init without gateway"); - crate::config::Config::init(service_provider, gateway).await + .ok_or(BackendError::CouldNotGetIdWithoutGateway) + .and_then(|gateway_id| socks5_config_id_appended_with(gateway_id)) } + /// Start connecting by first creating a config file, followed by starting a thread running the + /// SOCKS5 client. pub async fn start_connecting( &mut self, window: &tauri::Window, - ) -> Result { - log::info!("Connecting"); + ) -> Result { self.set_state(ConnectionStatusKind::Connecting, window); - self.status = ConnectionStatusKind::Connecting; // Setup configuration by writing to file if let Err(err) = self.init_config().await { @@ -91,40 +97,67 @@ impl State { // 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( - self.gateway - .as_ref() - .expect("Attempting to start without gateway"), - ); - let (sender, used_gateway, status_receiver) = start_nym_socks5_client(&id); - self.gateway = Some(used_gateway.gateway_id); - self.socks5_client_sender = Some(sender); - - self.status = ConnectionStatusKind::Connected; + let status_receiver = self.start_nym_socks5_client().await?; self.set_state(ConnectionStatusKind::Connected, window); - Ok(status_receiver) } - pub async fn start_disconnecting(&mut self, window: &tauri::Window) { - log::info!("Disconnecting"); + /// Create a configuration file + async fn init_config(&self) -> Result<()> { + let service_provider = self + .get_service_provider() + .as_ref() + .ok_or(BackendError::CouldNotInitWithoutServiceProvider)?; + let gateway = self + .get_gateway() + .as_ref() + .ok_or(BackendError::CouldNotInitWithoutGateway)?; + log::trace!(" service_provider: {:?}", service_provider); + log::trace!(" gateway: {:?}", gateway); + + config::Config::init(service_provider, gateway).await + } + + /// Spawn a new thread running the SOCKS5 client + async fn start_nym_socks5_client(&mut self) -> Result { + let id = self.get_config_id()?; + let (control_tx, status_rx, used_gateway) = tasks::start_nym_socks5_client(&id)?; + self.socks5_client_sender = Some(control_tx); + self.gateway = Some(used_gateway.gateway_id); + Ok(status_rx) + } + + /// Disconnect by sending a message to the SOCKS5 client thread. Once it has finished and is + /// disconnected, the disconnect handler will mark it as disconnected. + pub async fn start_disconnecting(&mut self, window: &tauri::Window) -> Result<()> { self.set_state(ConnectionStatusKind::Disconnecting, window); - self.status = ConnectionStatusKind::Disconnecting; // Send shutdown message - if let Some(ref mut sender) = self.socks5_client_sender { - sender.send(Socks5ControlMessage::Stop).await.unwrap(); + match self.socks5_client_sender { + Some(ref mut sender) => sender + .send(Socks5ControlMessage::Stop) + .await + .map_err(|err| { + log::warn!("Failed trying to send disconnect signal: {err}"); + BackendError::CoundNotSendDisconnectSignal + }), + None => { + log::warn!( + "Trying to disconnect without being able to talk to the SOCKS5 client, \ + is it running?" + ); + Err(BackendError::CoundNotSendDisconnectSignal) + } } } - pub async fn mark_disconnected(&mut self, window: &tauri::Window) { - log::info!("Disconnected"); - self.status = ConnectionStatusKind::Disconnected; + /// Once the SOCKS5 client has stopped, this should be called by the disconnect handler to mark + /// the state as disconnected. + pub fn mark_disconnected(&mut self, window: &tauri::Window) { self.set_state(ConnectionStatusKind::Disconnected, window); } } diff --git a/nym-connect/src-tauri/src/tasks.rs b/nym-connect/src-tauri/src/tasks.rs index 9a3ef1a669..214ac33e8a 100644 --- a/nym-connect/src-tauri/src/tasks.rs +++ b/nym-connect/src-tauri/src/tasks.rs @@ -1,7 +1,7 @@ use client_core::config::GatewayEndpoint; use futures::channel::mpsc; -use log::info; use std::sync::Arc; +use tap::TapFallible; use tokio::sync::RwLock; use config::NymConfig; @@ -9,7 +9,7 @@ use config::NymConfig; use nym_socks5::client::NymClient as Socks5NymClient; use nym_socks5::client::Socks5ControlMessageSender; -use crate::state::State; +use crate::{error::Result, state::State}; pub type StatusReceiver = futures::channel::oneshot::Receiver; @@ -21,14 +21,14 @@ pub enum Socks5StatusMessage { pub fn start_nym_socks5_client( id: &str, -) -> (Socks5ControlMessageSender, GatewayEndpoint, StatusReceiver) { - info!("Loading config from file: {id}"); - // TODO: handle this gracefully! - let config = nym_socks5::client::config::Config::load_from_file(Some(id)).unwrap(); +) -> Result<(Socks5ControlMessageSender, StatusReceiver, GatewayEndpoint)> { + log::info!("Loading config from file: {id}"); + let config = nym_socks5::client::config::Config::load_from_file(Some(id)) + .tap_err(|_| log::warn!("Failed to load configuration file"))?; let used_gateway = config.get_base().get_gateway_endpoint().clone(); let mut socks5_client = Socks5NymClient::new(config); - info!("Starting socks5 client"); + log::info!("Starting socks5 client"); // Channel to send control messages to the socks5 client let (socks5_ctrl_tx, socks5_ctrl_rx) = mpsc::unbounded(); @@ -38,17 +38,20 @@ pub fn start_nym_socks5_client( // Spawn a separate runtime for the socks5 client so we can forcefully terminate. // Once we can gracefully shutdown the socks5 client we can get rid of this. + // The status channel is used to both get the state of the task, and if it's closed, to check + // for panic. std::thread::spawn(|| { - let rt = tokio::runtime::Runtime::new().unwrap(); - rt.block_on(async move { - socks5_client.run_and_listen(socks5_ctrl_rx).await; - }); + tokio::runtime::Runtime::new() + .expect("Failed to create runtime for SOCKS5 client") + .block_on(async move { socks5_client.run_and_listen(socks5_ctrl_rx).await }); log::info!("SOCKS5 task finished"); - socks5_status_tx.send(Socks5StatusMessage::Stopped).unwrap(); + socks5_status_tx + .send(Socks5StatusMessage::Stopped) + .expect("Failed to send status message back to main task"); }); - (socks5_ctrl_tx, used_gateway, socks5_status_rx) + Ok((socks5_ctrl_tx, socks5_status_rx, used_gateway)) } pub fn start_disconnect_listener( @@ -64,10 +67,12 @@ pub fn start_disconnect_listener( } Err(_) => { log::info!("SOCKS5 task appears to have stopped abruptly"); + // TODO: we should probably generate some events here, or otherwise signal to the + // frontend. } } let mut state_w = state.write().await; - state_w.mark_disconnected(&window).await; + state_w.mark_disconnected(&window); }); }