Simplify retry logic in connecting state

7 files changed, 110 insertions, 53 deletions

diff --git a/talpid-core/src/tunnel/mod.rs b/talpid-core/src/tunnel/mod.rs
index 796a327193..3e6df61f76 100644
--- a/talpid-core/src/tunnel/mod.rs
+++ b/talpid-core/src/tunnel/mod.rs
@@ -53,6 +53,28 @@ pub enum Error {
     AssignMtuError,
 }
 
+impl Error {
+    /// Return whether retrying the operation that caused this error is likely to succeed.
+    pub fn is_recoverable(&self) -> bool {
+        match self {
+            Error::WireguardTunnelMonitoringError(error) => error.is_recoverable(),
+            #[cfg(not(target_os = "android"))]
+            Error::OpenVpnTunnelMonitoringError(error) => error.is_recoverable(),
+            _ => false,
+        }
+    }
+
+    /// Get the inner tunnel device error, if there is one
+    #[cfg(target_os = "windows")]
+    pub fn get_tunnel_device_error(&self) -> Option<&std::io::Error> {
+        match self {
+            Error::WireguardTunnelMonitoringError(error) => error.get_tunnel_device_error(),
+            Error::OpenVpnTunnelMonitoringError(error) => error.get_tunnel_device_error(),
+            _ => None,
+        }
+    }
+}
+
 /// Abstraction for monitoring a generic VPN tunnel.
 pub struct TunnelMonitor {
     monitor: InternalTunnelMonitor,
diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs
index ec8ddc5f2c..1a000c6136 100644
--- a/talpid-core/src/tunnel_state_machine/connecting_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs
@@ -265,7 +265,7 @@ impl ConnectingState {
                         #[cfg(target_os = "android")]
                         tunnel::Error::WireguardTunnelMonitoringError(
                             talpid_wireguard::Error::TunnelError(
-                                talpid_wireguard::TunnelError::SetupTunnelDeviceError(
+                                talpid_wireguard::TunnelError::SetupTunnelDevice(
                                     tun_provider::Error::PermissionDenied,
                                 ),
                             ),
@@ -273,7 +273,7 @@ impl ConnectingState {
                         #[cfg(target_os = "android")]
                         tunnel::Error::WireguardTunnelMonitoringError(
                             talpid_wireguard::Error::TunnelError(
-                                talpid_wireguard::TunnelError::SetupTunnelDeviceError(
+                                talpid_wireguard::TunnelError::SetupTunnelDevice(
                                     tun_provider::Error::InvalidDnsServers(addresses),
                                 ),
                             ),
@@ -580,56 +580,11 @@ impl ConnectingState {
 
 #[cfg_attr(not(target_os = "windows"), allow(unused_variables))]
 fn should_retry(error: &tunnel::Error, retry_attempt: u32) -> bool {
-    use talpid_wireguard::{Error, TunnelError};
-
-    match error {
-        tunnel::Error::WireguardTunnelMonitoringError(Error::CreateObfuscatorError(_)) => true,
-
-        tunnel::Error::WireguardTunnelMonitoringError(Error::ObfuscatorError(_)) => true,
-
-        tunnel::Error::WireguardTunnelMonitoringError(Error::PskNegotiationError(_)) => true,
-
-        tunnel::Error::WireguardTunnelMonitoringError(Error::TunnelError(
-            TunnelError::RecoverableStartWireguardError,
-        )) => true,
-
-        #[cfg(target_os = "android")]
-        tunnel::Error::WireguardTunnelMonitoringError(Error::TunnelError(
-            TunnelError::BypassError(_),
-        )) => true,
-
-        #[cfg(any(target_os = "windows", target_os = "macos"))]
-        tunnel::Error::WireguardTunnelMonitoringError(Error::SetupRoutingError(error)) => {
-            is_recoverable_routing_error(error)
-        }
-
-        #[cfg(windows)]
-        tunnel::Error::WireguardTunnelMonitoringError(Error::TunnelError(
-            TunnelError::SetupTunnelDevice(_),
-        ))
-        | tunnel::Error::OpenVpnTunnelMonitoringError(
-            talpid_openvpn::Error::WintunCreateAdapterError(_),
-        ) if retry_attempt < MAX_ATTEMPT_CREATE_TUN => true,
-
-        _ => false,
+    #[cfg(target_os = "windows")]
+    if error.get_tunnel_device_error().is_some() && retry_attempt < MAX_ATTEMPT_CREATE_TUN {
+        return true;
     }
-}
-
-#[cfg(windows)]
-fn is_recoverable_routing_error(error: &talpid_routing::Error) -> bool {
-    matches!(error, talpid_routing::Error::AddRoutesFailed(_))
-}
-
-#[cfg(target_os = "macos")]
-fn is_recoverable_routing_error(error: &talpid_routing::Error) -> bool {
-    // If the default route disappears while connecting but before it is caught by the offline
-    // monitor, then the gateway will be unreachable. In this case, just retry.
-    matches!(
-        error,
-        talpid_routing::Error::PlatformError(talpid_routing::PlatformError::AddRoute(
-            talpid_routing::RouteError::Unreachable,
-        ))
-    )
+    error.is_recoverable()
 }
 
 impl TunnelState for ConnectingState {
diff --git a/talpid-openvpn/src/lib.rs b/talpid-openvpn/src/lib.rs
index f47072a9f6..1cd58bdf11 100644
--- a/talpid-openvpn/src/lib.rs
+++ b/talpid-openvpn/src/lib.rs
@@ -125,6 +125,28 @@ pub enum Error {
     ParseRemoteHost(#[error(source)] std::net::AddrParseError),
 }
 
+impl Error {
+    /// Return whether retrying the operation that caused this error is likely to succeed.
+    pub fn is_recoverable(&self) -> bool {
+        match self {
+            #[cfg(windows)]
+            _ => self.get_tunnel_device_error().is_some(),
+
+            #[cfg(not(windows))]
+            _ => false,
+        }
+    }
+
+    /// Get the inner tunnel device error, if there is one
+    #[cfg(target_os = "windows")]
+    pub fn get_tunnel_device_error(&self) -> Option<&io::Error> {
+        match self {
+            Error::WintunCreateAdapterError(ref error) => Some(error),
+            _ => None,
+        }
+    }
+}
+
 #[cfg(unix)]
 static OPENVPN_DIE_TIMEOUT: Duration = Duration::from_secs(4);
 #[cfg(windows)]
diff --git a/talpid-routing/src/unix/mod.rs b/talpid-routing/src/unix/mod.rs
index 863e3a3aac..a0952e43be 100644
--- a/talpid-routing/src/unix/mod.rs
+++ b/talpid-routing/src/unix/mod.rs
@@ -52,6 +52,25 @@ pub enum Error {
     RouteManagerDown,
 }
 
+impl Error {
+    /// Return whether retrying the operation that caused this error is likely to succeed.
+    #[cfg(target_os = "macos")]
+    pub fn is_recoverable(&self) -> bool {
+        // If the default route disappears while connecting but before it is caught by the offline
+        // monitor, then the gateway will be unreachable. In this case, just retry.
+        matches!(
+            self,
+            Error::PlatformError(PlatformError::AddRoute(imp::RouteError::Unreachable,))
+        )
+    }
+
+    /// Return whether retrying the operation that caused this error is likely to succeed.
+    #[cfg(not(target_os = "macos"))]
+    pub fn is_recoverable(&self) -> bool {
+        false
+    }
+}
+
 /// Handle to a route manager.
 #[derive(Clone)]
 pub struct RouteManagerHandle {
diff --git a/talpid-routing/src/windows/mod.rs b/talpid-routing/src/windows/mod.rs
index 845366753b..7f30327cb0 100644
--- a/talpid-routing/src/windows/mod.rs
+++ b/talpid-routing/src/windows/mod.rs
@@ -91,6 +91,13 @@ pub enum Error {
     GetDeviceByGateway,
 }
 
+impl Error {
+    /// Return whether retrying the operation that caused this error is likely to succeed.
+    pub fn is_recoverable(&self) -> bool {
+        matches!(self, Error::AddRoutesFailed(_))
+    }
+}
+
 pub type Result<T> = std::result::Result<T, Error>;
 
 /// Manages routes by calling into WinNet
diff --git a/talpid-wireguard/src/lib.rs b/talpid-wireguard/src/lib.rs
index 82c3483e22..e8a63d0b1b 100644
--- a/talpid-wireguard/src/lib.rs
+++ b/talpid-wireguard/src/lib.rs
@@ -104,6 +104,38 @@ pub enum Error {
     SetIpAddressesError(#[error(source)] talpid_windows::net::Error),
 }
 
+impl Error {
+    /// Return whether retrying the operation that caused this error is likely to succeed.
+    pub fn is_recoverable(&self) -> bool {
+        match self {
+            Error::CreateObfuscatorError(_) => true,
+            Error::ObfuscatorError(_) => true,
+            Error::PskNegotiationError(_) => true,
+            Error::TunnelError(TunnelError::RecoverableStartWireguardError) => true,
+
+            Error::SetupRoutingError(error) => error.is_recoverable(),
+
+            #[cfg(target_os = "android")]
+            Error::TunnelError(TunnelError::BypassError(_)) => true,
+
+            #[cfg(windows)]
+            _ => self.get_tunnel_device_error().is_some(),
+
+            #[cfg(not(windows))]
+            _ => false,
+        }
+    }
+
+    /// Get the inner tunnel device error, if there is one
+    #[cfg(windows)]
+    pub fn get_tunnel_device_error(&self) -> Option<&io::Error> {
+        match self {
+            Error::TunnelError(TunnelError::SetupTunnelDevice(error)) => Some(error),
+            _ => None,
+        }
+    }
+}
+
 /// Spawns and monitors a wireguard tunnel
 pub struct WireguardMonitor {
     runtime: tokio::runtime::Handle,
@@ -983,7 +1015,7 @@ pub enum TunnelError {
     /// Failed to setup a tunnel device.
     #[cfg(not(windows))]
     #[error(display = "Failed to create tunnel device")]
-    SetupTunnelDeviceError(#[error(source)] tun_provider::Error),
+    SetupTunnelDevice(#[error(source)] tun_provider::Error),
 
     /// Failed to set up a tunnel device
     #[cfg(windows)]
diff --git a/talpid-wireguard/src/wireguard_go.rs b/talpid-wireguard/src/wireguard_go.rs
index 0e3ea3e9ce..28c05676de 100644
--- a/talpid-wireguard/src/wireguard_go.rs
+++ b/talpid-wireguard/src/wireguard_go.rs
@@ -159,7 +159,7 @@ impl WgGoTunnel {
         for _ in 1..=MAX_PREPARE_TUN_ATTEMPTS {
             let tunnel_device = tun_provider
                 .get_tun(tunnel_config.clone())
-                .map_err(TunnelError::SetupTunnelDeviceError)?;
+                .map_err(TunnelError::SetupTunnelDevice)?;
 
             match nix::unistd::dup(tunnel_device.as_raw_fd()) {
                 Ok(fd) => return Ok((tunnel_device, fd)),