Handle Errors Properly

src/api/config.rs

@@ -3,9 +3,10 @@ use jsonrpsee::types::Params;
 use crate::state::RpcState;
 
 use super::ApiError;
+use super::ApiError::ConfigError;
 
 pub fn get_pending_changelog(_: Params, state: &RpcState) -> Result<(), ApiError> {
-    Err(ApiError::Leet)
+    Err(ApiError::NotImplemented)
 }
 
 pub fn apply_pending_changes(_: Params, state: &RpcState) -> Result<(), ApiError> {
@@ -13,7 +14,7 @@ pub fn apply_pending_changes(_: Params, state: &RpcState) -> Result<(), ApiError
         .config_manager
         .clone()
         .apply_pending_changes()
-        .map_err(|source| ApiError::Leet)
+        .map_err(ConfigError)
 }
 
 pub fn discard_pending_changes(_: Params, state: &RpcState) -> Result<(), ApiError> {
@@ -21,5 +22,5 @@ pub fn discard_pending_changes(_: Params, state: &RpcState) -> Result<(), ApiErr
         .config_manager
         .clone()
         .discard_pending_changes()
-        .map_err(|source| ApiError::Leet)
+        .map_err(ConfigError)
 }

src/api/mod.rs

@@ -18,11 +18,17 @@ use tracing::info;
 
 #[derive(Error, Debug)]
 pub enum ApiError {
-    #[error("Unsupported config version")]
+    #[error("Not Implemented")]
-    InvalidParams,
+    NotImplemented,
 
-    #[error("1337")]
+    #[error("Not Found")]
-    Leet,
+    NotFound,
+
+    #[error("Hash Error")]
+    HashError(#[from] pwhash::error::Error),
+
+    #[error(transparent)]
+    ParameterDeserialize(#[from] jsonrpsee::types::ErrorObjectOwned),
 
     #[error(transparent)]
     ConfigError(#[from] crate::config_manager::ConfigError),
@@ -31,8 +37,9 @@ pub enum ApiError {
 impl Into<ErrorObject<'static>> for ApiError {
     fn into(self) -> ErrorObject<'static> {
         match self {
-            Self::InvalidParams => ErrorCode::InvalidParams,
+            // Todo Add Proper mappings
-            Self::Leet => ErrorCode::ServerError(1337),
+            // ApiError::ParameterDeserialize => ErrorCode::InvalidParams,
+            ApiError::NotImplemented => ErrorCode::ServerError(0),
             _ => ErrorCode::InternalError,
         }
         .into()

src/api/system.rs

@@ -1,10 +1,13 @@
-use std::collections::HashMap;
-
 use crate::{definitions::system::User, state::RpcState};
 use jsonrpsee::types::Params;
 use pwhash::sha512_crypt;
 use serde::{Deserialize, Serialize};
 
+use ApiError::ConfigError;
+use ApiError::HashError;
+use ApiError::NotFound;
+use ApiError::ParameterDeserialize;
+
 use super::{ApiError, GetStringID};
 
 #[derive(Serialize, Clone)]
@@ -14,7 +17,7 @@ pub struct GetUser {
 }
 
 pub fn get_user(p: Params, state: &RpcState) -> Result<GetUser, ApiError> {
-    let u: GetStringID = p.parse().unwrap();
+    let u: GetStringID = p.parse().map_err(ParameterDeserialize)?;
 
     match state
         .config_manager
@@ -27,7 +30,7 @@ pub fn get_user(p: Params, state: &RpcState) -> Result<GetUser, ApiError> {
             name: u.id,
             comment: user.comment.clone(),
         }),
-        None => Err(ApiError::InvalidParams),
+        None => Err(NotFound),
     }
 }
 
@@ -57,9 +60,9 @@ struct CreateUser {
 }
 
 pub fn create_user(p: Params, state: &RpcState) -> Result<(), ApiError> {
-    let u: CreateUser = p.parse().unwrap();
+    let u: CreateUser = p.parse().map_err(ParameterDeserialize)?;
 
-    let hash = sha512_crypt::hash(u.password).unwrap();
+    let hash = sha512_crypt::hash(u.password).map_err(HashError)?;
 
     let mut cm = state.config_manager.clone();
     let mut tx = cm.start_transaction();
@@ -80,10 +83,11 @@ pub fn create_user(p: Params, state: &RpcState) -> Result<(), ApiError> {
         )
         .is_none()
     {
-        return tx.commit().map_err(|source| ApiError::InvalidParams);
+        tx.commit().map_err(ConfigError)?;
+        Ok(())
     } else {
         tx.revert();
-        Err(ApiError::InvalidParams)
+        Err(NotFound)
     }
 }
 
@@ -95,13 +99,20 @@ struct UpdateUser {
 }
 
 pub fn update_user(p: Params, state: &RpcState) -> Result<(), ApiError> {
-    let u: UpdateUser = p.parse().unwrap();
+    let u: UpdateUser = p.parse().map_err(ParameterDeserialize)?;
 
     let mut cm = state.config_manager.clone();
     let mut tx = cm.start_transaction();
 
     match tx.changes.system.users.get(&u.name) {
         Some(user) => {
+            // Only Update Password if field is not empty
+            let hash = if u.password == "" {
+                user.hash.clone()
+            } else {
+                sha512_crypt::hash(u.password).map_err(HashError)?
+            };
+
             tx.changes.system.users.insert(
                 u.name,
                 User {
@@ -109,17 +120,12 @@ pub fn update_user(p: Params, state: &RpcState) -> Result<(), ApiError> {
                         Some(c) => c,
                         None => "".to_string(),
                     },
-                    // Only Update Password if field is not empty
+                    hash,
-                    hash: if u.password == "" {
-                        user.hash.clone()
-                    } else {
-                        sha512_crypt::hash(u.password).unwrap()
-                    },
                 },
             );
             Ok(())
         }
-        None => Err(ApiError::InvalidParams),
+        None => Err(NotFound),
     }
 }
 
@@ -129,16 +135,16 @@ struct DeleteUser {
 }
 
 pub fn delete_user(p: Params, state: &RpcState) -> Result<(), ApiError> {
-    let u: DeleteUser = p.parse().unwrap();
+    let u: DeleteUser = p.parse().map_err(ParameterDeserialize)?;
 
     let mut cm = state.config_manager.clone();
     let mut tx = cm.start_transaction();
 
     match tx.changes.system.users.remove(&u.name) {
-        Some(_) => return tx.commit().map_err(|source| ApiError::InvalidParams),
+        Some(_) => tx.commit().map_err(ConfigError),
         None => {
             tx.revert();
-            Err(ApiError::InvalidParams)
+            Err(NotFound)
         }
     }
 }

src/config_manager.rs

@@ -1,11 +1,7 @@
 use validator::Validate;
 
-use crate::api::ApiError;
-
 use super::definitions::config::Config;
-use anyhow::{Context, Result};
 use std::fs;
-use std::io;
 use std::sync::{Arc, Mutex, MutexGuard};
 
 use pwhash::sha512_crypt;
@@ -65,12 +61,11 @@ impl ConfigManager {
         self.shared_data.lock().unwrap().pending_config.clone()
     }
 
-    pub fn apply_pending_changes(&mut self) -> Result<()> {
+    pub fn apply_pending_changes(&mut self) -> Result<(), ConfigError> {
         let mut data = self.shared_data.lock().unwrap();
         // TODO run Apply functions
         // TODO Revert on Apply Failure and Return
-        write_config_to_file(CURRENT_CONFIG_PATH, data.pending_config.clone())
+        write_config_to_file(CURRENT_CONFIG_PATH, data.pending_config.clone())?;
-            .context("Writing Config to file".to_string())?;
         // TODO revert if config save fails
         // TODO Remove Pending Config File
         data.current_config = data.pending_config.clone();

src/web/rpc.rs

@@ -27,8 +27,7 @@ impl ToRpcParams for ParamConverter {
                     .map(Some)
                     .map_err(Error::ParseError)
             }
-            // TODO make this a Parse error wrapping Utf8Error
+            Err(err) => return Err(Error::Custom(err.to_string())),
-            Err(err) => return Err(Error::AlreadyStopped),
         }
     }
 }