Merge pull request #42 from mum-rs/handle-panics

3 files changed, 77 insertions, 50 deletions

diff --git a/mumlib/Cargo.toml b/mumlib/Cargo.toml
index 240e017..43bd8c5 100644
--- a/mumlib/Cargo.toml
+++ b/mumlib/Cargo.toml
@@ -12,6 +12,7 @@ license = "MIT"
 
 [dependencies]
 colored = "2.0"
+dirs = "3"
 fern = "0.6"
 log = "0.4"
 serde = { version = "1.0", features = ["derive"] }
diff --git a/mumlib/src/config.rs b/mumlib/src/config.rs
index 0a43253..9394b85 100644
--- a/mumlib/src/config.rs
+++ b/mumlib/src/config.rs
@@ -1,9 +1,12 @@
+use crate::error::ConfigError;
 use crate::DEFAULT_PORT;
+
+use log::*;
 use serde::{Deserialize, Serialize};
 use std::convert::TryFrom;
 use std::fs;
 use std::net::{SocketAddr, ToSocketAddrs};
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use toml::value::Array;
 use toml::Value;
 
@@ -20,13 +23,9 @@ pub struct Config {
 }
 
 impl Config {
-    pub fn write_default_cfg(&self, create: bool) -> Result<(), std::io::Error> {
-        let path = if create {
-            get_creatable_cfg_path()
-        } else {
-            get_cfg_path()
-        };
-        let path = std::path::Path::new(&path);
+    pub fn write_default_cfg(&self, create: bool) -> Result<(), ConfigError> {
+        let path = default_cfg_path();
+
         // Possible race here. It's fine since it shows when:
         //   1) the file doesn't exist when checked and is then created
         //   2) the file exists when checked but is then removed
@@ -35,14 +34,15 @@ impl Config {
         // should work. Unless the file is removed AND the permissions
         // change, but then we don't have permissions so we can't
         // do anything anyways.
+
         if !create && !path.exists() {
-            return Ok(());
+            return Err(ConfigError::WontCreateFile);
         }
 
-        fs::write(
-            path,
-            toml::to_string(&TOMLConfig::from(self.clone())).unwrap(),
-        )
+        Ok(fs::write(
+            &path,
+            toml::to_string(&TOMLConfig::from(self.clone()))?,
+        )?)
     }
 }
 
@@ -80,36 +80,15 @@ impl ServerConfig {
     }
 }
 
-pub fn get_cfg_path() -> String {
-    if let Ok(var) = std::env::var("XDG_CONFIG_HOME") {
-        let path = format!("{}/mumdrc", var);
-        if Path::new(&path).exists() {
-            return path;
-        }
-    } else if let Ok(var) = std::env::var("HOME") {
-        let path = format!("{}/.config/mumdrc", var);
-        if Path::new(&path).exists() {
-            return path;
+pub fn default_cfg_path() -> PathBuf {
+    match dirs::config_dir() {
+        Some(mut p) => {
+            p.push("mumdrc");
+            p
         }
+        //TODO This isn't cross platform.
+        None => PathBuf::from("/etc/mumdrc")
     }
-
-    "/etc/mumdrc".to_string()
-}
-
-pub fn get_creatable_cfg_path() -> String {
-    if let Ok(var) = std::env::var("XDG_CONFIG_HOME") {
-        let path = format!("{}/mumdrc", var);
-        if !Path::new(&path).exists() {
-            return path;
-        }
-    } else if let Ok(var) = std::env::var("HOME") {
-        let path = format!("{}/.config/mumdrc", var);
-        if !Path::new(&path).exists() {
-            return path;
-        }
-    }
-
-    "/etc/mumdrc".to_string()
 }
 
 pub fn cfg_exists() -> bool {
@@ -162,6 +141,7 @@ impl From<Config> for TOMLConfig {
                 config
                     .servers
                     .into_iter()
+                    // Safe since all ServerConfigs are valid TOML
                     .map(|s| Value::try_from::<ServerConfig>(s).unwrap())
                     .collect(),
             ),
@@ -169,13 +149,20 @@ impl From<Config> for TOMLConfig {
     }
 }
 
-pub fn read_default_cfg() -> Config {
-    Config::try_from(
-        toml::from_str::<TOMLConfig>(&match fs::read_to_string(get_cfg_path()) {
-            Ok(f) => f,
-            Err(_) => return Config::default(),
-        })
-        .expect("invalid TOML in config file"), //TODO
-    )
-    .expect("invalid config in TOML") //TODO
+pub fn read_default_cfg() -> Result<Config, ConfigError> {
+    let path = default_cfg_path();
+    match fs::read_to_string(&path) {
+        Ok(s) => {
+            let toml_config: TOMLConfig = toml::from_str(&s)?;
+            Ok(Config::try_from(toml_config)?)
+        },
+        Err(e) => {
+            if matches!(e.kind(), std::io::ErrorKind::NotFound) && !path.exists() {
+                warn!("Config file not found");
+            } else {
+                error!("Error reading config file: {}", e);
+            }
+            return Ok(Config::default());
+        }
+    }
 }
diff --git a/mumlib/src/error.rs b/mumlib/src/error.rs
index f6a02a7..6b7dccd 100644
--- a/mumlib/src/error.rs
+++ b/mumlib/src/error.rs
@@ -42,3 +42,42 @@ impl fmt::Display for ChannelIdentifierError {
         }
     }
 }
+
+pub enum ConfigError {
+    InvalidConfig,
+    TOMLErrorSer(toml::ser::Error),
+    TOMLErrorDe(toml::de::Error),
+
+    WontCreateFile,
+    IOError(std::io::Error),
+}
+
+impl fmt::Display for ConfigError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            ConfigError::InvalidConfig => write!(f, "Invalid configuration"),
+            ConfigError::TOMLErrorSer(e) => write!(f, "Invalid TOML when serializing: {}", e),
+            ConfigError::TOMLErrorDe(e) => write!(f, "Invalid TOML when deserializing: {}", e),
+            ConfigError::WontCreateFile => write!(f, "File does not exist but caller didn't allow creation"),
+            ConfigError::IOError(e) => write!(f, "IO error: {}", e),
+        }
+    }
+}
+
+impl From<std::io::Error> for ConfigError {
+    fn from(e: std::io::Error) -> Self {
+        ConfigError::IOError(e)
+    }
+}
+
+impl From<toml::ser::Error> for ConfigError {
+    fn from(e: toml::ser::Error) -> Self {
+        ConfigError::TOMLErrorSer(e)
+    }
+}
+
+impl From<toml::de::Error> for ConfigError {
+    fn from(e: toml::de::Error) -> Self {
+        ConfigError::TOMLErrorDe(e)
+    }
+}