error handling improvements

src/actions/users.rs

@@ -32,12 +32,11 @@ pub fn _find_user_by_name(
 
 pub fn get_all(
     conn: &SqliteConnection,
-) -> Result<Option<Vec<models::UserDto>>, errors::DomainError> {
+) -> Result<Vec<models::UserDto>, errors::DomainError> {
     use crate::schema::users::dsl::*;
     Ok(users
         .select((name, created_at))
-        .load::<models::UserDto>(conn)
-        .optional()?)
+        .load::<models::UserDto>(conn)?)
 }
 
 /// Run query using Diesel to insert a new database row and return the result.

src/errors/domain_error.rs

@@ -27,43 +27,67 @@ custom_error! { #[derive(new)] pub DomainError
     DbError {source: diesel::result::Error} = "Database error",
     DbPoolError {source: r2d2::Error} = "Failed to get connection from pool",
     PasswordError {cause: String} = "Failed to validate password - {cause}",
-    GenericError {cause: String} = "Generic Error - Reason: {cause}"
+    EntityDoesNotExistError {message: String} = "Entity does not exist - {message}",
+    ThreadPoolError {message: String} = "Thread pool error - {message}",
+    AuthError {message: String} = "Authentication Error - {message}"
 }
 
 impl ResponseError for DomainError {
     fn error_response(&self) -> HttpResponse {
         let err = self;
         match self {
-            DomainError::PwdHashError { source } => {
+            DomainError::PwdHashError { source: _ } => {
                 HttpResponse::InternalServerError().json(ErrorModel {
-                    error_code: 500,
-                    reason: format!("{} {}", err.to_string(), source).as_str(),
+                    // error_code: 500,
+                    success: false,
+                    reason: err.to_string().as_str(),
                 })
             }
-            DomainError::DbError { source } => {
+            DomainError::DbError { source: _ } => {
+                error!("{}", err);
                 HttpResponse::InternalServerError().json(ErrorModel {
-                    error_code: 500,
-                    reason: format!("{} {}", err.to_string(), source).as_str(),
+                    // error_code: 500,
+                    success: false,
+                    reason: "Error in database",
                 })
             }
-            DomainError::DbPoolError { source } => {
+            DomainError::DbPoolError { source: _ } => {
+                error!("{}", err);
                 HttpResponse::InternalServerError().json(ErrorModel {
-                    error_code: 500,
-                    reason: format!("{} {}", err.to_string(), source).as_str(),
+                    // error_code: 500,
+                    success: false,
+                    reason: "Error getting database pool",
                 })
             }
             DomainError::PasswordError { cause: _ } => {
                 HttpResponse::BadRequest().json(ErrorModel {
-                    error_code: 400,
+                    // error_code: 400,
+                    success: false,
+                    reason: err.to_string().as_str(),
+                })
+            }
+            DomainError::EntityDoesNotExistError { message: _ } => {
+                HttpResponse::Accepted().json(ErrorModel {
+                    // error_code: 400,
+                    success: false,
+                    reason: err.to_string().as_str(),
+                })
+            }
+            DomainError::ThreadPoolError { message: _ } => {
+                error!("{}", err);
+                HttpResponse::InternalServerError().json(ErrorModel {
+                    // error_code: 400,
+                    success: false,
+                    reason: "Thread pool error occurred",
+                })
+            }
+            DomainError::AuthError { message: _ } => {
+                HttpResponse::Accepted().json(ErrorModel {
+                    // error_code: 400,
+                    success: false,
                     reason: err.to_string().as_str(),
                 })
             }
-            DomainError::GenericError { cause } => HttpResponse::BadRequest()
-                .json(ErrorModel {
-                    error_code: 400,
-                    reason: format!("{} {}, ", err.to_string(), cause.clone())
-                        .as_str(),
-                }),
         }
     }
 }

src/main.rs

@@ -15,7 +15,6 @@ use rand::Rng;
 
 use diesel::prelude::*;
 use diesel::r2d2::{self, ConnectionManager};
-use listenfd::ListenFd;
 use std::io;
 use std::io::ErrorKind;
 use types::DbPool;
@@ -119,8 +118,7 @@ async fn main() -> std::io::Result<()> {
             ))
             .wrap(middleware::Logger::default())
             .service(
-                web::scope("/api/authzd") // endpoint requiring authentication
-                    // .wrap(_basic_auth_middleware.clone())
+                web::scope("/api")
                     .service(routes::users::get_user)
                     .service(routes::users::get_all_users),
             )
@@ -132,12 +130,5 @@ async fn main() -> std::io::Result<()> {
             .service(routes::users::add_user)
             .service(fs::Files::new("/", "./static"))
     };
-    // HttpServer::new(app).bind(addr)?.run().await
-    let mut listenfd = ListenFd::from_env();
-    let server = HttpServer::new(app);
-    let server = match listenfd.take_tcp_listener(0)? {
-        Some(l) => server.listen(l),
-        None => server.bind(addr),
-    }?;
-    server.run().await
+    HttpServer::new(app).bind(addr)?.run().await
 }

src/models/errors.rs

@@ -8,6 +8,7 @@ pub struct JsonErrorModel<'a> {
 }
 #[derive(Debug, Clone, Serialize, new)]
 pub struct ErrorModel<'a> {
-    pub error_code: i16,
+    // pub error_code: i16,
+    pub success: bool,
     pub reason: &'a str,
 }

src/routes/auth.rs

@@ -2,7 +2,7 @@ use actix_web::web;
 use actix_web_httpauth::extractors::basic::BasicAuth;
 
 use crate::actions::users;
-use crate::{errors, AppConfig};
+use crate::{errors::DomainError, AppConfig};
 use actix_identity::Identity;
 use actix_web::{get, Error, HttpResponse};
 
@@ -11,7 +11,7 @@ pub async fn login(
     id: Identity,
     credentials: BasicAuth,
     config: web::Data<AppConfig>,
-) -> Result<HttpResponse, Error> {
+) -> Result<HttpResponse, DomainError> {
     let maybe_identity = id.identity();
     let response = if let Some(identity) = maybe_identity {
         Ok(HttpResponse::Found()
@@ -22,16 +22,16 @@ pub async fn login(
         let credentials2 = credentials.clone();
         let valid =
             web::block(move || validate_basic_auth(credentials2, &config))
-                .await?;
+                .await
+                .map_err(|_err| {
+                    DomainError::new_thread_pool_error(_err.to_string())
+                })?;
         if valid {
             id.remember(credentials.user_id().to_string());
             Ok(HttpResponse::Found().header("location", "/").finish())
         } else {
-            Ok(HttpResponse::BadRequest().json(
-                crate::models::errors::ErrorModel::new(
-                    20,
-                    "Wrong password or account does not exist",
-                ),
+            Err(DomainError::new_auth_error(
+                "Wrong password or account does not exist".to_owned(),
             ))
         }
     };
@@ -69,7 +69,7 @@ pub async fn index(id: Identity) -> String {
 pub fn validate_basic_auth(
     credentials: BasicAuth,
     config: &AppConfig,
-) -> Result<bool, errors::DomainError> {
+) -> Result<bool, DomainError> {
     let result = if let Some(password_ref) = credentials.password() {
         let pool = &config.pool;
         let conn = pool.get()?;
@@ -81,7 +81,7 @@ pub fn validate_basic_auth(
         )?;
         Ok(valid)
     } else {
-        Err(errors::DomainError::new_password_error(
+        Err(DomainError::new_password_error(
             "No password given".to_owned(),
         ))
     };

src/routes/users.rs

@@ -2,6 +2,7 @@ use actix_web::{get, post, web, HttpResponse};
 
 use crate::errors::DomainError;
 use crate::services::UserService;
+use crate::utils::LogErrorResult;
 use crate::AppConfig;
 use crate::{actions, models};
 use actix_web::error::ResponseError;
@@ -11,9 +12,9 @@ use validator::Validate;
 #[get("/get/users/{user_id}")]
 pub async fn get_user(
     config: web::Data<AppConfig>,
-    user_id: web::Path<i32>,
+    user_id_param: web::Path<i32>,
 ) -> Result<HttpResponse, DomainError> {
-    let u_id = user_id.into_inner();
+    let u_id = user_id_param.into_inner();
     // use web::block to offload blocking Diesel code without blocking server thread
     let res = web::block(move || {
         let pool = &config.pool;
@@ -21,19 +22,16 @@ pub async fn get_user(
         actions::find_user_by_uid(u_id, &conn)
     })
     .await
-    .map_err(|_err| {
-        let res = DomainError::new_generic_error(format!(
-            "No user found with uid: {}",
-            u_id
-        ));
-        res
-    })?;
+    .map_err(|err| DomainError::new_thread_pool_error(err.to_string()))
+    .log_err()?;
     if let Some(user) = res {
         Ok(HttpResponse::Ok().json(user))
     } else {
-        let res = HttpResponse::NotFound()
-            .body(format!("No user found with uid: {}", u_id));
-        Ok(res)
+        let err = DomainError::new_entity_does_not_exist_error(format!(
+            "No user found with uid: {}",
+            u_id
+        ));
+        Err(err)
     }
 }
 
@@ -47,43 +45,39 @@ pub async fn get_user2(
     if let Some(user) = user {
         Ok(HttpResponse::Ok().json(user))
     } else {
-        let res = HttpResponse::NotFound()
-            .body(format!("No user found with uid: {}", u_id));
-        Ok(res)
+        let err = DomainError::new_entity_does_not_exist_error(format!(
+            "No user found with uid: {}",
+            u_id
+        ));
+        Err(err)
     }
 }
 
 #[get("/get/users")]
 pub async fn get_all_users(
     config: web::Data<AppConfig>,
-) -> Result<HttpResponse, impl ResponseError> {
+) -> Result<HttpResponse, DomainError> {
     // use web::block to offload blocking Diesel code without blocking server thread
-    let res = web::block(move || {
+    let users = web::block(move || {
         let pool = &config.pool;
         let conn = pool.get()?;
         actions::get_all(&conn)
     })
     .await
-    .map(|maybe_users| {
-        debug!("{:?}", maybe_users);
-        if let Some(users) = maybe_users {
-            if users.is_empty() {
-                let res = HttpResponse::NotFound()
-                    .json(models::ErrorModel::new(40, "No users available"));
-                // let res = crate::errors::DomainError::new_generic_error("".to_owned());
-                res
-            } else {
-                HttpResponse::Ok().json(users)
-            }
-        } else {
-            let res = HttpResponse::NotFound()
-                .json(models::ErrorModel::new(40, "No users available"));
-            res
-        }
-    });
-    res
-}
+    .map_err(|err| DomainError::new_thread_pool_error(err.to_string()))
+    .log_err()?;
+
+    debug!("{:?}", users);
 
+    if !users.is_empty() {
+        Ok(HttpResponse::Ok().json(users))
+    } else {
+        Err(DomainError::new_entity_does_not_exist_error(
+            "No users available".to_owned(),
+        ))
+    }
+}
+//TODO: Add refinement here
 /// Inserts new user with name defined in form.
 #[post("/do_registration")]
 pub async fn add_user(

src/services/user_service.rs

@@ -10,9 +10,7 @@ pub trait UserService {
         user_name: String,
     ) -> Result<Option<models::UserDto>, errors::DomainError>;
 
-    fn get_all(
-        &self,
-    ) -> Result<Option<Vec<models::UserDto>>, errors::DomainError>;
+    fn get_all(&self) -> Result<Vec<models::UserDto>, errors::DomainError>;
 
     fn insert_new_user(
         &self,
@@ -50,9 +48,7 @@ impl UserService for UserServiceImpl {
         actions::_find_user_by_name(user_name, &conn)
     }
 
-    fn get_all(
-        &self,
-    ) -> Result<Option<Vec<models::UserDto>>, errors::DomainError> {
+    fn get_all(&self) -> Result<Vec<models::UserDto>, errors::DomainError> {
         let conn = self.pool.get()?;
         actions::get_all(&conn)
     }

src/utils.rs

@@ -2,3 +2,5 @@ pub mod auth;
 pub mod regexs;
 pub use self::auth::*;
 pub use self::regexs::*;
+pub mod ops;
+pub use self::ops::*;

src/utils/ops.rs

@@ -0,0 +1,34 @@
+use std::fmt::Display;
+
+pub trait LogErrorResult<T, E> {
+    fn log_err(self) -> Result<T, E>;
+}
+impl<T, E: Display> LogErrorResult<T, E> for Result<T, E> {
+    fn log_err(self) -> Result<T, E> {
+        self.map_err(|err| {
+            error!("{}", err.to_string());
+            err
+        })
+    }
+}
+
+trait ResultOps<T, E> {
+    fn tap<U, F: FnOnce(T) -> U>(self, op: F) -> Result<T, E>;
+    fn tap_err<F, O: FnOnce(E) -> F>(self, op: O) -> Result<T, E>;
+}
+
+impl<T: Clone, E: Clone> ResultOps<T, E> for Result<T, E> {
+    fn tap<U, F: FnOnce(T) -> U>(self, op: F) -> Result<T, E> {
+        self.map(|x| {
+            op(x.clone());
+            x
+        })
+    }
+
+    fn tap_err<F, O: FnOnce(E) -> F>(self, op: O) -> Result<T, E> {
+        self.map_err(|err| {
+            op(err.clone());
+            err
+        })
+    }
+}