Merge branch 'cleanup-device-err'

3 files changed, 37 insertions, 59 deletions

diff --git a/mullvad-api/src/rest.rs b/mullvad-api/src/rest.rs
index 9211450cbb..39ed98d370 100644
--- a/mullvad-api/src/rest.rs
+++ b/mullvad-api/src/rest.rs
@@ -729,20 +729,16 @@ fn flatten_result<T, E>(
     }
 }
 
-impl From<hyper::Error> for Error {
-    fn from(value: hyper::Error) -> Self {
-        Error::HyperError(Arc::new(value))
-    }
-}
-
-impl From<serde_json::Error> for Error {
-    fn from(value: serde_json::Error) -> Self {
-        Error::DeserializeError(Arc::new(value))
-    }
+macro_rules! impl_into_arc_err {
+    ($ty:ty) => {
+        impl From<$ty> for Error {
+            fn from(error: $ty) -> Self {
+                Error::from(Arc::from(error))
+            }
+        }
+    };
 }
 
-impl From<http::Error> for Error {
-    fn from(value: http::Error) -> Self {
-        Error::HttpError(Arc::new(value))
-    }
-}
+impl_into_arc_err!(hyper::Error);
+impl_into_arc_err!(serde_json::Error);
+impl_into_arc_err!(http::Error);
diff --git a/mullvad-daemon/src/device/mod.rs b/mullvad-daemon/src/device/mod.rs
index df5606a143..a73d77cbf6 100644
--- a/mullvad-daemon/src/device/mod.rs
+++ b/mullvad-daemon/src/device/mod.rs
@@ -48,7 +48,7 @@ const LOGOUT_TIMEOUT: Duration = Duration::from_secs(2);
 /// to set up a WireGuard tunnel.
 const WG_DEVICE_CHECK_THRESHOLD: usize = 2;
 
-#[derive(err_derive::Error, Debug)]
+#[derive(err_derive::Error, Debug, Clone)]
 pub enum Error {
     #[error(display = "The account already has a maximum number of devices")]
     MaxDevicesReached,
@@ -63,22 +63,32 @@ pub enum Error {
     #[error(display = "The voucher has already been used")]
     UsedVoucher,
     #[error(display = "Failed to read or write device cache")]
-    DeviceIoError(#[error(source)] io::Error),
+    DeviceIoError(#[error(source)] Arc<io::Error>),
     #[error(display = "Failed parse device cache")]
-    ParseDeviceCache(#[error(source)] serde_json::Error),
+    ParseDeviceCache(#[error(source)] Arc<serde_json::Error>),
     #[error(display = "Unexpected HTTP request error")]
     OtherRestError(#[error(source)] rest::Error),
     #[error(display = "The device update task is not running")]
     Cancelled,
-    /// Intended to be broadcast to requesters
-    #[error(display = "Broadcast error")]
-    ResponseFailure(#[error(source)] Arc<Error>),
     #[error(display = "Account changed during operation")]
     AccountChange,
     #[error(display = "The account manager is down")]
     AccountManagerDown,
 }
 
+macro_rules! impl_into_arc_err {
+    ($ty:ty) => {
+        impl From<$ty> for Error {
+            fn from(error: $ty) -> Self {
+                Error::from(Arc::from(error))
+            }
+        }
+    };
+}
+
+impl_into_arc_err!(io::Error);
+impl_into_arc_err!(serde_json::Error);
+
 /// Contains the current device state.
 #[derive(Debug, Clone, serde::Deserialize, serde::Serialize, PartialEq, Eq)]
 #[serde(rename_all = "snake_case")]
@@ -272,27 +282,11 @@ impl PrivateDeviceEvent {
 
 impl Error {
     pub fn is_network_error(&self) -> bool {
-        if let Error::OtherRestError(error) = self.unpack() {
-            error.is_network_error()
-        } else {
-            false
-        }
+        matches!(self, Error::OtherRestError(error) if error.is_network_error())
     }
 
     pub fn is_aborted(&self) -> bool {
-        if let Error::OtherRestError(error) = self.unpack() {
-            error.is_aborted()
-        } else {
-            false
-        }
-    }
-
-    pub fn unpack(&self) -> &Error {
-        if let Error::ResponseFailure(ref inner) = self {
-            inner
-        } else {
-            self
-        }
+        matches!(self, Error::OtherRestError(error) if error.is_aborted())
     }
 }
 
@@ -787,10 +781,7 @@ impl AccountManager {
             }
             Err(err) => {
                 log::error!("Failed to check account expiry: {}", err);
-                let cloneable_err = Arc::new(err);
-                Self::drain_requests(&mut self.expiry_requests, || {
-                    Err(Error::ResponseFailure(cloneable_err.clone()))
-                });
+                Self::drain_requests(&mut self.expiry_requests, || Err(err.clone()));
             }
         }
     }
@@ -827,10 +818,10 @@ impl AccountManager {
                         }
                         Err(err) => {
                             log::error!("Failed to save device data to disk");
-                            let cloneable_err = Arc::new(err);
-                            Self::drain_requests(&mut self.validation_requests, || {
-                                Err(Error::ResponseFailure(cloneable_err.clone()))
-                            });
+                            Self::drain_requests(
+                                &mut self.validation_requests,
+                                || Err(err.clone()),
+                            );
                         }
                     }
                 } else {
@@ -845,10 +836,7 @@ impl AccountManager {
             }
             Err(err) => {
                 log::error!("Failed to validate device: {}", err);
-                let cloneable_err = Arc::new(err);
-                Self::drain_requests(&mut self.validation_requests, || {
-                    Err(Error::ResponseFailure(cloneable_err.clone()))
-                });
+                Self::drain_requests(&mut self.validation_requests, || Err(err.clone()));
             }
         }
 
@@ -936,13 +924,8 @@ impl AccountManager {
     }
 
     fn drain_device_requests_with_err(&mut self, err: Error) {
-        let cloneable_err = Arc::new(err);
-        Self::drain_requests(&mut self.rotation_requests, || {
-            Err(Error::ResponseFailure(cloneable_err.clone()))
-        });
-        Self::drain_requests(&mut self.validation_requests, || {
-            Err(Error::ResponseFailure(cloneable_err.clone()))
-        });
+        Self::drain_requests(&mut self.rotation_requests, || Err(err.clone()));
+        Self::drain_requests(&mut self.validation_requests, || Err(err.clone()));
     }
 
     fn drain_requests<T>(requests: &mut Vec<ResponseTx<T>>, result: impl Fn() -> Result<T, Error>) {
diff --git a/mullvad-daemon/src/management_interface.rs b/mullvad-daemon/src/management_interface.rs
index 61e4b025ba..b494c53fbb 100644
--- a/mullvad-daemon/src/management_interface.rs
+++ b/mullvad-daemon/src/management_interface.rs
@@ -1122,7 +1122,6 @@ fn map_device_error(error: &device::Error) -> Status {
             Status::new(Code::Unavailable, error.to_string())
         }
         device::Error::OtherRestError(error) => map_rest_error(error),
-        device::Error::ResponseFailure(error) => map_device_error(error.unpack()),
         _ => Status::new(Code::Unknown, error.to_string()),
     }
 }