improv: Use anyhow::Error instead of String for error types

Cargo.toml

@@ -5,6 +5,7 @@ authors = ["Jeremy Soller <[email protected]>"]
 edition = "2018"
 
 [dependencies]
+anyhow = "1"
 dbus = "0.8.4"
 libc = "0.2.76"
 clap = "2.33.3"

src/client.rs

@@ -1,4 +1,5 @@
-use crate::{err_str, Power, DBUS_IFACE, DBUS_NAME, DBUS_PATH};
+use anyhow::{Context, Error, Result};
+use crate::{Power, DBUS_IFACE, DBUS_NAME, DBUS_PATH};
 use clap::ArgMatches;
 use dbus::{arg::Append, ffidisp::Connection, Message};
 use pstate::PState;
@@ -12,81 +13,82 @@ pub struct PowerClient {
 }
 
 impl PowerClient {
-    pub fn new() -> Result<PowerClient, String> {
-        let bus = Connection::new_system().map_err(err_str)?;
+    pub fn new() -> Result<PowerClient> {
+        let bus = Connection::new_system()?;
         Ok(PowerClient { bus })
     }
 
     fn call_method<A: Append>(
         &mut self,
         method: &str,
         append: Option<A>,
-    ) -> Result<Message, String> {
-        let mut m = Message::new_method_call(DBUS_NAME, DBUS_PATH, DBUS_IFACE, method)?;
+    ) -> Result<Message> {
+        let mut m = Message::new_method_call(DBUS_NAME, DBUS_PATH, DBUS_IFACE, method).map_err(Error::msg)?;
         if let Some(arg) = append {
             m = m.append1(arg);
         }
 
         let r = self
             .bus
             .send_with_reply_and_block(m, TIMEOUT)
-            .map_err(|why| format!("daemon returned an error message: {}", err_str(why)))?;
+            .map_err(Error::new)
+            .context("daemon returned an error message")?;
 
         Ok(r)
     }
 
-    fn set_profile(&mut self, profile: &str) -> Result<(), String> {
+    fn set_profile(&mut self, profile: &str) -> Result<()> {
         println!("setting power profile to {}", profile);
         self.call_method::<bool>(profile, None)?;
         Ok(())
     }
 }
 
 impl Power for PowerClient {
-    fn performance(&mut self) -> Result<(), String> {
+    fn performance(&mut self) -> Result<()> {
         self.set_profile("Performance")
     }
 
-    fn balanced(&mut self) -> Result<(), String> {
+    fn balanced(&mut self) -> Result<()> {
         self.set_profile("Balanced")
     }
 
-    fn battery(&mut self) -> Result<(), String> {
+    fn battery(&mut self) -> Result<()> {
         self.set_profile("Battery")
     }
 
-    fn get_graphics(&mut self) -> Result<String, String> {
+    fn get_graphics(&mut self) -> Result<String> {
         let r = self.call_method::<bool>("GetGraphics", None)?;
-        r.get1().ok_or_else(|| "return value not found".to_string())
+        r.get1().ok_or_else(|| Error::msg("return value not found"))
     }
 
-    fn get_profile(&mut self) -> Result<String, String> {
-        let m = Message::new_method_call(DBUS_NAME, DBUS_PATH, DBUS_IFACE, "GetProfile")?;
-        let r = self.bus.send_with_reply_and_block(m, TIMEOUT).map_err(err_str)?;
-        r.get1().ok_or_else(|| "return value not found".to_string())
+    fn get_profile(&mut self) -> Result<String> {
+        let m = Message::new_method_call(DBUS_NAME, DBUS_PATH, DBUS_IFACE, "GetProfile").map_err(Error::msg)?;
+        let r = self.bus.send_with_reply_and_block(m, TIMEOUT)?;
+        r.get1().ok_or_else(|| Error::msg("return value not found"))
     }
 
-    fn get_switchable(&mut self) -> Result<bool, String> {
+    fn get_switchable(&mut self) -> Result<bool> {
         let r = self.call_method::<bool>("GetSwitchable", None)?;
-        r.get1().ok_or_else(|| "return value not found".to_string())
+        r.get1().ok_or_else(|| Error::msg("return value not found"))
     }
 
-    fn set_graphics(&mut self, vendor: &str) -> Result<(), String> {
+    fn set_graphics(&mut self, vendor: &str) -> Result<()> {
         println!("setting graphics to {}", vendor);
         self.call_method::<&str>("SetGraphics", Some(vendor)).map(|_| ())
     }
 
-    fn get_graphics_power(&mut self) -> Result<bool, String> {
+    fn get_graphics_power(&mut self) -> Result<bool> {
         let r = self.call_method::<bool>("GetGraphicsPower", None)?;
-        r.get1().ok_or_else(|| "return value not found".to_string())
+        r.get1().ok_or_else(|| Error::msg("return value not found"))
     }
 
-    fn set_graphics_power(&mut self, power: bool) -> Result<(), String> {
+    fn set_graphics_power(&mut self, power: bool) -> Result<()> {
         println!("turning discrete graphics {}", if power { "on" } else { "off " });
         self.call_method::<bool>("SetGraphicsPower", Some(power)).map(|_| ())
     }
 
-    fn auto_graphics_power(&mut self) -> Result<(), String> {
+    fn auto_graphics_power(&mut self) -> Result<()> {
         println!("setting discrete graphics to turn off when not in use");
         self.call_method::<bool>("AutoGraphicsPower", None).map(|_| ())
     }
@@ -133,15 +135,15 @@ fn profile(client: &mut PowerClient) -> io::Result<()> {
     Ok(())
 }
 
-pub fn client(subcommand: &str, matches: &ArgMatches) -> Result<(), String> {
+pub fn client(subcommand: &str, matches: &ArgMatches) -> Result<()> {
     let mut client = PowerClient::new()?;
 
     match subcommand {
         "profile" => match matches.value_of("profile") {
             Some("balanced") => client.balanced(),
             Some("battery") => client.battery(),
             Some("performance") => client.performance(),
-            _ => profile(&mut client).map_err(err_str),
+            _ => profile(&mut client).map_err(Error::new),
         },
         "graphics" => match matches.subcommand() {
             ("compute", _) => client.set_graphics("compute"),
@@ -174,6 +176,6 @@ pub fn client(subcommand: &str, matches: &ArgMatches) -> Result<(), String> {
                 Ok(())
             }
         },
-        _ => Err(format!("unknown sub-command {}", subcommand)),
+        _ => Err(Error::msg(format!("unknown sub-command {}", subcommand))),
     }
 }

src/daemon/mod.rs

@@ -1,3 +1,5 @@
+use anyhow::{Error, Result};
+
 use dbus::{
     ffidisp::{Connection, NameFlag},
     tree::{Factory, MethodErr, Signal},
@@ -14,7 +16,6 @@ use std::{
 };
 
 use crate::{
-    err_str,
     errors::ProfileError,
     fan::FanDaemon,
     graphics::Graphics,
@@ -67,8 +68,8 @@ impl PowerDaemon {
     fn new(
         power_switch_signal: Arc<Signal<()>>,
         dbus_connection: Arc<Connection>,
-    ) -> Result<PowerDaemon, String> {
-        let graphics = Graphics::new().map_err(err_str)?;
+    ) -> Result<PowerDaemon> {
+        let graphics = Graphics::new()?;
         Ok(PowerDaemon {
             initial_set: false,
             graphics,
@@ -83,7 +84,7 @@ impl PowerDaemon {
         &mut self,
         func: fn(&mut Vec<ProfileError>, bool),
         name: &str,
-    ) -> Result<(), String> {
+    ) -> Result<()> {
         if &self.power_profile == name {
             info!("profile was already set");
             return Ok(())
@@ -108,50 +109,50 @@ impl PowerDaemon {
                 error_message = format!("{}\n    - {}", error_message, error);
             }
 
-            Err(error_message)
+            Err(Error::msg(error_message))
         }
     }
 }
 
 impl Power for PowerDaemon {
-    fn battery(&mut self) -> Result<(), String> {
-        self.apply_profile(battery, "Battery").map_err(err_str)
+    fn battery(&mut self) -> Result<()> {
+        self.apply_profile(battery, "Battery")
     }
 
-    fn balanced(&mut self) -> Result<(), String> {
-        self.apply_profile(balanced, "Balanced").map_err(err_str)
+    fn balanced(&mut self) -> Result<()> {
+        self.apply_profile(balanced, "Balanced")
     }
 
-    fn performance(&mut self) -> Result<(), String> {
-        self.apply_profile(performance, "Performance").map_err(err_str)
+    fn performance(&mut self) -> Result<()> {
+        self.apply_profile(performance, "Performance")
     }
 
-    fn get_graphics(&mut self) -> Result<String, String> {
-        self.graphics.get_vendor().map_err(err_str)
+    fn get_graphics(&mut self) -> Result<String> {
+        self.graphics.get_vendor().map_err(Error::new)
     }
 
-    fn get_profile(&mut self) -> Result<String, String> { Ok(self.power_profile.clone()) }
+    fn get_profile(&mut self) -> Result<String> { Ok(self.power_profile.clone()) }
 
-    fn get_switchable(&mut self) -> Result<bool, String> { Ok(self.graphics.can_switch()) }
+    fn get_switchable(&mut self) -> Result<bool> { Ok(self.graphics.can_switch()) }
 
-    fn set_graphics(&mut self, vendor: &str) -> Result<(), String> {
-        self.graphics.set_vendor(vendor).map_err(err_str)
+    fn set_graphics(&mut self, vendor: &str) -> Result<()> {
+        self.graphics.set_vendor(vendor).map_err(Error::new)
     }
 
-    fn get_graphics_power(&mut self) -> Result<bool, String> {
-        self.graphics.get_power().map_err(err_str)
+    fn get_graphics_power(&mut self) -> Result<bool> {
+        self.graphics.get_power().map_err(Error::new)
     }
 
-    fn set_graphics_power(&mut self, power: bool) -> Result<(), String> {
-        self.graphics.set_power(power).map_err(err_str)
+    fn set_graphics_power(&mut self, power: bool) -> Result<()> {
+        self.graphics.set_power(power).map_err(Error::new)
     }
 
-    fn auto_graphics_power(&mut self) -> Result<(), String> {
-        self.graphics.auto_power().map_err(err_str)
+    fn auto_graphics_power(&mut self) -> Result<()> {
+        self.graphics.auto_power().map_err(Error::new)
     }
 }
 
-pub fn daemon() -> Result<(), String> {
+pub fn daemon() -> Result<()> {
     signal_handling();
     let pci_runtime_pm = std::env::var("S76_POWER_PCI_RUNTIME_PM").ok().map_or(false, |v| v == "1");
 
@@ -162,7 +163,7 @@ pub fn daemon() -> Result<(), String> {
     PCI_RUNTIME_PM.store(pci_runtime_pm, Ordering::SeqCst);
 
     info!("Connecting to dbus system bus");
-    let c = Arc::new(Connection::new_system().map_err(err_str)?);
+    let c = Arc::new(Connection::new_system()?);
 
     let f = Factory::new_fn::<()>();
     let hotplug_signal = Arc::new(f.signal("HotPlugDetect", ()).sarg::<u64, _>("port"));
@@ -202,7 +203,7 @@ pub fn daemon() -> Result<(), String> {
     }
 
     info!("Registering dbus name {}", DBUS_NAME);
-    c.register_name(DBUS_NAME, NameFlag::ReplaceExisting as u32).map_err(err_str)?;
+    c.register_name(DBUS_NAME, NameFlag::ReplaceExisting as u32)?;
 
     // Defines whether the value returned by the method should be appended.
     macro_rules! append {
@@ -278,7 +279,7 @@ pub fn daemon() -> Result<(), String> {
         ),
     );
 
-    tree.set_registered(&c, true).map_err(err_str)?;
+    tree.set_registered(&c, true)?;
 
     c.add_handler(tree);
 
@@ -312,7 +313,7 @@ pub fn daemon() -> Result<(), String> {
             if hpd[i] != last[i] && hpd[i] {
                 info!("HotPlugDetect {}", i);
                 c.send(hotplug_signal.msg(&DBUS_PATH.into(), &DBUS_NAME.into()).append1(i as u64))
-                    .map_err(|()| "failed to send message".to_string())?;
+                    .map_err(|_| Error::msg("failed to send message"))?;
             }
         }
 

src/lib.rs

@@ -4,6 +4,8 @@ extern crate intel_pstate as pstate;
 #[macro_use]
 extern crate log;
 
+use anyhow::Result;
+
 pub mod client;
 pub mod daemon;
 pub mod disks;
@@ -31,17 +33,14 @@ pub static DBUS_PATH: &'static str = "/com/system76/PowerDaemon";
 pub static DBUS_IFACE: &'static str = "com.system76.PowerDaemon";
 
 pub trait Power {
-    fn performance(&mut self) -> Result<(), String>;
-    fn balanced(&mut self) -> Result<(), String>;
-    fn battery(&mut self) -> Result<(), String>;
-    fn get_graphics(&mut self) -> Result<String, String>;
-    fn get_profile(&mut self) -> Result<String, String>;
-    fn get_switchable(&mut self) -> Result<bool, String>;
-    fn set_graphics(&mut self, vendor: &str) -> Result<(), String>;
-    fn get_graphics_power(&mut self) -> Result<bool, String>;
-    fn set_graphics_power(&mut self, power: bool) -> Result<(), String>;
-    fn auto_graphics_power(&mut self) -> Result<(), String>;
-}
-
-// Helper function for errors
-pub(crate) fn err_str<E: ::std::fmt::Display>(err: E) -> String { format!("{}", err) }
+    fn performance(&mut self) -> Result<()>;
+    fn balanced(&mut self) -> Result<()>;
+    fn battery(&mut self) -> Result<()>;
+    fn get_graphics(&mut self) -> Result<String>;
+    fn get_profile(&mut self) -> Result<String>;
+    fn get_switchable(&mut self) -> Result<bool>;
+    fn set_graphics(&mut self, vendor: &str) -> Result<()>;
+    fn get_graphics_power(&mut self) -> Result<bool>;
+    fn set_graphics_power(&mut self, power: bool) -> Result<()>;
+    fn auto_graphics_power(&mut self) -> Result<()>;
+}

src/main.rs

@@ -1,3 +1,4 @@
+use anyhow::Error;
 use clap::{App, AppSettings, Arg, SubCommand};
 use log::LevelFilter;
 use std::process;
@@ -101,7 +102,7 @@ fn main() {
             if unsafe { libc::geteuid() } == 0 {
                 daemon::daemon()
             } else {
-                Err("must be run as root".to_string())
+                Err(Error::msg("must be run as root"))
             }
         }
         (subcommand, Some(matches)) => client::client(subcommand, matches),