From 2bfff6c05d601eee385a19ca39635fd676494c24 Mon Sep 17 00:00:00 2001 From: Awiteb Date: Sat, 13 Jul 2024 23:15:09 +0300 Subject: [PATCH] refactor: Refactor server errors Signed-off-by: Awiteb --- crates/oxidetalis/src/database/blacklist.rs | 6 +- .../src/database/in_chat_requests.rs | 15 ++-- .../src/database/out_chat_requests.rs | 24 ++---- crates/oxidetalis/src/database/user.rs | 20 ++--- crates/oxidetalis/src/database/whitelist.rs | 26 +++---- crates/oxidetalis/src/errors.rs | 76 ++++++++----------- crates/oxidetalis/src/macros.rs | 7 +- crates/oxidetalis/src/main.rs | 6 +- crates/oxidetalis/src/routes/errors.rs | 68 +++++++++++++++++ crates/oxidetalis/src/routes/mod.rs | 3 + crates/oxidetalis/src/routes/user.rs | 2 +- 11 files changed, 147 insertions(+), 106 deletions(-) create mode 100644 crates/oxidetalis/src/routes/errors.rs diff --git a/crates/oxidetalis/src/database/blacklist.rs b/crates/oxidetalis/src/database/blacklist.rs index a35eccb..8448843 100644 --- a/crates/oxidetalis/src/database/blacklist.rs +++ b/crates/oxidetalis/src/database/blacklist.rs @@ -20,7 +20,7 @@ use oxidetalis_core::types::PublicKey; use oxidetalis_entities::prelude::*; use sea_orm::DatabaseConnection; -use crate::errors::ApiResult; +use crate::errors::ServerResult; /// Extension trait for the `DatabaseConnection` to work with the blacklist /// table @@ -31,7 +31,7 @@ pub trait BlackListExt { &self, blacklister: &UserModel, target_public_key: &PublicKey, - ) -> ApiResult; + ) -> ServerResult; } impl BlackListExt for DatabaseConnection { @@ -40,7 +40,7 @@ impl BlackListExt for DatabaseConnection { &self, blacklister: &UserModel, target_public_key: &PublicKey, - ) -> ApiResult { + ) -> ServerResult { blacklister .find_related(BlacklistEntity) .filter(BlacklistColumn::Target.eq(target_public_key.to_string())) diff --git a/crates/oxidetalis/src/database/in_chat_requests.rs b/crates/oxidetalis/src/database/in_chat_requests.rs index 137fee3..537b3fe 100644 --- a/crates/oxidetalis/src/database/in_chat_requests.rs +++ b/crates/oxidetalis/src/database/in_chat_requests.rs @@ -21,7 +21,7 @@ use oxidetalis_core::types::PublicKey; use oxidetalis_entities::prelude::*; use sea_orm::DatabaseConnection; -use crate::websocket::errors::{WsError, WsResult}; +use crate::errors::ServerResult; /// Extension trait for the `in_chat_requests` table. pub trait InChatRequestsExt { @@ -30,7 +30,7 @@ pub trait InChatRequestsExt { &self, requester: &PublicKey, recipient: &UserModel, - ) -> WsResult<()>; + ) -> ServerResult<()>; } impl InChatRequestsExt for DatabaseConnection { @@ -39,16 +39,12 @@ impl InChatRequestsExt for DatabaseConnection { &self, sender: &PublicKey, recipient: &UserModel, - ) -> WsResult<()> { - if sender.to_string() == recipient.public_key { - return Err(WsError::CannotSendChatRequestToSelf); - } + ) -> ServerResult<()> { if recipient .find_related(InChatRequestsEntity) .filter(InChatRequestsColumn::Sender.eq(sender.to_string())) .one(self) - .await - .map_err(|_| WsError::InternalServerError)? + .await? .is_none() { InChatRequestsActiveModel { @@ -58,8 +54,7 @@ impl InChatRequestsExt for DatabaseConnection { ..Default::default() } .save(self) - .await - .map_err(|_| WsError::InternalServerError)?; + .await?; } Ok(()) } diff --git a/crates/oxidetalis/src/database/out_chat_requests.rs b/crates/oxidetalis/src/database/out_chat_requests.rs index b687997..b51274d 100644 --- a/crates/oxidetalis/src/database/out_chat_requests.rs +++ b/crates/oxidetalis/src/database/out_chat_requests.rs @@ -21,10 +21,7 @@ use oxidetalis_core::types::PublicKey; use oxidetalis_entities::prelude::*; use sea_orm::DatabaseConnection; -use crate::{ - errors::ApiResult, - websocket::errors::{WsError, WsResult}, -}; +use crate::{errors::ServerResult, websocket::errors::WsError}; /// Extension trait for the `out_chat_requests` table. pub trait OutChatRequestsExt { @@ -33,14 +30,14 @@ pub trait OutChatRequestsExt { &self, requester: &UserModel, recipient: &PublicKey, - ) -> ApiResult; + ) -> ServerResult; /// Save the chat request in the requester table async fn save_out_chat_request( &self, requester: &UserModel, recipient: &PublicKey, - ) -> WsResult<()>; + ) -> ServerResult<()>; } impl OutChatRequestsExt for DatabaseConnection { @@ -49,7 +46,7 @@ impl OutChatRequestsExt for DatabaseConnection { &self, requester: &UserModel, recipient: &PublicKey, - ) -> ApiResult { + ) -> ServerResult { requester .find_related(OutChatRequestsEntity) .filter(OutChatRequestsColumn::Recipient.eq(recipient.to_string())) @@ -64,13 +61,9 @@ impl OutChatRequestsExt for DatabaseConnection { &self, requester: &UserModel, recipient: &PublicKey, - ) -> WsResult<()> { - if self - .have_chat_request_to(requester, recipient) - .await - .map_err(|_| WsError::InternalServerError)? - { - return Err(WsError::AlreadySendChatRequest); + ) -> ServerResult<()> { + if self.have_chat_request_to(requester, recipient).await? { + return Err(WsError::AlreadySendChatRequest.into()); } OutChatRequestsActiveModel { sender_id: Set(requester.id), @@ -79,8 +72,7 @@ impl OutChatRequestsExt for DatabaseConnection { ..Default::default() } .save(self) - .await - .map_err(|_| WsError::InternalServerError)?; + .await?; Ok(()) } } diff --git a/crates/oxidetalis/src/database/user.rs b/crates/oxidetalis/src/database/user.rs index 4cc27a0..4e94901 100644 --- a/crates/oxidetalis/src/database/user.rs +++ b/crates/oxidetalis/src/database/user.rs @@ -21,29 +21,29 @@ use oxidetalis_core::types::PublicKey; use oxidetalis_entities::prelude::*; use sea_orm::DatabaseConnection; -use crate::errors::{ApiError, ApiResult}; +use crate::{errors::ServerResult, routes::ApiError}; pub trait UserTableExt { /// Returns true if there is users in the database - async fn users_exists_in_database(&self) -> ApiResult; + async fn users_exists_in_database(&self) -> ServerResult; /// Register new user - async fn register_user(&self, public_key: &PublicKey, is_admin: bool) -> ApiResult<()>; + async fn register_user(&self, public_key: &PublicKey, is_admin: bool) -> ServerResult<()>; /// Returns user by its public key - async fn get_user_by_pubk(&self, public_key: &PublicKey) -> ApiResult>; + async fn get_user_by_pubk(&self, public_key: &PublicKey) -> ServerResult>; } impl UserTableExt for DatabaseConnection { #[logcall] - async fn users_exists_in_database(&self) -> ApiResult { + async fn users_exists_in_database(&self) -> ServerResult { UserEntity::find() .one(self) .await - .map_err(Into::into) .map(|u| u.is_some()) + .map_err(Into::into) } #[logcall] - async fn register_user(&self, public_key: &PublicKey, is_admin: bool) -> ApiResult<()> { + async fn register_user(&self, public_key: &PublicKey, is_admin: bool) -> ServerResult<()> { if let Err(err) = (UserActiveModel { public_key: Set(public_key.to_string()), is_admin: Set(is_admin), @@ -53,7 +53,7 @@ impl UserTableExt for DatabaseConnection { .await { if let Some(SqlErr::UniqueConstraintViolation(_)) = err.sql_err() { - return Err(ApiError::AlreadyRegistered); + return Err(ApiError::AlreadyRegistered.into()); } } @@ -61,11 +61,11 @@ impl UserTableExt for DatabaseConnection { } #[logcall] - async fn get_user_by_pubk(&self, public_key: &PublicKey) -> ApiResult> { + async fn get_user_by_pubk(&self, public_key: &PublicKey) -> ServerResult> { UserEntity::find() .filter(UserColumn::PublicKey.eq(public_key.to_string())) .one(self) .await - .map_err(ApiError::SeaOrm) + .map_err(Into::into) } } diff --git a/crates/oxidetalis/src/database/whitelist.rs b/crates/oxidetalis/src/database/whitelist.rs index 7a810d2..91d3fa2 100644 --- a/crates/oxidetalis/src/database/whitelist.rs +++ b/crates/oxidetalis/src/database/whitelist.rs @@ -21,10 +21,7 @@ use oxidetalis_core::types::PublicKey; use oxidetalis_entities::prelude::*; use sea_orm::DatabaseConnection; -use crate::{ - errors::ApiResult, - websocket::errors::{WsError, WsResult}, -}; +use crate::{errors::ServerResult, websocket::errors::WsError}; /// Extension trait for the `DatabaseConnection` to work with the whitelist /// table @@ -35,14 +32,14 @@ pub trait WhiteListExt { &self, whitelister: &UserModel, target_public_key: &PublicKey, - ) -> ApiResult; + ) -> ServerResult; /// Add the `target_public_key` to the whitelist of the `whitelister` async fn add_to_whitelist( &self, whitelister: &UserModel, target_public_key: &PublicKey, - ) -> WsResult<()>; + ) -> ServerResult<()>; } impl WhiteListExt for DatabaseConnection { @@ -50,7 +47,7 @@ impl WhiteListExt for DatabaseConnection { &self, whitelister: &UserModel, target_public_key: &PublicKey, - ) -> ApiResult { + ) -> ServerResult { whitelister .find_related(WhitelistEntity) .filter(WhitelistColumn::Target.eq(target_public_key.to_string())) @@ -64,16 +61,12 @@ impl WhiteListExt for DatabaseConnection { &self, whitelister: &UserModel, target_public_key: &PublicKey, - ) -> WsResult<()> { - if self - .is_whitelisted(whitelister, target_public_key) - .await - .map_err(|_| WsError::InternalServerError)? - { - return Err(WsError::AlreadyOnTheWhitelist); + ) -> ServerResult<()> { + if self.is_whitelisted(whitelister, target_public_key).await? { + return Err(WsError::AlreadyOnTheWhitelist.into()); } if whitelister.public_key == target_public_key.to_string() { - return Err(WsError::CannotAddSelfToWhitelist); + return Err(WsError::CannotAddSelfToWhitelist.into()); } WhitelistActiveModel { user_id: Set(whitelister.id), @@ -82,8 +75,7 @@ impl WhiteListExt for DatabaseConnection { ..Default::default() } .save(self) - .await - .map_err(|_| WsError::InternalServerError)?; + .await?; Ok(()) } } diff --git a/crates/oxidetalis/src/errors.rs b/crates/oxidetalis/src/errors.rs index 325cfc3..68ef86a 100644 --- a/crates/oxidetalis/src/errors.rs +++ b/crates/oxidetalis/src/errors.rs @@ -14,24 +14,16 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -use salvo::{ - http::StatusCode, - oapi::{Components as OapiComponents, EndpointOutRegister, Operation as OapiOperation}, - Response, - Scribe, -}; +use sea_orm::DbErr; -use crate::{routes::write_json_body, schemas::MessageSchema}; +use crate::{routes::ApiError, websocket::errors::WsError}; /// Result type of the homeserver -#[allow(clippy::absolute_paths)] -pub(crate) type Result = std::result::Result; -#[allow(clippy::absolute_paths)] -pub type ApiResult = std::result::Result; +pub(crate) type ServerResult = Result; /// The homeserver errors #[derive(Debug, thiserror::Error)] -pub(crate) enum Error { +pub enum InternalError { #[error("Database Error: {0}")] Database(#[from] sea_orm::DbErr), #[error("{0}")] @@ -39,43 +31,39 @@ pub(crate) enum Error { } #[derive(Debug, thiserror::Error)] -pub enum ApiError { - /// Error from the database (500 Internal Server Error) - #[error("Internal server error")] - SeaOrm(#[from] sea_orm::DbErr), - /// The server registration is closed (403 Forbidden) - #[error("Server registration is closed")] - RegistrationClosed, - /// The entered public key is already registered (400 Bad Request) - #[error("The entered public key is already registered")] - AlreadyRegistered, - /// The user entered two different public keys - /// one in the header and other in the request body - /// (400 Bad Request) - #[error("You entered two different public keys")] - TwoDifferentKeys, +/// The homeserver errors +pub enum ServerError { + /// Internal server error, should not be exposed to the user + #[error("{0}")] + Internal(#[from] InternalError), + /// API error, errors happening in the API + #[error("{0}")] + Api(#[from] ApiError), + /// WebSocket error, errors happening in the WebSocket + #[error("{0}")] + Ws(#[from] WsError), } -impl ApiError { - /// Status code of the error - pub const fn status_code(&self) -> StatusCode { - match self { - Self::SeaOrm(_) => StatusCode::INTERNAL_SERVER_ERROR, - Self::RegistrationClosed => StatusCode::FORBIDDEN, - Self::AlreadyRegistered | Self::TwoDifferentKeys => StatusCode::BAD_REQUEST, +impl From for ServerError { + fn from(err: DbErr) -> Self { + Self::Internal(err.into()) + } +} + +impl From for WsError { + fn from(err: ServerError) -> Self { + match err { + ServerError::Internal(_) | ServerError::Api(_) => WsError::InternalServerError, + ServerError::Ws(err) => err, } } } -impl EndpointOutRegister for ApiError { - fn register(_: &mut OapiComponents, _: &mut OapiOperation) {} -} - -impl Scribe for ApiError { - fn render(self, res: &mut Response) { - log::error!("Error: {self}"); - - res.status_code(self.status_code()); - write_json_body(res, MessageSchema::new(self.to_string())); +impl From for ApiError { + fn from(err: ServerError) -> Self { + match err { + ServerError::Internal(_) | ServerError::Ws(_) => ApiError::Internal, + ServerError::Api(err) => err, + } } } diff --git a/crates/oxidetalis/src/macros.rs b/crates/oxidetalis/src/macros.rs index 54c7149..2943e59 100644 --- a/crates/oxidetalis/src/macros.rs +++ b/crates/oxidetalis/src/macros.rs @@ -39,9 +39,9 @@ macro_rules! try_ws { match $result_expr { Ok(val) => val, Err(err) => { - log::error!("Error in try_ws macro: {err:?}"); + log::error!("{err}"); return $crate::websocket::ServerEvent::<$crate::websocket::Unsigned>::from( - $crate::websocket::errors::WsError::InternalServerError, + $crate::websocket::errors::WsError::from(err), ); } } @@ -60,11 +60,12 @@ macro_rules! try_ws { #[macro_export] macro_rules! ws_errors { ($($name:ident = $reason:tt),+ $(,)?) => { - #[derive(Debug)] + #[derive(Debug, thiserror::Error)] #[doc = "Websocket errors, returned in the websocket communication"] pub enum WsError { $( #[doc = $reason] + #[error($reason)] $name ),+ } diff --git a/crates/oxidetalis/src/main.rs b/crates/oxidetalis/src/main.rs index b15cfb5..3b373a0 100644 --- a/crates/oxidetalis/src/main.rs +++ b/crates/oxidetalis/src/main.rs @@ -19,6 +19,7 @@ use std::process::ExitCode; +use errors::ServerError; use oxidetalis_config::{CliArgs, Parser}; use oxidetalis_migrations::MigratorTrait; use salvo::{conn::TcpListener, Listener, Server}; @@ -34,11 +35,12 @@ mod schemas; mod utils; mod websocket; -async fn try_main() -> errors::Result<()> { +async fn try_main() -> errors::ServerResult<()> { pretty_env_logger::init_timed(); log::info!("Parsing configuration"); - let config = oxidetalis_config::Config::load(CliArgs::parse())?; + let config = oxidetalis_config::Config::load(CliArgs::parse()) + .map_err(|err| ServerError::Internal(err.into()))?; log::info!("Configuration parsed successfully"); log::info!("Connecting to the database"); let connection = sea_orm::Database::connect(utils::postgres_url(&config.postgresdb)).await?; diff --git a/crates/oxidetalis/src/routes/errors.rs b/crates/oxidetalis/src/routes/errors.rs new file mode 100644 index 0000000..29686de --- /dev/null +++ b/crates/oxidetalis/src/routes/errors.rs @@ -0,0 +1,68 @@ +// OxideTalis Messaging Protocol homeserver implementation +// Copyright (C) 2024 OxideTalis Developers +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as +// published by the Free Software Foundation, either version 3 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +use salvo::{ + http::StatusCode, + oapi::{Components as OapiComponents, EndpointOutRegister, Operation as OapiOperation}, + Response, + Scribe, +}; + +use crate::{routes::write_json_body, schemas::MessageSchema}; + +/// Result type of the API +pub type ApiResult = Result; + +#[derive(Debug, thiserror::Error)] +pub enum ApiError { + #[error("Internal server error")] + Internal, + /// The server registration is closed (403 Forbidden) + #[error("Server registration is closed")] + RegistrationClosed, + /// The entered public key is already registered (400 Bad Request) + #[error("The entered public key is already registered")] + AlreadyRegistered, + /// The user entered two different public keys + /// one in the header and other in the request body + /// (400 Bad Request) + #[error("You entered two different public keys")] + TwoDifferentKeys, +} + +impl ApiError { + /// Status code of the error + pub const fn status_code(&self) -> StatusCode { + match self { + Self::Internal => StatusCode::INTERNAL_SERVER_ERROR, + Self::RegistrationClosed => StatusCode::FORBIDDEN, + Self::AlreadyRegistered | Self::TwoDifferentKeys => StatusCode::BAD_REQUEST, + } + } +} + +impl EndpointOutRegister for ApiError { + fn register(_: &mut OapiComponents, _: &mut OapiOperation) {} +} + +impl Scribe for ApiError { + fn render(self, res: &mut Response) { + log::error!("Error: {self}"); + + res.status_code(self.status_code()); + write_json_body(res, MessageSchema::new(self.to_string())); + } +} diff --git a/crates/oxidetalis/src/routes/mod.rs b/crates/oxidetalis/src/routes/mod.rs index 45bdbd5..1923cb6 100644 --- a/crates/oxidetalis/src/routes/mod.rs +++ b/crates/oxidetalis/src/routes/mod.rs @@ -27,8 +27,11 @@ use crate::nonce::NonceCache; use crate::schemas::MessageSchema; use crate::{middlewares, websocket}; +mod errors; mod user; +pub use errors::*; + pub fn write_json_body(res: &mut Response, json_body: impl serde::Serialize) { res.write_body(serde_json::to_string(&json_body).expect("Json serialization can't be fail")) .ok(); diff --git a/crates/oxidetalis/src/routes/user.rs b/crates/oxidetalis/src/routes/user.rs index ccefe73..16a3e95 100644 --- a/crates/oxidetalis/src/routes/user.rs +++ b/crates/oxidetalis/src/routes/user.rs @@ -26,9 +26,9 @@ use salvo::{ Writer, }; +use super::{ApiError, ApiResult}; use crate::{ database::UserTableExt, - errors::{ApiError, ApiResult}, extensions::DepotExt, middlewares, schemas::{EmptySchema, MessageSchema, RegisterUserBody},