Factor out error types in talpid-core::tunnel::wireguard

6 files changed, 129 insertions, 99 deletions

diff --git a/talpid-core/src/tunnel/wireguard/connectivity_check.rs b/talpid-core/src/tunnel/wireguard/connectivity_check.rs
index 2b85b748bd..63f296d2e3 100644
--- a/talpid-core/src/tunnel/wireguard/connectivity_check.rs
+++ b/talpid-core/src/tunnel/wireguard/connectivity_check.rs
@@ -5,7 +5,7 @@ use std::{
     time::{Duration, Instant},
 };
 
-use super::{Error, Tunnel};
+use super::{Tunnel, TunnelError};
 
 /// Sleep time used when initially establishing connectivity
 const DELAY_ON_INITIAL_SETUP: Duration = Duration::from_millis(50);
@@ -26,6 +26,18 @@ const PING_TIMEOUT: Duration = Duration::from_secs(15);
 /// Number of seconds to wait between sending ICMP packets
 const SECONDS_PER_PING: Duration = Duration::from_secs(3);
 
+/// Connectivity monitor errors
+#[derive(err_derive::Error, Debug)]
+pub enum Error {
+    /// Failed to read tunnel's configuration
+    #[error(display = "Failed to read tunnel's configuration")]
+    ConfigReadError(TunnelError),
+
+    /// Failed to send ping
+    #[error(display = "Ping monitor failed")]
+    PingError(#[error(source)] crate::ping_monitor::Error),
+}
+
 
 /// Verifies if a connection to a tunnel is working.
 /// The connectivity monitor is biased to receiving traffic - it is expected that all outgoing
@@ -144,7 +156,7 @@ impl ConnectivityMonitor {
             .lock()
             .ok()?
             .as_ref()
-            .map(|tunnel| tunnel.get_config())
+            .map(|tunnel| tunnel.get_tunnel_stats().map_err(Error::ConfigReadError))
     }
 
     fn maybe_send_ping(&mut self) -> Result<(), Error> {
diff --git a/talpid-core/src/tunnel/wireguard/logging.rs b/talpid-core/src/tunnel/wireguard/logging.rs
index 5d177441e3..1a122ddca8 100644
--- a/talpid-core/src/tunnel/wireguard/logging.rs
+++ b/talpid-core/src/tunnel/wireguard/logging.rs
@@ -1,4 +1,3 @@
-use super::{Error, Result};
 use chrono;
 use parking_lot::Mutex;
 use std::{collections::HashMap, fs, io::Write, path::Path};
@@ -9,7 +8,15 @@ lazy_static::lazy_static! {
 
 static mut LOG_CONTEXT_NEXT_ORDINAL: u32 = 0;
 
-pub fn initialize_logging(log_path: Option<&Path>) -> Result<u32> {
+/// Errors encountered when initializing logging
+#[derive(err_derive::Error, Debug)]
+pub enum Error {
+    /// Failed to move or create a log file.
+    #[error(display = "Failed to setup a logging file")]
+    PrepareLogFileError(#[error(source)] std::io::Error),
+}
+
+pub fn initialize_logging(log_path: Option<&Path>) -> Result<u32, Error> {
     let log_file = create_log_file(log_path)?;
 
     let log_context_ordinal = unsafe {
@@ -29,7 +36,7 @@ static NULL_DEVICE: &str = "NUL";
 #[cfg(not(target_os = "windows"))]
 static NULL_DEVICE: &str = "/dev/null";
 
-fn create_log_file(log_path: Option<&Path>) -> Result<fs::File> {
+fn create_log_file(log_path: Option<&Path>) -> Result<fs::File, Error> {
     fs::File::create(log_path.unwrap_or(NULL_DEVICE.as_ref())).map_err(Error::PrepareLogFileError)
 }
 
diff --git a/talpid-core/src/tunnel/wireguard/mod.rs b/talpid-core/src/tunnel/wireguard/mod.rs
index 1381681a5b..7d081bd226 100644
--- a/talpid-core/src/tunnel/wireguard/mod.rs
+++ b/talpid-core/src/tunnel/wireguard/mod.rs
@@ -2,10 +2,9 @@ use self::config::Config;
 #[cfg(not(windows))]
 use super::tun_provider;
 use super::{tun_provider::TunProvider, TunnelEvent, TunnelMetadata};
-use crate::{ping_monitor, routing};
+use crate::routing;
 use std::{
     collections::HashMap,
-    io,
     path::Path,
     sync::{mpsc, Arc, Mutex},
 };
@@ -24,81 +23,22 @@ type Result<T> = std::result::Result<T, Error>;
 
 /// Errors that can happen in the Wireguard tunnel monitor.
 #[derive(err_derive::Error, Debug)]
-#[error(no_from)]
 pub enum Error {
-    /// Failed to setup a tunnel device.
-    #[cfg(not(windows))]
-    #[error(display = "Failed to create tunnel device")]
-    SetupTunnelDeviceError(#[error(source)] tun_provider::Error),
-
-    /// A recoverable error occurred while starting the wireguard tunnel
-    ///
-    /// This is an error returned by wireguard-go that indicates that trying to establish the
-    /// tunnel again should work normally. The error encountered is known to be sporadic.
-    #[error(display = "Recoverable error while starting wireguard tunnel")]
-    RecoverableStartWireguardError,
-
-    /// An unrecoverable error occurred while starting the wireguard tunnel
-    ///
-    /// This is an error returned by wireguard-go that indicates that trying to establish the
-    /// tunnel again will likely fail with the same error. An error was encountered during tunnel
-    /// configuration which can't be dealt with gracefully.
-    #[error(display = "Failed to start wireguard tunnel")]
-    FatalStartWireguardError,
-
-    /// Failed to tear down wireguard tunnel.
-    #[error(display = "Failed to stop wireguard tunnel - {}", status)]
-    StopWireguardError {
-        /// Returned error code
-        status: i32,
-    },
-
-    /// Failed to get tunnel config
-    #[error(display = "Failed to obtain tunnel config")]
-    GetConfigError,
-
-    /// Failed to set ip addresses on tunnel interface.
-    #[cfg(target_os = "windows")]
-    #[error(display = "Failed to set IP addresses on WireGuard interface")]
-    SetIpAddressesError,
-
     /// Failed to set up routing.
     #[error(display = "Failed to setup routing")]
     SetupRoutingError(#[error(source)] crate::routing::Error),
 
-    /// Failed to move or craete a log file.
-    #[error(display = "Failed to setup a logging file")]
-    PrepareLogFileError(#[error(source)] io::Error),
-
-    /// Invalid tunnel interface name.
-    #[error(display = "Invalid tunnel interface name")]
-    InterfaceNameError(#[error(source)] std::ffi::NulError),
-
-    /// Failed to configure Wireguard sockets to bypass the tunnel.
-    #[cfg(target_os = "android")]
-    #[error(display = "Failed to configure Wireguard sockets to bypass the tunnel")]
-    BypassError(#[error(source)] tun_provider::Error),
-
-    /// Failed to duplicate tunnel file descriptor for wireguard-go
-    #[cfg(any(target_os = "linux", target_os = "macos", target_os = "android"))]
-    #[error(display = "Failed to duplicate tunnel file descriptor for wireguard-go")]
-    FdDuplicationError(#[error(source)] nix::Error),
-
-    /// Error whilst trying to read stats
-    #[error(display = "Reading tunnel stats failed")]
-    StatsError(#[error(source)] stats::Error),
-
-    /// Tunnel handle is invalid
-    #[error(display = "Tunnel handle is invalid")]
-    InvalidTunnelHandle,
-
-    /// Pinging timed out.
-    #[error(display = "Ping timed out")]
-    PingError(#[error(source)] ping_monitor::Error),
-
     /// Tunnel timed out
     #[error(display = "Tunnel timed out")]
     TimeoutError,
+
+    /// An interaction with a tunnel failed
+    #[error(display = "Tunnel failed")]
+    TunnelError(#[error(source)] TunnelError),
+
+    /// Failed to setup connectivity monitor
+    #[error(display = "Connectivity monitor failed")]
+    ConnectivityMonitorError(#[error(source)] connectivity_check::Error),
 }
 
 
@@ -296,6 +236,69 @@ impl CloseHandle {
 
 pub(crate) trait Tunnel: Send {
     fn get_interface_name(&self) -> &str;
-    fn stop(self: Box<Self>) -> Result<()>;
-    fn get_config(&self) -> Result<stats::Stats>;
+    fn stop(self: Box<Self>) -> std::result::Result<(), TunnelError>;
+    fn get_tunnel_stats(&self) -> std::result::Result<stats::Stats, TunnelError>;
+}
+
+/// Errors to be returned from WireGuard implementations, namely implementers of the Tunnel trait
+#[derive(err_derive::Error, Debug)]
+#[error(no_from)]
+pub enum TunnelError {
+    /// A recoverable error occurred while starting the wireguard tunnel
+    ///
+    /// This is an error returned by wireguard-go that indicates that trying to establish the
+    /// tunnel again should work normally. The error encountered is known to be sporadic.
+    #[error(display = "Recoverable error while starting wireguard tunnel")]
+    RecoverableStartWireguardError,
+
+    /// An unrecoverable error occurred while starting the wireguard tunnel
+    ///
+    /// This is an error returned by wireguard-go that indicates that trying to establish the
+    /// tunnel again will likely fail with the same error. An error was encountered during tunnel
+    /// configuration which can't be dealt with gracefully.
+    #[error(display = "Failed to start wireguard tunnel")]
+    FatalStartWireguardError,
+
+    /// Failed to tear down wireguard tunnel.
+    #[error(display = "Failed to stop wireguard tunnel - {}", status)]
+    StopWireguardError {
+        /// Returned error code
+        status: i32,
+    },
+
+    /// Error whilst trying to parse the WireGuard config to read the stats
+    #[error(display = "Reading tunnel stats failed")]
+    StatsError(#[error(source)] stats::Error),
+
+    /// Error whilst trying to retrieve config of a WireGuard tunnel
+    #[error(display = "Failed to get config of WireGuard tunnel")]
+    GetConfigError,
+
+    /// Failed to duplicate tunnel file descriptor for wireguard-go
+    #[cfg(any(target_os = "linux", target_os = "macos", target_os = "android"))]
+    #[error(display = "Failed to duplicate tunnel file descriptor for wireguard-go")]
+    FdDuplicationError(#[error(source)] nix::Error),
+
+    /// Failed to setup a tunnel device.
+    #[cfg(not(windows))]
+    #[error(display = "Failed to create tunnel device")]
+    SetupTunnelDeviceError(#[error(source)] tun_provider::Error),
+
+    /// Failed to configure Wireguard sockets to bypass the tunnel.
+    #[cfg(target_os = "android")]
+    #[error(display = "Failed to configure Wireguard sockets to bypass the tunnel")]
+    BypassError(#[error(source)] tun_provider::Error),
+
+    /// Invalid tunnel interface name.
+    #[error(display = "Invalid tunnel interface name")]
+    InterfaceNameError(#[error(source)] std::ffi::NulError),
+
+    /// Failed to set ip addresses on tunnel interface.
+    #[cfg(target_os = "windows")]
+    #[error(display = "Failed to set IP addresses on WireGuard interface")]
+    SetIpAddressesError,
+
+    /// Failure to set up logging
+    #[error(display = "Failed to set up logging")]
+    LoggingError(#[error(source)] logging::Error),
 }
diff --git a/talpid-core/src/tunnel/wireguard/stats.rs b/talpid-core/src/tunnel/wireguard/stats.rs
index 545a49d688..474a096ada 100644
--- a/talpid-core/src/tunnel/wireguard/stats.rs
+++ b/talpid-core/src/tunnel/wireguard/stats.rs
@@ -1,5 +1,4 @@
 #[derive(err_derive::Error, Debug, PartialEq)]
-#[error(no_from)]
 pub enum Error {
     #[error(display = "Failed to parse integer from string \"_0\"")]
     IntParseError(String, #[error(source)] std::num::ParseIntError),
diff --git a/talpid-core/src/tunnel/wireguard/wireguard_go.rs b/talpid-core/src/tunnel/wireguard/wireguard_go.rs
index eb657a04aa..058735536a 100644
--- a/talpid-core/src/tunnel/wireguard/wireguard_go.rs
+++ b/talpid-core/src/tunnel/wireguard/wireguard_go.rs
@@ -1,4 +1,4 @@
-use super::{stats::Stats, Config, Error, Result, Tunnel};
+use super::{stats::Stats, Config, Tunnel, TunnelError};
 use crate::tunnel::{
     tun_provider::TunProvider,
     wireguard::logging::{clean_up_logging, initialize_logging, logging_callback, WgLogLevel},
@@ -26,6 +26,8 @@ use {
     },
 };
 
+type Result<T> = std::result::Result<T, TunnelError>;
+
 #[cfg(target_os = "windows")]
 use crate::winnet::{self, add_device_ip_addresses};
 
@@ -63,7 +65,9 @@ impl WgGoTunnel {
         let (mut tunnel_device, tunnel_fd) = Self::get_tunnel(tun_provider, config, routes)?;
         let interface_name: String = tunnel_device.interface_name().to_string();
         let wg_config_str = config.to_userspace_format();
-        let logging_context = initialize_logging(log_path).map(LoggingContext)?;
+        let logging_context = initialize_logging(log_path)
+            .map(LoggingContext)
+            .map_err(TunnelError::LoggingError)?;
 
         #[cfg(not(target_os = "android"))]
         let handle = unsafe {
@@ -89,14 +93,15 @@ impl WgGoTunnel {
         if handle < 0 {
             // Error values returned from the wireguard-go library
             return match handle {
-                -1 => Err(Error::FatalStartWireguardError),
-                -2 => Err(Error::RecoverableStartWireguardError),
+                -1 => Err(TunnelError::FatalStartWireguardError),
+                -2 => Err(TunnelError::RecoverableStartWireguardError),
                 _ => unreachable!("Unknown status code returned from wireguard-go"),
             };
         }
 
         #[cfg(target_os = "android")]
-        Self::bypass_tunnel_sockets(&mut tunnel_device, handle).map_err(Error::BypassError)?;
+        Self::bypass_tunnel_sockets(&mut tunnel_device, handle)
+            .map_err(TunnelError::BypassError)?;
 
         Ok(WgGoTunnel {
             interface_name,
@@ -116,8 +121,10 @@ impl WgGoTunnel {
         let wg_config_str = config.to_userspace_format();
         let iface_name: String = "wg-mullvad".to_string();
         let cstr_iface_name =
-            CString::new(iface_name.as_bytes()).map_err(Error::InterfaceNameError)?;
-        let logging_context = initialize_logging(log_path).map(LoggingContext)?;
+            CString::new(iface_name.as_bytes()).map_err(TunnelError::InterfaceNameError)?;
+        let logging_context = initialize_logging(log_path)
+            .map(LoggingContext)
+            .map_err(TunnelError::LoggingError)?;
 
         let handle = unsafe {
             wgTurnOn(
@@ -130,12 +137,12 @@ impl WgGoTunnel {
         };
 
         if handle < 0 {
-            return Err(Error::FatalStartWireguardError);
+            return Err(TunnelError::FatalStartWireguardError);
         }
 
         if !add_device_ip_addresses(&iface_name, &config.tunnel.addresses) {
             // Todo: what kind of clean-up is required?
-            return Err(Error::SetIpAddressesError);
+            return Err(TunnelError::SetIpAddressesError);
         }
 
         Ok(WgGoTunnel {
@@ -224,7 +231,7 @@ impl WgGoTunnel {
         if let Some(handle) = self.handle.take() {
             let status = unsafe { wgTurnOff(handle) };
             if status < 0 {
-                return Err(Error::StopWireguardError { status });
+                return Err(TunnelError::StopWireguardError { status });
             }
         }
         Ok(())
@@ -242,18 +249,18 @@ impl WgGoTunnel {
         for _ in 1..=MAX_PREPARE_TUN_ATTEMPTS {
             let tunnel_device = tun_provider
                 .get_tun(tunnel_config.clone())
-                .map_err(Error::SetupTunnelDeviceError)?;
+                .map_err(TunnelError::SetupTunnelDeviceError)?;
 
             match nix::unistd::dup(tunnel_device.as_raw_fd()) {
                 Ok(fd) => return Ok((tunnel_device, fd)),
                 #[cfg(not(target_os = "macos"))]
                 Err(error @ nix::Error::Sys(nix::errno::Errno::EBADFD)) => last_error = Some(error),
                 Err(error @ nix::Error::Sys(nix::errno::Errno::EBADF)) => last_error = Some(error),
-                Err(error) => return Err(Error::FdDuplicationError(error)),
+                Err(error) => return Err(TunnelError::FdDuplicationError(error)),
             }
         }
 
-        Err(Error::FdDuplicationError(
+        Err(TunnelError::FdDuplicationError(
             last_error.expect("Should be collected in loop"),
         ))
     }
@@ -272,12 +279,12 @@ impl Tunnel for WgGoTunnel {
         &self.interface_name
     }
 
-    fn get_config(&self) -> Result<Stats> {
+    fn get_tunnel_stats(&self) -> Result<Stats> {
         let config_str = unsafe {
             let ptr = wgGetConfig(self.handle.unwrap());
             if ptr.is_null() {
                 log::error!("Failed to get config !");
-                return Err(Error::GetConfigError);
+                return Err(TunnelError::GetConfigError);
             }
 
             CStr::from_ptr(ptr)
@@ -285,7 +292,7 @@ impl Tunnel for WgGoTunnel {
 
         let result =
             Stats::parse_config_str(config_str.to_str().expect("Go strings are always UTF-8"))
-                .map_err(Error::StatsError);
+                .map_err(TunnelError::StatsError);
         unsafe {
             // Zeroing out config string to not leave private key in memory.
             let slice = std::slice::from_raw_parts_mut(
diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs
index 848b2c7485..77608c4c3d 100644
--- a/talpid-core/src/tunnel_state_machine/connecting_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs
@@ -295,16 +295,18 @@ fn get_openvpn_proxy_settings(
 }
 
 fn should_retry(error: &tunnel::Error) -> bool {
+    #[cfg(not(windows))]
+    use tunnel::wireguard::{Error, TunnelError};
     match error {
         #[cfg(not(windows))]
-        tunnel::Error::WireguardTunnelMonitoringError(
-            tunnel::wireguard::Error::RecoverableStartWireguardError,
-        ) => true,
+        tunnel::Error::WireguardTunnelMonitoringError(Error::TunnelError(
+            TunnelError::RecoverableStartWireguardError,
+        )) => true,
 
         #[cfg(target_os = "android")]
-        tunnel::Error::WireguardTunnelMonitoringError(tunnel::wireguard::Error::BypassError(_)) => {
-            true
-        }
+        tunnel::Error::WireguardTunnelMonitoringError(Error::TunnelError(
+            TunnelError::BypassError(_),
+        )) => true,
 
         _ => false,
     }