Overhaul all unwraps/expects to use new logging interface

build.rs

@@ -5,7 +5,7 @@ extern crate winres;
 fn main() {
     let mut res = winres::WindowsResource::new();
     res.set_icon("static/favicon.ico");
-    res.compile().unwrap();
+    res.compile().expect("Failed to generate metadata");
 }
 
 #[cfg(not(windows))]

src/installer.rs

@@ -22,6 +22,8 @@ use tasks::install::InstallTask;
 use tasks::uninstall::UninstallTask;
 use tasks::DependencyTree;
 
+use logging::LoggingErrors;
+
 /// A message thrown during the installation of packages.
 #[derive(Serialize)]
 pub enum InstallMessage {
@@ -91,7 +93,7 @@ impl InstallerFramework {
             items,
             self.install_path
                 .clone()
-                .expect("Install directory not initialised")
+                .log_expect("Install directory not initialised")
         );
 
         // Calculate packages to *uninstall*

src/logging.rs

@@ -1,9 +1,10 @@
 //! Contains functions to help with logging.
 
-use fern;
 use chrono;
+use fern;
 use log;
 
+use std::fmt::Debug;
 use std::io;
 
 pub fn setup_logger() -> Result<(), fern::InitError> {
@@ -23,3 +24,42 @@ pub fn setup_logger() -> Result<(), fern::InitError> {
         .apply()?;
     Ok(())
 }
+
+/// An additional wrapper usable on Result/Option types to add logging to the regular
+/// panic route.
+pub trait LoggingErrors<T>
+where
+    Self: Sized,
+{
+    /// Unwraps this object. See `unwrap()`.
+    fn log_unwrap(self) -> T {
+        self.log_expect("Failed to unwrap")
+    }
+
+    /// Unwraps this object, with a specified error message on failure. See `expect()`.
+    fn log_expect(self, msg: &str) -> T;
+}
+
+impl<T, E: Debug> LoggingErrors<T> for Result<T, E> {
+    fn log_expect(self, msg: &str) -> T {
+        match self {
+            Ok(v) => v,
+            Err(v) => {
+                error!("Fatal error: {}: {:?}", msg, v);
+                panic!("Expectation failed");
+            }
+        }
+    }
+}
+
+impl<T> LoggingErrors<T> for Option<T> {
+    fn log_expect(self, msg: &str) -> T {
+        match self {
+            Some(v) => v,
+            None => {
+                error!("Fatal error: {}", msg);
+                panic!("Expectation failed");
+            }
+        }
+    }
+}

src/main.rs

@@ -38,10 +38,10 @@ mod assets;
 mod config;
 mod http;
 mod installer;
+mod logging;
 mod rest;
 mod sources;
 mod tasks;
-mod logging;
 
 use web_view::*;
 
@@ -60,6 +60,8 @@ use std::net::TcpListener;
 use std::sync::Arc;
 use std::sync::RwLock;
 
+use logging::LoggingErrors;
+
 // TODO: Fetch this over a HTTP request?
 static RAW_CONFIG: &'static str = include_str!("../config.toml");
 
@@ -71,18 +73,20 @@ enum CallbackType {
 fn main() {
     logging::setup_logger().expect("Unable to setup logging!");
 
-    let config = Config::from_toml_str(RAW_CONFIG).unwrap();
+    let config = Config::from_toml_str(RAW_CONFIG).log_expect("Config file could not be read");
 
     let app_name = config.general.name.clone();
 
     info!("{} installer", app_name);
 
-    let current_exe = std::env::current_exe().unwrap();
+    let current_exe = std::env::current_exe().log_expect("Current executable could not be found");
-    let current_path = current_exe.parent().unwrap();
+    let current_path = current_exe
+        .parent()
+        .log_expect("Parent directory of executable could not be found");
     let metadata_file = current_path.join("metadata.json");
     let framework = if metadata_file.exists() {
         info!("Using pre-existing metadata file: {:?}", metadata_file);
-        InstallerFramework::new_with_db(config, current_path).unwrap()
+        InstallerFramework::new_with_db(config, current_path).log_expect("Unable to parse metadata")
     } else {
         info!("Starting fresh install");
         InstallerFramework::new(config)
@@ -90,18 +94,18 @@ fn main() {
 
     // Firstly, allocate us an epidermal port
     let target_port = {
-        let listener =
+        let listener = TcpListener::bind("127.0.0.1:0")
-            TcpListener::bind("127.0.0.1:0").expect("At least one local address should be free");
+            .log_expect("At least one local address should be free");
         listener
             .local_addr()
-            .expect("Should be able to pull address from listener")
+            .log_expect("Should be able to pull address from listener")
             .port()
     };
 
     // Now, iterate over all ports
     let addresses = "localhost:0"
         .to_socket_addrs()
-        .expect("No localhost address found");
+        .log_expect("No localhost address found");
 
     let mut servers = Vec::new();
     let mut http_address = None;
@@ -112,12 +116,12 @@ fn main() {
     for mut address in addresses {
         address.set_port(target_port);
 
-        let server = WebServer::with_addr(framework.clone(), address).unwrap();
+        let server = WebServer::with_addr(framework.clone(), address.clone())
+            .log_expect("Failed to bind to address");
 
-        let addr = server.get_addr();
+        debug!("Server: {:?}", address);
-        debug!("Server: {:?}", addr);
 
-        http_address = Some(addr);
+        http_address = Some(address);
 
         servers.push(server);
     }
@@ -143,26 +147,27 @@ fn main() {
         |_| {},
         |wv, msg, _| {
             let command: CallbackType =
-                serde_json::from_str(msg).expect(&format!("Unable to parse string: {:?}", msg));
+                serde_json::from_str(msg).log_expect(&format!("Unable to parse string: {:?}", msg));
 
             debug!("Incoming payload: {:?}", command);
 
             match command {
                 CallbackType::SelectInstallDir { callback_name } => {
                     #[cfg(windows)]
-                    let result =
+                    let result = match nfd::open_pick_folder(None)
-                        match nfd::open_pick_folder(None).expect("Unable to open folder dialog") {
+                        .log_expect("Unable to open folder dialog")
-                            Response::Okay(v) => v,
+                    {
-                            _ => return,
+                        Response::Okay(v) => v,
-                        };
+                        _ => return,
+                    };
 
                     #[cfg(not(windows))]
                     let result =
                         wv.dialog(Dialog::ChooseDirectory, "Select a install directory...", "");
 
                     if result.len() > 0 {
-                        let result =
+                        let result = serde_json::to_string(&result)
-                            serde_json::to_string(&result).expect("Unable to serialize response");
+                            .log_expect("Unable to serialize response");
                         let command = format!("{}({});", callback_name, result);
                         debug!("Injecting response: {}", command);
                         wv.eval(&command);

src/rest.rs

@@ -29,6 +29,8 @@ use assets;
 use installer::InstallMessage;
 use installer::InstallerFramework;
 
+use logging::LoggingErrors;
+
 #[derive(Serialize)]
 struct FileSelection {
     path: Option<String>,
@@ -38,22 +40,14 @@ struct FileSelection {
 /// application.
 pub struct WebServer {
     _handle: JoinHandle<()>,
-    addr: SocketAddr,
 }
 
 impl WebServer {
-    /// Returns the bound address that the server is running from.
-    pub fn get_addr(&self) -> SocketAddr {
-        self.addr.clone()
-    }
-
     /// Creates a new web server with the specified address.
     pub fn with_addr(
         framework: Arc<RwLock<InstallerFramework>>,
         addr: SocketAddr,
     ) -> Result<Self, HyperError> {
-        let (sender, receiver) = channel();
-
         let handle = thread::spawn(move || {
             let server = Http::new()
                 .bind(&addr, move || {
@@ -61,19 +55,12 @@ impl WebServer {
                         framework: framework.clone(),
                     })
                 })
-                .unwrap();
+                .log_expect("Failed to bind to port");
 
-            sender.send(server.local_addr().unwrap()).unwrap();
+            server.run().log_expect("Failed to run HTTP server");
-
-            server.run().unwrap();
         });
 
-        let addr = receiver.recv().unwrap();
+        Ok(WebServer { _handle: handle })
-
-        Ok(WebServer {
-            _handle: handle,
-            addr,
-        })
     }
 }
 
@@ -94,10 +81,18 @@ impl Service for WebService {
             // This endpoint should be usable directly from a <script> tag during loading.
             // TODO: Handle errors
             (&Get, "/api/config") => {
-                let framework = self.framework.read().unwrap();
+                let framework = self
+                    .framework
+                    .read()
+                    .log_expect("InstallerFramework has been dirtied");
 
-                let file =
+                let file = enscapsulate_json(
-                    enscapsulate_json("config", &framework.get_config().to_json_str().unwrap());
+                    "config",
+                    &framework
+                        .get_config()
+                        .to_json_str()
+                        .log_expect("Failed to render JSON representation of config"),
+                );
 
                 Response::<hyper::Body>::new()
                     .with_header(ContentLength(file.len() as u64))
@@ -107,11 +102,15 @@ impl Service for WebService {
             // This endpoint should be usable directly from a <script> tag during loading.
             // TODO: Handle errors
             (&Get, "/api/packages") => {
-                let framework = self.framework.read().unwrap();
+                let framework = self
+                    .framework
+                    .read()
+                    .log_expect("InstallerFramework has been dirtied");
 
                 let file = enscapsulate_json(
                     "packages",
-                    &serde_json::to_string(&framework.database).unwrap(),
+                    &serde_json::to_string(&framework.database)
+                        .log_expect("Failed to render JSON representation of database"),
                 );
 
                 Response::<hyper::Body>::new()
@@ -121,12 +120,16 @@ impl Service for WebService {
             }
             // Returns the default path for a installation
             (&Get, "/api/default-path") => {
-                let framework = self.framework.read().unwrap();
+                let framework = self
+                    .framework
+                    .read()
+                    .log_expect("InstallerFramework has been dirtied");
                 let path = framework.get_default_path();
 
                 let response = FileSelection { path };
 
-                let file = serde_json::to_string(&response).unwrap();
+                let file = serde_json::to_string(&response)
+                    .log_expect("Failed to render JSON payload of default path object");
 
                 Response::<hyper::Body>::new()
                     .with_header(ContentLength(file.len() as u64))
@@ -139,11 +142,15 @@ impl Service for WebService {
             }
             // Gets properties such as if the application is in maintenance mode
             (&Get, "/api/installation-status") => {
-                let framework = self.framework.read().unwrap();
+                let framework = self
+                    .framework
+                    .read()
+                    .log_expect("InstallerFramework has been dirtied");
 
                 let response = framework.get_installation_status();
 
-                let file = serde_json::to_string(&response).unwrap();
+                let file = serde_json::to_string(&response)
+                    .log_expect("Failed to render JSON payload of installation status object");
 
                 Response::<hyper::Body>::new()
                     .with_header(ContentLength(file.len() as u64))
@@ -161,32 +168,51 @@ impl Service for WebService {
 
                     // Startup a thread to do this operation for us
                     thread::spawn(move || {
-                        let mut framework = framework.write().unwrap();
+                        let mut framework = framework
+                            .write()
+                            .log_expect("InstallerFramework has been dirtied");
 
                         match framework.uninstall(&sender) {
                             Err(v) => {
                                 error!("Uninstall error occurred: {:?}", v);
-                                sender.send(InstallMessage::Error(v)).unwrap();
+                                match sender.send(InstallMessage::Error(v)) {
-                            },
+                                    Err(v) => {
+                                        error!("Failed to send uninstall error: {:?}", v);
+                                    }
+                                    _ => {}
+                                };
+                            }
                             _ => {}
                         }
-                        sender.send(InstallMessage::EOF).unwrap();
+
+                        match sender.send(InstallMessage::EOF) {
+                            Err(v) => {
+                                error!("Failed to send EOF to client: {:?}", v);
+                            }
+                            _ => {}
+                        };
                     });
 
                     // Spawn a thread for transforming messages to chunk messages
                     thread::spawn(move || {
                         let mut tx = tx;
                         loop {
-                            let response = receiver.recv().unwrap();
+                            let response = receiver
+                                .recv()
+                                .log_expect("Failed to recieve message from runner thread");
 
                             match &response {
                                 &InstallMessage::EOF => break,
                                 _ => {}
                             }
 
-                            let mut response = serde_json::to_string(&response).unwrap();
+                            let mut response = serde_json::to_string(&response)
+                                .log_expect("Failed to render JSON logging response payload");
                             response.push('\n');
-                            tx = tx.send(Ok(response.into_bytes().into())).wait().unwrap();
+                            tx = tx
+                                .send(Ok(response.into_bytes().into()))
+                                .wait()
+                                .log_expect("Failed to write JSON response payload to client");
                         }
                     });
 
@@ -222,14 +248,18 @@ impl Service for WebService {
                     }
 
                     // The frontend always provides this
-                    let path = path.unwrap();
+                    let path = path.log_expect(
+                        "No path specified by frontend when one should have already existed",
+                    );
 
                     let (sender, receiver) = channel();
                     let (tx, rx) = hyper::Body::pair();
 
                     // Startup a thread to do this operation for us
                     thread::spawn(move || {
-                        let mut framework = framework.write().unwrap();
+                        let mut framework = framework
+                            .write()
+                            .log_expect("InstallerFramework has been dirtied");
 
                         let new_install = !framework.preexisting_install;
                         if new_install {
@@ -238,28 +268,45 @@ impl Service for WebService {
 
                         match framework.install(to_install, &sender, new_install) {
                             Err(v) => {
-                                error!("Install error occurred: {:?}", v);
+                                error!("Uninstall error occurred: {:?}", v);
-                                sender.send(InstallMessage::Error(v)).unwrap();
+                                match sender.send(InstallMessage::Error(v)) {
-                            },
+                                    Err(v) => {
+                                        error!("Failed to send uninstall error: {:?}", v);
+                                    }
+                                    _ => {}
+                                };
+                            }
                             _ => {}
                         }
-                        sender.send(InstallMessage::EOF).unwrap();
+
+                        match sender.send(InstallMessage::EOF) {
+                            Err(v) => {
+                                error!("Failed to send EOF to client: {:?}", v);
+                            }
+                            _ => {}
+                        };
                     });
 
                     // Spawn a thread for transforming messages to chunk messages
                     thread::spawn(move || {
                         let mut tx = tx;
                         loop {
-                            let response = receiver.recv().unwrap();
+                            let response = receiver
+                                .recv()
+                                .log_expect("Failed to recieve message from runner thread");
 
                             match &response {
                                 &InstallMessage::EOF => break,
                                 _ => {}
                             }
 
-                            let mut response = serde_json::to_string(&response).unwrap();
+                            let mut response = serde_json::to_string(&response)
+                                .log_expect("Failed to render JSON logging response payload");
                             response.push('\n');
-                            tx = tx.send(Ok(response.into_bytes().into())).wait().unwrap();
+                            tx = tx
+                                .send(Ok(response.into_bytes().into()))
+                                .wait()
+                                .log_expect("Failed to write JSON response payload to client");
                         }
                     });
 
@@ -281,7 +328,9 @@ impl Service for WebService {
 
                 match assets::file_from_string(&path) {
                     Some((content_type, file)) => {
-                        let content_type = ContentType(content_type.parse().unwrap());
+                        let content_type = ContentType(content_type.parse().log_expect(
+                            "Failed to parse content type into correct representation",
+                        ));
                         Response::<hyper::Body>::new()
                             .with_header(ContentLength(file.len() as u64))
                             .with_header(content_type)

src/tasks/download_pkg.rs

@@ -11,6 +11,8 @@ use http::stream_file;
 
 use number_prefix::{decimal_prefix, Prefixed, Standalone};
 
+use logging::LoggingErrors;
+
 pub struct DownloadPackageTask {
     pub name: String,
 }
@@ -24,7 +26,7 @@ impl Task for DownloadPackageTask {
     ) -> Result<TaskParamType, String> {
         assert_eq!(input.len(), 1);
 
-        let file = input.pop().expect("Should have input from resolver!");
+        let file = input.pop().log_expect("Should have input from resolver!");
         let (version, file) = match file {
             TaskParamType::File(v, f) => (v, f),
             _ => return Err(format!("Unexpected param type to download package")),

src/tasks/install_dir.rs

@@ -8,6 +8,8 @@ use tasks::TaskParamType;
 use std::fs::create_dir_all;
 use std::fs::read_dir;
 
+use logging::LoggingErrors;
+
 pub struct VerifyInstallDirTask {
     pub clean_install: bool,
 }
@@ -25,7 +27,7 @@ impl Task for VerifyInstallDirTask {
         let path = context
             .install_path
             .as_ref()
-            .expect("No install path specified");
+            .log_expect("No install path specified");
 
         if !path.exists() {
             create_dir_all(&path)

src/tasks/install_pkg.rs

@@ -18,6 +18,8 @@ use tasks::uninstall_pkg::UninstallPackageTask;
 
 use zip::ZipArchive;
 
+use logging::LoggingErrors;
+
 pub struct InstallPackageTask {
     pub name: String,
 }
@@ -34,7 +36,7 @@ impl Task for InstallPackageTask {
         let path = context
             .install_path
             .as_ref()
-            .expect("No install path specified");
+            .log_expect("No install path specified");
 
         let mut installed_files = Vec::new();
 
@@ -52,13 +54,16 @@ impl Task for InstallPackageTask {
         };
 
         // Check to see if no archive was available.
-        match input.pop().expect("Should have input from uninstaller!") {
+        match input
+            .pop()
+            .log_expect("Should have input from uninstaller!")
+        {
             // No file to install, but all is good.
             TaskParamType::Break => return Ok(TaskParamType::None),
             _ => {}
         }
 
-        let data = input.pop().expect("Should have input from resolver!");
+        let data = input.pop().log_expect("Should have input from resolver!");
         let (file, data) = match data {
             TaskParamType::FileContents(file, data) => (file, data),
             _ => return Err(format!("Unexpected param type to install package")),
@@ -74,7 +79,7 @@ impl Task for InstallPackageTask {
         let zip_size = zip.len();
 
         for i in 0..zip_size {
-            let mut file = zip.by_index(i).unwrap();
+            let mut file = zip.by_index(i).log_expect("Failed to iterate on .zip file");
 
             messenger(
                 &format!("Extracting {} ({} of {})", file.name(), i + 1, zip_size),

src/tasks/resolver.rs

@@ -11,6 +11,8 @@ use config::PackageDescription;
 
 use regex::Regex;
 
+use logging::LoggingErrors;
+
 pub struct ResolvePackageTask {
     pub name: String,
 }
@@ -76,7 +78,7 @@ impl Task for ResolvePackageTask {
             .into_iter()
             .filter(|x| regex.is_match(&x.name))
             .next()
-            .unwrap();
+            .log_expect("Searched file should have existed, but didn't");
 
         info!("Selected file: {:?}", latest_file);
 

src/tasks/save_executable.rs

@@ -12,6 +12,8 @@ use std::io::copy;
 
 use std::env::current_exe;
 
+use logging::LoggingErrors;
+
 pub struct SaveExecutableTask {}
 
 impl Task for SaveExecutableTask {
@@ -27,7 +29,7 @@ impl Task for SaveExecutableTask {
         let path = context
             .install_path
             .as_ref()
-            .expect("No install path specified");
+            .log_expect("No install path specified");
 
         let current_app = match current_exe() {
             Ok(v) => v,

src/tasks/uninstall_pkg.rs

@@ -10,6 +10,8 @@ use installer::LocalInstallation;
 use std::fs::remove_dir;
 use std::fs::remove_file;
 
+use logging::LoggingErrors;
+
 pub struct UninstallPackageTask {
     pub name: String,
     pub optional: bool,
@@ -27,7 +29,7 @@ impl Task for UninstallPackageTask {
         let path = context
             .install_path
             .as_ref()
-            .expect("No install path specified");
+            .log_expect("No install path specified");
 
         let mut metadata: Option<LocalInstallation> = None;
         for i in 0..context.database.len() {