Change TunnelParametersGenerator to return errors

6 files changed, 96 insertions, 42 deletions

diff --git a/Cargo.lock b/Cargo.lock
index a21a47e007..5f8d8b155b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2308,6 +2308,7 @@ name = "talpid-types"
 version = "0.1.0"
 dependencies = [
  "base64 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "err-derive 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "hex 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "ipnetwork 0.14.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index 9fd270c81a..472e62e7d3 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -56,7 +56,7 @@ use talpid_core::{
 };
 use talpid_types::{
     net::{openvpn, TunnelParameters},
-    tunnel::{BlockReason, TunnelStateTransition},
+    tunnel::{BlockReason, ParameterGenerationError, TunnelStateTransition},
     ErrorExt,
 };
 
@@ -133,7 +133,10 @@ pub(crate) enum InternalDaemonEvent {
     /// Tunnel has changed state.
     TunnelStateTransition(TunnelStateTransition),
     /// Request from the `MullvadTunnelParametersGenerator` to obtain a new relay.
-    GenerateTunnelParameters(mpsc::Sender<TunnelParameters>, u32),
+    GenerateTunnelParameters(
+        mpsc::Sender<std::result::Result<TunnelParameters, ParameterGenerationError>>,
+        u32,
+    ),
     /// An event coming from the JSONRPC-2.0 management interface.
     ManagementInterfaceEvent(ManagementCommand),
     /// Triggered if the server hosting the JSONRPC-2.0 management interface dies unexpectedly.
@@ -538,18 +541,21 @@ where
 
     fn handle_generate_tunnel_parameters(
         &mut self,
-        tunnel_parameters_tx: &mpsc::Sender<TunnelParameters>,
+        tunnel_parameters_tx: &mpsc::Sender<
+            std::result::Result<TunnelParameters, ParameterGenerationError>,
+        >,
         retry_attempt: u32,
     ) {
         if let Some(account_token) = self.settings.get_account_token() {
-            if let Err(error_str) = match self.settings.get_relay_settings() {
+            let result = match self.settings.get_relay_settings() {
                 RelaySettings::CustomTunnelEndpoint(custom_relay) => {
                     self.last_generated_relay = None;
                     custom_relay
                         // TODO(emilsp): generate proxy settings for custom tunnels
                         .to_tunnel_parameters(self.settings.get_tunnel_options().clone(), None)
                         .map_err(|e| {
-                            e.display_chain_with_msg("Custom tunnel endpoint could not be resolved")
+                            log::error!("Failed to resolve hostname for custom tunnel config: {}", e);
+                            ParameterGenerationError::CustomTunnelHostResultionError
                         })
                 }
                 RelaySettings::Normal(constraints) => self
@@ -559,30 +565,37 @@ where
                         self.settings.get_bridge_state(),
                         retry_attempt,
                     )
-                    .map_err(|e| {
-                        e.display_chain_with_msg(
-                            "No valid relay servers match the current settings",
-                        )
-                    })
+                    .map_err(|_| ParameterGenerationError::NoMatchingRelay)
                     .and_then(|(relay, endpoint)| {
-                        let result = self
-                            .create_tunnel_parameters(
-                                &relay,
-                                endpoint,
-                                account_token,
-                                retry_attempt,
-                            )
-                            .map_err(|e| e.display_chain());
+                        let result = self.create_tunnel_parameters(
+                            &relay,
+                            endpoint,
+                            account_token,
+                            retry_attempt,
+                        );
                         self.last_generated_relay = Some(relay);
-                        result
+                        match result {
+                            Ok(result) => Ok(result),
+                            Err(Error::NoKeyAvailable) => {
+                                Err(ParameterGenerationError::NoWireguardKey)
+                            }
+                            Err(Error::NoBridgeAvailable) => {
+                                Err(ParameterGenerationError::NoMatchingBridgeRelay)
+                            }
+                            Err(err) => {
+                                log::error!(
+                                    "{}",
+                                    err.display_chain_with_msg(
+                                        "Failed to generate tunnel parameters"
+                                    )
+                                );
+                                Err(ParameterGenerationError::NoMatchingRelay)
+                            }
+                        }
                     }),
-            }
-            .and_then(|tunnel_params| {
-                tunnel_parameters_tx.send(tunnel_params).map_err(|e| {
-                    e.display_chain_with_msg("Tunnel parameters receiver stopped listening")
-                })
-            }) {
-                error!("{}", error_str);
+            };
+            if tunnel_parameters_tx.send(result).is_err() {
+                log::error!("Failed to send tunnel parameters");
             }
         } else {
             error!("No account token configured");
@@ -1439,14 +1452,25 @@ struct MullvadTunnelParametersGenerator {
 }
 
 impl TunnelParametersGenerator for MullvadTunnelParametersGenerator {
-    fn generate(&mut self, retry_attempt: u32) -> Option<TunnelParameters> {
+    fn generate(
+        &mut self,
+        retry_attempt: u32,
+    ) -> std::result::Result<TunnelParameters, ParameterGenerationError> {
         let (response_tx, response_rx) = mpsc::channel();
-        self.tx
-            .send(InternalDaemonEvent::GenerateTunnelParameters(
-                response_tx,
-                retry_attempt,
-            ))
-            .ok()
-            .and_then(|_| response_rx.recv().ok())
+        if let Err(_) = self.tx.send(InternalDaemonEvent::GenerateTunnelParameters(
+            response_tx,
+            retry_attempt,
+        )) {
+            log::error!("Failed to send daemon command to generate tunnel parameters!");
+            return Err(ParameterGenerationError::NoMatchingRelay);
+        }
+
+        match response_rx.recv() {
+            Ok(result) => result,
+            Err(_) => {
+                log::error!("Failed to receive tunnel parameter generation result!");
+                Err(ParameterGenerationError::NoMatchingRelay)
+            }
+        }
     }
 }
diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs
index 36424a08be..2c8b112537 100644
--- a/talpid-core/src/tunnel_state_machine/connecting_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs
@@ -335,8 +335,8 @@ impl TunnelState for ConnectingState {
             .tunnel_parameters_generator
             .generate(retry_attempt)
         {
-            None => BlockedState::enter(shared_values, BlockReason::NoMatchingRelay),
-            Some(tunnel_parameters) => {
+            Err(err) => BlockedState::enter(shared_values, BlockReason::TunnelParameterError(err)),
+            Ok(tunnel_parameters) => {
                 if let Err(error) = Self::set_firewall_policy(shared_values, &tunnel_parameters) {
                     error!(
                         "{}",
diff --git a/talpid-core/src/tunnel_state_machine/mod.rs b/talpid-core/src/tunnel_state_machine/mod.rs
index d947792da6..e0ea948aa2 100644
--- a/talpid-core/src/tunnel_state_machine/mod.rs
+++ b/talpid-core/src/tunnel_state_machine/mod.rs
@@ -30,7 +30,7 @@ use std::{
 };
 use talpid_types::{
     net::TunnelParameters,
-    tunnel::{BlockReason, TunnelStateTransition},
+    tunnel::{BlockReason, ParameterGenerationError, TunnelStateTransition},
     ErrorExt,
 };
 use tokio_core::reactor::Core;
@@ -280,12 +280,16 @@ impl<T: TunnelState> From<EventConsequence<T>> for TunnelStateMachineAction {
     }
 }
 
+
 /// Trait for any type that can provide a stream of `TunnelParameters` to the `TunnelStateMachine`.
 pub trait TunnelParametersGenerator: Send + 'static {
     /// Given the number of consecutive failed retry attempts, it should yield a `TunnelParameters`
     /// to establish a tunnel with.
     /// If this returns `None` then the state machine goes into the `Blocked` state.
-    fn generate(&mut self, retry_attempt: u32) -> Option<TunnelParameters>;
+    fn generate(
+        &mut self,
+        retry_attempt: u32,
+    ) -> Result<TunnelParameters, ParameterGenerationError>;
 }
 
 /// Values that are common to all tunnel states.
diff --git a/talpid-types/Cargo.toml b/talpid-types/Cargo.toml
index 9fa2bd04a2..bfcb2bed5d 100644
--- a/talpid-types/Cargo.toml
+++ b/talpid-types/Cargo.toml
@@ -13,3 +13,4 @@ base64 = "0.10"
 hex = "0.3"
 x25519-dalek = { version = "0.4.5", features = [ "std", "u64_backend" ], default-features = false }
 rand = "0.7"
+err-derive = "0.1.5"
diff --git a/talpid-types/src/tunnel.rs b/talpid-types/src/tunnel.rs
index 4f2e630c2d..55098ed4af 100644
--- a/talpid-types/src/tunnel.rs
+++ b/talpid-types/src/tunnel.rs
@@ -1,6 +1,6 @@
 use crate::net::TunnelEndpoint;
 use serde::{Deserialize, Serialize};
-use std::fmt;
+use std::{error::Error, fmt};
 
 /// Event resulting from a transition to a new tunnel state.
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
@@ -52,14 +52,32 @@ pub enum BlockReason {
     SetDnsError,
     /// Failed to start connection to remote server.
     StartTunnelError,
-    /// No relay server matching the current filter parameters.
-    NoMatchingRelay,
+    /// Tunnel parameter generation failure
+    TunnelParameterError(ParameterGenerationError),
     /// This device is offline, no tunnels can be established.
     IsOffline,
     /// A problem with the TAP adapter has been detected.
     TapAdapterProblem,
 }
 
+/// Errors that can occur when generating tunnel parameters.
+#[derive(err_derive::Error, Debug, Serialize, Clone, PartialEq, Deserialize)]
+#[serde(rename_all = "snake_case")]
+pub enum ParameterGenerationError {
+    /// Failure to select a matching tunnel relay
+    #[error(display = "Failure to select a matching tunnel relay")]
+    NoMatchingRelay,
+    /// Failure to select a matching bridge relay
+    #[error(display = "Failure to select a matching bridge relay")]
+    NoMatchingBridgeRelay,
+    /// Returned when tunnel parameters can't be generated because wireguard key is not available.
+    #[error(display = "No wireguard key available")]
+    NoWireguardKey,
+    /// Failure to resolve the hostname of a custom tunnel configuration
+    #[error(display = "Can't resolve hostname for custom tunnel host")]
+    CustomTunnelHostResultionError,
+}
+
 impl fmt::Display for BlockReason {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         use self::BlockReason::*;
@@ -78,7 +96,13 @@ impl fmt::Display for BlockReason {
             SetFirewallPolicyError => "Failed to set firewall policy",
             SetDnsError => "Failed to set system DNS server",
             StartTunnelError => "Failed to start connection to remote server",
-            NoMatchingRelay => "No relay server matches the current settings",
+            TunnelParameterError(ref err) => {
+                return write!(
+                    f,
+                    "Failure to generate tunnel parameters: {}",
+                    err.description(),
+                );
+            }
             IsOffline => "This device is offline, no tunnels can be established",
             TapAdapterProblem => "A problem with the TAP adapter has been detected",
         };