Fix error handling in system_service

3 files changed, 43 insertions, 27 deletions

diff --git a/mullvad-daemon/src/cli.rs b/mullvad-daemon/src/cli.rs
index 722a176878..d39a80309e 100644
--- a/mullvad-daemon/src/cli.rs
+++ b/mullvad-daemon/src/cli.rs
@@ -11,7 +11,14 @@ pub struct Config {
     pub register_service: bool,
 }
 
-pub fn get_config() -> Config {
+pub fn get_config() -> &'static Config {
+    lazy_static::lazy_static! {
+        static ref CONFIG: Config = create_config();
+    }
+    &*CONFIG
+}
+
+pub fn create_config() -> Config {
     let app = create_app();
     let matches = app.get_matches();
 
diff --git a/mullvad-daemon/src/main.rs b/mullvad-daemon/src/main.rs
index 71cd61ff31..e9346405d9 100644
--- a/mullvad-daemon/src/main.rs
+++ b/mullvad-daemon/src/main.rs
@@ -24,11 +24,11 @@ const DAEMON_LOG_FILENAME: &str = "daemon.log";
 
 fn main() {
     let config = cli::get_config();
-    let log_dir = init_logging(&config).unwrap_or_else(|error| {
+    let log_dir = init_logging(config).unwrap_or_else(|error| {
         eprintln!("{}", error);
         std::process::exit(1)
     });
-    let exit_code = match run_platform(&config, log_dir) {
+    let exit_code = match run_platform(config, log_dir) {
         Ok(_) => 0,
         Err(error) => {
             error!("{}", error);
@@ -40,14 +40,7 @@ fn main() {
 }
 
 fn init_logging(config: &cli::Config) -> Result<Option<PathBuf>, String> {
-    let log_dir = if config.log_to_file {
-        Some(
-            mullvad_paths::log_dir()
-                .map_err(|e| e.display_chain_with_msg("Unable to get log directory"))?,
-        )
-    } else {
-        None
-    };
+    let log_dir = get_log_dir(config)?;
     let log_file = log_dir.as_ref().map(|dir| dir.join(DAEMON_LOG_FILENAME));
 
     logging::init_logger(
@@ -64,14 +57,23 @@ fn init_logging(config: &cli::Config) -> Result<Option<PathBuf>, String> {
     Ok(log_dir)
 }
 
+fn get_log_dir(config: &cli::Config) -> Result<Option<PathBuf>, String> {
+    if config.log_to_file {
+        Ok(Some(mullvad_paths::log_dir().map_err(|e| {
+            e.display_chain_with_msg("Unable to get log directory")
+        })?))
+    } else {
+        Ok(None)
+    }
+}
+
 #[cfg(windows)]
 fn run_platform(config: &cli::Config, log_dir: Option<PathBuf>) -> Result<(), String> {
     if config.run_as_service {
         system_service::run()
     } else {
         if config.register_service {
-            let install_result =
-                system_service::install_service().chain_err(|| "Unable to install the service");
+            let install_result = system_service::install_service().map_err(|e| e.display_chain());
             if install_result.is_ok() {
                 println!("Installed the service.");
             }
diff --git a/mullvad-daemon/src/system_service.rs b/mullvad-daemon/src/system_service.rs
index c4b98d88fc..b8d4245e60 100644
--- a/mullvad-daemon/src/system_service.rs
+++ b/mullvad-daemon/src/system_service.rs
@@ -1,5 +1,5 @@
 use crate::cli;
-use error_chain::ChainedError;
+use mullvad_daemon::DaemonShutdownHandle;
 use std::{
     env,
     ffi::OsString,
@@ -10,6 +10,7 @@ use std::{
     thread,
     time::Duration,
 };
+use talpid_types::ErrorExt;
 use windows_service::{
     service::{
         ServiceAccess, ServiceControl, ServiceControlAccept, ServiceDependency,
@@ -21,19 +22,16 @@ use windows_service::{
     service_manager::{ServiceManager, ServiceManagerAccess},
 };
 
-use super::{Result, ResultExt};
-use mullvad_daemon::DaemonShutdownHandle;
-
 static SERVICE_NAME: &'static str = "MullvadVPN";
 static SERVICE_DISPLAY_NAME: &'static str = "Mullvad VPN Service";
 static SERVICE_TYPE: ServiceType = ServiceType::OWN_PROCESS;
 
-pub fn run() -> Result<()> {
+pub fn run() -> Result<(), String> {
     // Start the service dispatcher.
     // This will block current thread until the service stopped and spawn `service_main` on a
     // background thread.
     service_dispatcher::start(SERVICE_NAME, service_main)
-        .chain_err(|| "Failed to start a service dispatcher")
+        .map_err(|e| e.display_chain_with_msg("Failed to start a service dispatcher"))
 }
 
 windows_service::define_windows_service!(service_main, handle_service_main);
@@ -46,7 +44,7 @@ pub fn handle_service_main(_arguments: Vec<OsString>) {
     };
 }
 
-fn run_service() -> Result<()> {
+fn run_service() -> Result<(), String> {
     let (event_tx, event_rx) = mpsc::channel();
 
     // Register service event handler
@@ -65,14 +63,14 @@ fn run_service() -> Result<()> {
         }
     };
     let status_handle = service_control_handler::register(SERVICE_NAME, event_handler)
-        .chain_err(|| "Failed to register a service control handler")?;
+        .map_err(|e| e.display_chain_with_msg("Failed to register a service control handler"))?;
     let mut persistent_service_status = PersistentServiceStatus::new(status_handle);
     persistent_service_status
         .set_pending_start(Duration::from_secs(1))
         .unwrap();
 
-    let config = cli::get_config();
-    let result = crate::create_daemon(&config).and_then(|daemon| {
+    let log_dir = crate::get_log_dir(cli::get_config()).expect("Log dir should be available here");
+    let result = crate::create_daemon(log_dir).and_then(|daemon| {
         let shutdown_handle = daemon.shutdown_handle();
 
         // Register monitor that translates `ServiceControl` to Daemon events
@@ -90,7 +88,7 @@ fn run_service() -> Result<()> {
 
     persistent_service_status.set_stopped(exit_code).unwrap();
 
-    result
+    result.map_err(|e| e.display_chain())
 }
 
 /// Start event monitor thread that polls for `ServiceControl` and translates them into calls to
@@ -216,14 +214,23 @@ fn accepted_controls_by_state(state: ServiceState) -> ServiceControlAccept {
     }
 }
 
-pub fn install_service() -> Result<()> {
+#[derive(err_derive::Error, Debug)]
+pub enum InstallError {
+    #[error(display = "Unable to connect to service manager")]
+    ConnectServiceManager(#[error(cause)] windows_service::Error),
+
+    #[error(display = "Unable to create a service")]
+    CreateService(#[error(cause)] windows_service::Error),
+}
+
+pub fn install_service() -> Result<(), InstallError> {
     let manager_access = ServiceManagerAccess::CONNECT | ServiceManagerAccess::CREATE_SERVICE;
     let service_manager = ServiceManager::local_computer(None::<&str>, manager_access)
-        .chain_err(|| "Unable to connect to service manager")?;
+        .map_err(InstallError::ConnectServiceManager)?;
     service_manager
         .create_service(get_service_info()?, ServiceAccess::empty())
         .map(|_| ())
-        .chain_err(|| "Unable to create a service")
+        .map_err(InstallError::CreateService)
 }
 
 fn get_service_info() -> Result<ServiceInfo> {