Add basic error handling

Cargo.toml

@@ -52,6 +52,7 @@ wildmatch = "2.1.1"
 clap = { version = "4.4.7", features = ["derive"] }
 cli-table = "0.4.7"
 textwrap = "0.16.0"
+log = "0.4.20"
 
 [dev-dependencies]
 criterion = { version = "0.5", features = ["html_reports"] }

src/bin/config-store.rs

@@ -1,9 +1,11 @@
+use anyhow::anyhow;
 use clap::{Parser, Subcommand};
 use derive_more::Display;
 use gosub_engine::config::settings::Setting;
 use gosub_engine::config::storage::json_storage::JsonStorageAdapter;
 use gosub_engine::config::storage::sqlite_storage::SqliteStorageAdapter;
 use gosub_engine::config::{ConfigStore, Store};
+use std::str::FromStr;
 
 #[derive(Debug, Parser)]
 #[clap(name = "Config-Store", version = "0.1.0", author = "Gosub")]
@@ -45,13 +47,13 @@ enum Engine {
 }
 
 impl std::str::FromStr for Engine {
-    type Err = ();
+    type Err = anyhow::Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s {
             "sqlite" => Ok(Engine::Sqlite),
             "json" => Ok(Engine::Json),
-            _ => Err(()),
+            _ => Err(anyhow!("problem reading config")),
         }
     }
 }
@@ -69,25 +71,25 @@ struct GlobalOpts {
     path: String,
 }
 
-fn main() {
+fn main() -> anyhow::Result<()> {
     let args = Cli::parse();
 
     let storage_box: Box<dyn Store> = match args.global_opts.engine {
-        Engine::Sqlite => Box::new(SqliteStorageAdapter::new(args.global_opts.path.as_str())),
-        Engine::Json => Box::new(JsonStorageAdapter::new(args.global_opts.path.as_str())),
+        Engine::Sqlite => Box::new(SqliteStorageAdapter::try_from(&args.global_opts.path)?),
+        Engine::Json => Box::new(JsonStorageAdapter::try_from(&args.global_opts.path)?),
     };
 
-    let mut store = ConfigStore::new(storage_box, true);
+    let mut store = ConfigStore::from_storage(storage_box, true)?;
 
     match args.command {
         Commands::View { key } => {
             if !store.has(&key) {
                 println!("Key not found");
-                return;
+                return Ok(());
             }
 
-            let info = store.get_info(key.as_str()).unwrap();
-            let value = store.get(key.as_str());
+            let info = store.get_info(&key).unwrap();
+            let value = store.get(&key);
 
             println!("Key            : {}", key);
             println!("Current Value  : {}", value);
@@ -96,18 +98,20 @@ fn main() {
         }
         Commands::List => {
             for key in store.find("*") {
-                let value = store.get(key.as_str());
+                let value = store.get(&key);
                 println!("{:40}: {}", key, value);
             }
         }
         Commands::Set { key, value } => {
-            store.set(&key, Setting::from_string(&value).expect("incorrect value"));
+            store.set(&key, Setting::from_str(&value).expect("incorrect value"));
         }
         Commands::Search { key } => {
-            for key in store.find(key.as_str()) {
+            for key in store.find(&key) {
                 let value = store.get(&key);
                 println!("{:40}: {}", key, value);
             }
         }
     }
+
+    Ok(())
 }

src/config.rs

@@ -2,10 +2,12 @@ pub mod settings;
 pub mod storage;
 
 use crate::config::settings::{Setting, SettingInfo};
+use crate::types::Result;
 use serde_derive::Deserialize;
 use serde_json::Value;
 use std::collections::HashMap;
 use std::mem;
+use std::str::FromStr;
 use wildmatch::WildMatch;
 
 const SETTINGS_JSON: &str = include_str!("./config/settings.json");
@@ -29,7 +31,7 @@ pub trait Store {
     /// Retrieves all the settings in the storage in one go. This is used for preloading the settings
     /// into the ConfigStore and is more performant normally than calling get_setting manually for each
     /// setting.
-    fn get_all_settings(&self) -> HashMap<String, Setting>;
+    fn get_all_settings(&self) -> Result<HashMap<String, Setting>>;
 }
 
 /// Configuration store is the place where the gosub engine can find all configurable options
@@ -46,7 +48,7 @@ pub struct ConfigStore {
 
 impl ConfigStore {
     /// Creates a new store with the given storage adapter and preloads the store if needed
-    pub fn new(storage: Box<dyn Store>, preload: bool) -> Self {
+    pub fn from_storage(storage: Box<dyn Store>, preload: bool) -> Result<Self> {
         let mut store = ConfigStore {
             settings: HashMap::new(),
             settings_info: HashMap::new(),
@@ -55,17 +57,17 @@ impl ConfigStore {
         };
 
         // Populate the settings from the json file
-        store.populate_settings();
+        store.populate_settings()?;
 
         // preload the settings if requested
         if preload {
-            let all_settings = store.storage.get_all_settings();
+            let all_settings = store.storage.get_all_settings()?;
             for (key, value) in all_settings {
                 store.settings.insert(key, value);
             }
         }
 
-        store
+        Ok(store)
     }
 
     /// Returns true when the store knows about the given key
@@ -134,7 +136,7 @@ impl ConfigStore {
     }
 
     /// Populates the settings in the store from the settings.json file
-    fn populate_settings(&mut self) {
+    fn populate_settings(&mut self) -> Result<()> {
         let json_data: Value =
             serde_json::from_str(SETTINGS_JSON).expect("Failed to parse settings.json");
 
@@ -150,8 +152,7 @@ impl ConfigStore {
                     let info = SettingInfo {
                         key: key.clone(),
                         description: entry.description,
-                        default: Setting::from_string(entry.default.as_str())
-                            .expect("cannot parse default setting"),
+                        default: Setting::from_str(&entry.default)?,
                         last_accessed: 0,
                     };
 
@@ -161,6 +162,8 @@ impl ConfigStore {
                 }
             }
         }
+
+        Ok(())
     }
 }
 
@@ -171,7 +174,8 @@ mod test {
 
     #[test]
     fn config_store() {
-        let mut store = ConfigStore::new(Box::new(MemoryStorageAdapter::new()), true);
+        let mut store =
+            ConfigStore::from_storage(Box::new(MemoryStorageAdapter::new()), true).unwrap();
         let setting = store.get("dns.local_resolver.enabled");
         assert_eq!(setting, Setting::Bool(false));
 
@@ -183,7 +187,8 @@ mod test {
     #[test]
     #[should_panic]
     fn invalid_setting() {
-        let mut store = ConfigStore::new(Box::new(MemoryStorageAdapter::new()), true);
+        let mut store =
+            ConfigStore::from_storage(Box::new(MemoryStorageAdapter::new()), true).unwrap();
         store.set(
             "dns.local_resolver.enabled",
             Setting::String("wont accept strings".into()),

src/config/settings.rs

@@ -1,5 +1,7 @@
+use crate::types::{Error, Result};
 use core::fmt::Display;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
+use std::str::FromStr;
 
 /// A setting can be either a signed integer, unsigned integer, string, map or boolean.
 /// Maps could be created by using comma separated strings maybe
@@ -13,7 +15,7 @@ pub enum Setting {
 }
 
 impl Serialize for Setting {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
     where
         S: Serializer,
     {
@@ -23,16 +25,13 @@ impl Serialize for Setting {
 }
 
 impl<'de> Deserialize<'de> for Setting {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
     where
         D: Deserializer<'de>,
     {
         let value = String::deserialize(deserializer)?;
-
-        match Setting::from_string(value.as_str()) {
-            None => Err(serde::de::Error::custom("Cannot deserialize")),
-            Some(setting) => Ok(setting),
-        }
+        Setting::from_str(&value)
+            .map_err(|err| serde::de::Error::custom(format!("cannot deserialize: {err}")))
     }
 }
 
@@ -56,7 +55,9 @@ impl Display for Setting {
     }
 }
 
-impl Setting {
+impl FromStr for Setting {
+    type Err = Error;
+
     // first element is the type:
     //   b:true
     //   i:-123
@@ -65,32 +66,38 @@ impl Setting {
     //   m:foo,bar,baz
 
     /// Converts a string to a setting or None when the string is invalid
-    pub fn from_string(key: &str) -> Option<Setting> {
+    fn from_str(key: &str) -> Result<Setting> {
         let (key_type, key_value) = key.split_once(':').unwrap();
 
-        match key_type {
-            "b" => match key_value.parse::<bool>() {
-                Ok(value) => Some(Setting::Bool(value)),
-                Err(_) => None,
-            },
-            "i" => match key_value.parse::<isize>() {
-                Ok(value) => Some(Setting::SInt(value)),
-                Err(_) => None,
-            },
-            "u" => match key_value.parse::<usize>() {
-                Ok(value) => Some(Setting::UInt(value)),
-                Err(_) => None,
-            },
-            "s" => Some(Setting::String(key_value.to_string())),
+        let setting = match key_type {
+            "b" => Setting::Bool(
+                key_value
+                    .parse::<bool>()
+                    .map_err(|err| Error::Config(format!("error parsing {key_value}: {err}")))?,
+            ),
+            "i" => Setting::SInt(
+                key_value
+                    .parse::<isize>()
+                    .map_err(|err| Error::Config(format!("error parsing {key_value}: {err}")))?,
+            ),
+            "u" => Setting::UInt(
+                key_value
+                    .parse::<usize>()
+                    .map_err(|err| Error::Config(format!("error parsing {key_value}: {err}")))?,
+            ),
+            "s" => Setting::String(key_value.to_string()),
+
             "m" => {
                 let mut values = Vec::new();
                 for value in key_value.split(',') {
                     values.push(value.to_string());
                 }
-                Some(Setting::Map(values))
+                Setting::Map(values)
             }
-            _ => None,
-        }
+            _ => return Err(Error::Config(format!("unknown setting: {key_value}"))),
+        };
+
+        Ok(setting)
     }
 }
 
@@ -113,34 +120,34 @@ mod test {
 
     #[test]
     fn setting() {
-        let s = Setting::from_string("b:true");
-        assert_eq!(s, Some(Setting::Bool(true)));
+        let s = Setting::from_str("b:true").unwrap();
+        assert_eq!(s, Setting::Bool(true));
 
-        let s = Setting::from_string("i:-1");
-        assert_eq!(s, Some(Setting::SInt(-1)));
+        let s = Setting::from_str("i:-1").unwrap();
+        assert_eq!(s, Setting::SInt(-1));
 
-        let s = Setting::from_string("i:1");
-        assert_eq!(s, Some(Setting::SInt(1)));
+        let s = Setting::from_str("i:1").unwrap();
+        assert_eq!(s, Setting::SInt(1));
 
-        let s = Setting::from_string("s:hello world");
-        assert_eq!(s, Some(Setting::String("hello world".into())));
+        let s = Setting::from_str("s:hello world").unwrap();
+        assert_eq!(s, Setting::String("hello world".into()));
 
-        let s = Setting::from_string("m:foo,bar,baz");
+        let s = Setting::from_str("m:foo,bar,baz").unwrap();
         assert_eq!(
             s,
-            Some(Setting::Map(vec!["foo".into(), "bar".into(), "baz".into()]))
+            Setting::Map(vec!["foo".into(), "bar".into(), "baz".into()])
         );
 
-        let s = Setting::from_string("notexist:true");
-        assert_eq!(s, None);
+        let s = Setting::from_str("notexist:true");
+        assert!(matches!(s, Err(Error::Config(_))));
 
-        let s = Setting::from_string("b:foobar");
-        assert_eq!(s, None);
+        let s = Setting::from_str("b:foobar");
+        assert!(matches!(s, Err(Error::Config(_))));
 
-        let s = Setting::from_string("i:foobar");
-        assert_eq!(s, None);
+        let s = Setting::from_str("i:foobar");
+        assert!(matches!(s, Err(Error::Config(_))));
 
-        let s = Setting::from_string("u:-1");
-        assert_eq!(s, None);
+        let s = Setting::from_str("u:-1");
+        assert!(matches!(s, Err(Error::Config(_))));
     }
 }