From 9f76228ebeefc1a87568c20962c87ee5888d4ba7 Mon Sep 17 00:00:00 2001 From: StNicolay Date: Mon, 5 Aug 2024 23:32:16 +0300 Subject: [PATCH] Error handling --- src/auth.rs | 4 +- src/db/permissions.rs | 32 ++++---- src/endpoints/file/delete.rs | 15 ++-- src/endpoints/file/download.rs | 14 ++-- src/endpoints/file/modify.rs | 22 ++++-- src/endpoints/file/upload.rs | 6 +- src/endpoints/folder/create.rs | 18 +++-- src/endpoints/folder/delete.rs | 13 ++-- src/endpoints/folder/get_structure.rs | 12 +-- src/endpoints/folder/list.rs | 8 +- src/endpoints/permissions/delete.rs | 6 +- src/endpoints/permissions/get.rs | 6 +- src/endpoints/permissions/get_top_level.rs | 4 +- src/endpoints/permissions/set.rs | 26 ++++--- src/endpoints/users/delete.rs | 4 +- src/endpoints/users/get.rs | 6 +- src/endpoints/users/put.rs | 9 +-- src/endpoints/users/register.rs | 20 ++--- src/endpoints/users/search.rs | 4 +- src/errors.rs | 90 +++++++++++++++++++--- src/prelude.rs | 7 +- 21 files changed, 209 insertions(+), 117 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 19460a7..56218b2 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -16,7 +16,7 @@ use rand::{rngs::OsRng, RngCore}; use serde::{Deserialize, Serialize}; use subtle::ConstantTimeEq; -use crate::{db, errors::handle_error, Pool}; +use crate::{db, Pool}; pub const HASH_LENGTH: usize = 64; pub const SALT_LENGTH: usize = 64; @@ -186,7 +186,7 @@ where Ok(true) => Ok(claims), Ok(false) => Err(Error::WrongCredentials), Err(err) => { - handle_error(err); + tracing::error!(%err); Err(Error::Validation) } } diff --git a/src/db/permissions.rs b/src/db/permissions.rs index 16690fa..dcb8f5e 100644 --- a/src/db/permissions.rs +++ b/src/db/permissions.rs @@ -32,41 +32,39 @@ impl From> for PermissionType { } } -impl From for PermissionRaw { - fn from(value: PermissionType) -> Self { - match value { - PermissionType::Manage => Self::Manage, - PermissionType::Write => Self::Write, - PermissionType::Read => Self::Read, - PermissionType::NoPermission => unreachable!(), - } - } -} - impl PermissionType { pub fn can_read(self) -> bool { self >= PermissionType::Read } - pub fn can_read_guard(self) -> Result<(), StatusCode> { + pub fn can_read_guard(self) -> GeneralResult<()> { if !self.can_read() { - return Err(StatusCode::NOT_FOUND); + return Err(GeneralError::message( + StatusCode::NOT_FOUND, + "Item not found", + )); } Ok(()) } - pub fn can_write_guard(self) -> Result<(), StatusCode> { + pub fn can_write_guard(self) -> GeneralResult<()> { self.can_read_guard()?; if self < PermissionType::Write { - return Err(StatusCode::FORBIDDEN); + return Err(GeneralError::message( + StatusCode::FORBIDDEN, + "Cannot write to the folder", + )); } Ok(()) } - pub fn can_manage_guard(self) -> Result<(), StatusCode> { + pub fn can_manage_guard(self) -> GeneralResult<()> { self.can_read_guard()?; if self < PermissionType::Manage { - return Err(StatusCode::FORBIDDEN); + return Err(GeneralError::message( + StatusCode::FORBIDDEN, + "Cannot manage the folder", + )); } Ok(()) } diff --git a/src/endpoints/file/delete.rs b/src/endpoints/file/delete.rs index 11f125c..c3039d5 100644 --- a/src/endpoints/file/delete.rs +++ b/src/endpoints/file/delete.rs @@ -1,4 +1,4 @@ -pub use crate::prelude::*; +use crate::prelude::*; #[derive(Deserialize, Debug)] pub struct Params { @@ -9,24 +9,27 @@ pub async fn delete( Query(params): Query, State(state): State, claims: Claims, -) -> Result { +) -> GeneralResult { db::file::get_permissions(params.file_id, claims.user_id, &state.pool) .await - .handle_internal()? + .map_err(GeneralError::permissions)? .can_write_guard()?; let deleted = db::file::delete(params.file_id, &state.pool) .await - .handle_internal()?; + .handle_internal("Error deleting the file")?; if !deleted { - return Err(StatusCode::NOT_FOUND); // Will not happen most of the time due to can write guard + return Err(GeneralError::message( + StatusCode::NOT_FOUND, + "Item not found", + )); // Will not happen most of the time due to can write guard } state .storage .delete(params.file_id) .await - .handle_internal()?; + .handle_internal("Error deleting the file")?; Ok(StatusCode::NO_CONTENT) } diff --git a/src/endpoints/file/download.rs b/src/endpoints/file/download.rs index 7f4e24d..2e4fff0 100644 --- a/src/endpoints/file/download.rs +++ b/src/endpoints/file/download.rs @@ -12,16 +12,16 @@ pub async fn download( Query(params): Query, State(state): State, claims: Claims, -) -> Result { +) -> GeneralResult { db::file::get_permissions(params.file_id, claims.user_id, &state.pool) .await - .handle_internal()? + .map_err(GeneralError::permissions)? .can_read_guard()?; let mut name = db::file::get_name(params.file_id, &state.pool) .await - .handle_internal()? - .ok_or(StatusCode::NOT_FOUND)?; + .handle_internal("Error getting file info")? + .ok_or_else(GeneralError::item_not_found)?; name = name .chars() .fold(String::with_capacity(name.len()), |mut result, char| { @@ -32,7 +32,11 @@ pub async fn download( result }); - let file = state.storage.read(params.file_id).await.handle_internal()?; + let file = state + .storage + .read(params.file_id) + .await + .handle_internal("Error reading the file")?; let body = Body::from_stream(ReaderStream::new(file)); let disposition = format!("attachment; filename=\"{name}\""); let headers = [(header::CONTENT_DISPOSITION, disposition)]; diff --git a/src/endpoints/file/modify.rs b/src/endpoints/file/modify.rs index e696faf..ed95fe7 100644 --- a/src/endpoints/file/modify.rs +++ b/src/endpoints/file/modify.rs @@ -12,10 +12,10 @@ pub async fn modify( State(state): State, claims: Claims, mut multipart: Multipart, -) -> Result { +) -> GeneralResult { db::file::get_permissions(params.file_id, claims.user_id, &state.pool) .await - .handle_internal()? + .map_err(GeneralError::permissions)? .can_write_guard()?; // Very weird work around to get the first file in multipart @@ -23,7 +23,12 @@ pub async fn modify( match multipart.next_field().await { Ok(Some(field)) if field.file_name().is_some() => break field, Ok(Some(_)) => continue, - _ => return Err(StatusCode::BAD_REQUEST), + _ => { + return Err(GeneralError::message( + StatusCode::BAD_REQUEST, + "No file in the multipart", + )) + } } }; @@ -31,19 +36,22 @@ pub async fn modify( .storage .write(params.file_id) .await - .handle_internal()? - .ok_or(StatusCode::NOT_FOUND)?; + .handle_internal("Error writing to the file")? + .ok_or_else(GeneralError::item_not_found)?; let (hash, size) = crate::FileStorage::write_to_file(&mut file, &mut field) .await .map_err(|err| { tracing::warn!(%err); - StatusCode::INTERNAL_SERVER_ERROR + GeneralError::message( + StatusCode::INTERNAL_SERVER_ERROR, + "Error writing to the file", + ) })?; db::file::update(params.file_id, size, hash, &state.pool) .await - .handle_internal()?; + .handle_internal("Error updating the file")?; Ok(StatusCode::NO_CONTENT) } diff --git a/src/endpoints/file/upload.rs b/src/endpoints/file/upload.rs index c71ff6d..8314078 100644 --- a/src/endpoints/file/upload.rs +++ b/src/endpoints/file/upload.rs @@ -36,16 +36,16 @@ pub async fn upload( State(state): State, claims: Claims, mut multi: Multipart, -) -> Result>, StatusCode> { +) -> GeneralResult>> { db::folder::get_permissions(params.parent_folder, claims.user_id, &state.pool) .await - .handle_internal()? + .map_err(GeneralError::permissions)? .can_write_guard()?; let existing_names: HashSet = db::folder::get_names(params.parent_folder, &state.pool) .try_collect() .await - .handle_internal()?; + .handle_internal("Error getting existing names")?; let mut result = HashMap::new(); while let Ok(Some(mut field)) = multi.next_field().await { diff --git a/src/endpoints/folder/create.rs b/src/endpoints/folder/create.rs index 1835781..7110c66 100644 --- a/src/endpoints/folder/create.rs +++ b/src/endpoints/folder/create.rs @@ -10,22 +10,24 @@ pub async fn create( State(pool): State, claims: Claims, Json(params): Json, -) -> Result, StatusCode> { +) -> GeneralResult> { db::folder::get_permissions(params.parent_folder_id, claims.user_id, &pool) .await - .handle_internal()? + .map_err(GeneralError::permissions)? .can_write_guard()?; let exists = db::folder::name_exists(params.parent_folder_id, ¶ms.folder_name, &pool) .await - .handle_internal()?; + .handle_internal("Error getting existing names")?; if exists { - return Err(StatusCode::CONFLICT); + return Err(GeneralError::message( + StatusCode::CONFLICT, + "Name already taken", + )); } - let id = db::folder::insert(params.parent_folder_id, ¶ms.folder_name, &pool) + db::folder::insert(params.parent_folder_id, ¶ms.folder_name, &pool) .await - .handle_internal()?; - - Ok(Json(id)) + .handle_internal("Error creating the folder") + .map(Json) } diff --git a/src/endpoints/folder/delete.rs b/src/endpoints/folder/delete.rs index 0f9440c..a482a68 100644 --- a/src/endpoints/folder/delete.rs +++ b/src/endpoints/folder/delete.rs @@ -9,17 +9,20 @@ pub async fn delete( State(state): State, claims: Claims, Json(params): Json, -) -> Result<(), StatusCode> { +) -> GeneralResult<()> { let root = db::folder::get_root(claims.user_id, &state.pool) .await - .handle_internal()?; + .handle_internal("Error getting the root folder")?; if params.folder_id == root { - return Err(StatusCode::BAD_REQUEST); + return Err(GeneralError::message( + StatusCode::BAD_REQUEST, + "Cannot delete the root folder", + )); } db::folder::get_permissions(params.folder_id, claims.user_id, &state.pool) .await - .handle_internal()? + .map_err(GeneralError::permissions)? .can_write_guard()?; let storage = &state.storage; @@ -29,5 +32,5 @@ pub async fn delete( Ok(()) }) .await - .handle_internal() + .handle_internal("Error deleting the fodler") } diff --git a/src/endpoints/folder/get_structure.rs b/src/endpoints/folder/get_structure.rs index 67c220a..3be156e 100644 --- a/src/endpoints/folder/get_structure.rs +++ b/src/endpoints/folder/get_structure.rs @@ -25,16 +25,16 @@ pub async fn structure( Query(params): Query, State(pool): State, claims: Claims, -) -> Result, StatusCode> { +) -> GeneralResult> { let folder_id = db::folder::process_id(params.folder_id, claims.user_id, &pool) .await - .handle_internal()? - .ok_or(StatusCode::NOT_FOUND)?; + .map_err(GeneralError::permissions)? + .ok_or_else(GeneralError::item_not_found)?; let folder = db::folder::get_by_id(folder_id, &pool) .await - .handle_internal()? - .ok_or(StatusCode::NOT_FOUND)?; + .handle_internal("Error getting folder info")? + .ok_or_else(GeneralError::item_not_found)?; let mut response: FolderStructure = folder.into(); let mut stack = vec![&mut response]; @@ -45,7 +45,7 @@ pub async fn structure( .map_ok(Into::into) .try_collect() ) - .handle_internal()?; + .handle_internal("Error getting folder contents")?; folder.folders = folders; folder.files = files; stack.extend(folder.folders.iter_mut()); diff --git a/src/endpoints/folder/list.rs b/src/endpoints/folder/list.rs index 4168b2a..a35500f 100644 --- a/src/endpoints/folder/list.rs +++ b/src/endpoints/folder/list.rs @@ -18,17 +18,17 @@ pub async fn list( Query(params): Query, State(pool): State, claims: Claims, -) -> Result, StatusCode> { +) -> GeneralResult> { let folder_id = db::folder::process_id(params.folder_id, claims.user_id, &pool) .await - .handle_internal()? - .ok_or(StatusCode::NOT_FOUND)?; + .map_err(GeneralError::permissions)? + .handle(StatusCode::NOT_FOUND, "Item not found")?; let (files, folders) = try_join!( db::file::get_files(folder_id, &pool).try_collect(), db::folder::get_folders(folder_id, claims.user_id, &pool).try_collect() ) - .handle_internal()?; + .handle_internal("Error getting folder contents")?; Ok(Json(Response { folder_id, diff --git a/src/endpoints/permissions/delete.rs b/src/endpoints/permissions/delete.rs index 52acafd..0c1d7df 100644 --- a/src/endpoints/permissions/delete.rs +++ b/src/endpoints/permissions/delete.rs @@ -10,17 +10,17 @@ pub async fn delete( State(pool): State, claims: Claims, Json(params): Json, -) -> Result { +) -> GeneralResult { if params.user_id != claims.user_id { db::folder::get_permissions(params.folder_id, claims.user_id, &pool) .await - .handle_internal()? + .map_err(GeneralError::permissions)? .can_manage_guard()?; } db::permissions::delete_for_folder(params.folder_id, params.user_id, &pool) .await - .handle_internal()?; + .handle_internal("Error deleting the permissions")?; Ok(StatusCode::NO_CONTENT) } diff --git a/src/endpoints/permissions/get.rs b/src/endpoints/permissions/get.rs index eec875a..79e561f 100644 --- a/src/endpoints/permissions/get.rs +++ b/src/endpoints/permissions/get.rs @@ -13,14 +13,14 @@ pub async fn get( State(pool): State, Query(params): Query, claims: Claims, -) -> Result>, StatusCode> { +) -> GeneralResult>> { db::folder::get_permissions(params.folder_id, claims.user_id, &pool) .await - .handle_internal()? + .map_err(GeneralError::permissions)? .can_manage_guard()?; let permissions = db::permissions::get_all_for_folder(params.folder_id, &pool) .await - .handle_internal()?; + .handle_internal("Error getting permissions")?; Ok(Json(permissions)) } diff --git a/src/endpoints/permissions/get_top_level.rs b/src/endpoints/permissions/get_top_level.rs index 95d7d0e..55ebba6 100644 --- a/src/endpoints/permissions/get_top_level.rs +++ b/src/endpoints/permissions/get_top_level.rs @@ -3,9 +3,9 @@ use crate::prelude::*; pub async fn get_top_level( State(pool): State, claims: Claims, -) -> Result>, StatusCode> { +) -> GeneralResult>> { let folders = db::permissions::get_top_level_permitted_folders(claims.user_id, &pool) .await - .handle_internal()?; + .handle_internal("Error reading from the database")?; Ok(Json(folders)) } diff --git a/src/endpoints/permissions/set.rs b/src/endpoints/permissions/set.rs index 238e66e..f1e4558 100644 --- a/src/endpoints/permissions/set.rs +++ b/src/endpoints/permissions/set.rs @@ -1,6 +1,4 @@ -use db::permissions::PermissionRaw; - -use crate::prelude::*; +use crate::{db::permissions::PermissionRaw, prelude::*}; #[derive(Deserialize, Debug)] pub struct Params { @@ -13,25 +11,31 @@ pub async fn set( claims: Claims, State(pool): State, Json(params): Json, -) -> Result { +) -> GeneralResult { let root = db::folder::get_root(claims.user_id, &pool) .await - .handle_internal()?; + .handle_internal("Error getting the root folder")?; if params.folder_id == root { - return Err(StatusCode::BAD_REQUEST); + return Err(GeneralError::message( + StatusCode::BAD_REQUEST, + "Cannot delete the root folder", + )); } db::folder::get_permissions(params.folder_id, claims.user_id, &pool) .await - .handle_internal()? + .map_err(GeneralError::permissions)? .can_manage_guard()?; let folder_info = db::folder::get_by_id(params.folder_id, &pool) .await - .handle_internal()? - .ok_or(StatusCode::NOT_FOUND)?; + .handle_internal("Error getting folder info")? + .ok_or_else(GeneralError::item_not_found)?; if folder_info.owner_id == params.user_id { - return Err(StatusCode::BAD_REQUEST); + return Err(GeneralError::message( + StatusCode::BAD_REQUEST, + "Cannot set permissions of the folder owner", + )); } db::permissions::insert( @@ -41,7 +45,7 @@ pub async fn set( &pool, ) .await - .handle_internal()?; + .handle_internal("Error writing to the database")?; Ok(StatusCode::NO_CONTENT) } diff --git a/src/endpoints/users/delete.rs b/src/endpoints/users/delete.rs index 427b9a2..898e998 100644 --- a/src/endpoints/users/delete.rs +++ b/src/endpoints/users/delete.rs @@ -3,12 +3,12 @@ use crate::prelude::*; pub async fn delete( State(AppState { pool, ref storage }): State, claims: Claims, -) -> Result<(), StatusCode> { +) -> GeneralResult<()> { db::users::delete_user(claims.user_id, &pool) .try_for_each_concurrent(5, |file_id| async move { let _ = storage.delete(file_id).await; Ok(()) }) .await - .handle_internal() + .handle_internal("Error deleting the user") } diff --git a/src/endpoints/users/get.rs b/src/endpoints/users/get.rs index 074a2f9..96f040c 100644 --- a/src/endpoints/users/get.rs +++ b/src/endpoints/users/get.rs @@ -5,13 +5,13 @@ pub struct Params { user_id: i32, } -type Response = Result, StatusCode>; +type Response = GeneralResult>; pub async fn get(State(pool): State, Query(params): Query) -> Response { let info = db::users::get(params.user_id, &pool) .await - .handle_internal()? - .ok_or(StatusCode::NOT_FOUND)?; + .handle_internal("Error getting the user")? + .handle(StatusCode::NOT_FOUND, "User not found")?; Ok(Json(info)) } diff --git a/src/endpoints/users/put.rs b/src/endpoints/users/put.rs index 394421b..9c611ca 100644 --- a/src/endpoints/users/put.rs +++ b/src/endpoints/users/put.rs @@ -14,13 +14,10 @@ pub async fn put( State(pool): State, claims: Claims, Json(params): Json, -) -> Result, (StatusCode, String)> { - params - .validate() - .map_err(|err| (StatusCode::BAD_REQUEST, err.to_string()))?; +) -> GeneralResult> { + params.validate().map_err(GeneralError::validation)?; db::users::update(claims.user_id, ¶ms.username, ¶ms.email, &pool) .await - .handle_internal() - .map_err(|status| (status, String::new())) + .handle_internal("Error updating the user") .map(Json) } diff --git a/src/endpoints/users/register.rs b/src/endpoints/users/register.rs index 2597966..a092c11 100644 --- a/src/endpoints/users/register.rs +++ b/src/endpoints/users/register.rs @@ -48,22 +48,22 @@ fn validate_password(password: &str) -> Result<(), ValidationError> { pub async fn register( State(pool): State, Form(params): Form, -) -> Result, Either<(StatusCode, String), Error>> { +) -> Result, Either> { params .validate() - .map_err(|err| Either::E1((StatusCode::BAD_REQUEST, err.to_string())))?; + .map_err(GeneralError::validation) + .map_err(Either::E1)?; let password = HashedBytes::hash_bytes(params.password.as_bytes()).as_bytes(); - let Some(id) = db::users::create_user(¶ms.username, ¶ms.email, &password, &pool) + let id = db::users::create_user(¶ms.username, ¶ms.email, &password, &pool) .await - .handle_internal() - .map_err(|status| Either::E1((status, String::new())))? - else { - return Err(Either::E1(( + .handle_internal("Error creating the user") + .map_err(Either::E1)? + .handle( StatusCode::BAD_REQUEST, - "Either the user name or the email are taken".to_owned(), - ))); - }; + "The username or the email are taken", + ) + .map_err(Either::E1)?; let token = Claims::new(id).encode().map_err(Either::E2)?; Ok(Json(token)) diff --git a/src/endpoints/users/search.rs b/src/endpoints/users/search.rs index b699507..2851d5b 100644 --- a/src/endpoints/users/search.rs +++ b/src/endpoints/users/search.rs @@ -8,12 +8,12 @@ pub struct Params { pub async fn search( State(pool): State, Query(params): Query, -) -> sqlx::Result>, StatusCode> { +) -> GeneralResult>> { db::users::search_for_user(¶ms.search_string, &pool) .take(20) .try_filter(|user| future::ready(user.similarity > 0.1)) .try_collect() .await - .handle_internal() + .handle_internal("Error getting users from the database") .map(Json) } diff --git a/src/errors.rs b/src/errors.rs index da67177..dc48e0d 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -1,28 +1,96 @@ -use axum::http::StatusCode; +use std::{borrow::Cow, convert::Infallible}; + +use axum::{http::StatusCode, response::IntoResponse}; type BoxError = Box; -pub fn handle_error(error: impl Into) { - let error: BoxError = error.into(); - tracing::error!(error); +pub struct GeneralError { + pub status_code: StatusCode, + pub message: Cow<'static, str>, + pub error: Option, +} + +pub type GeneralResult = Result; + +impl GeneralError { + pub fn message(status_code: StatusCode, message: impl Into>) -> Self { + Self { + status_code, + message: message.into(), + error: None, + } + } + + #[allow(clippy::needless_pass_by_value)] + pub fn validation(error: validator::ValidationErrors) -> Self { + Self::message(StatusCode::BAD_REQUEST, error.to_string()) + } + + pub fn permissions(error: sqlx::Error) -> Self { + GeneralError { + status_code: StatusCode::INTERNAL_SERVER_ERROR, + message: Cow::Borrowed("Error getting permissions"), + error: Some(error.into()), + } + } + + pub const fn item_not_found() -> Self { + GeneralError { + status_code: StatusCode::NOT_FOUND, + message: Cow::Borrowed("Item not found"), + error: None, + } + } +} + +impl IntoResponse for GeneralError { + fn into_response(self) -> axum::response::Response { + if let Some(err) = self.error { + tracing::error!(err, message = %self.message, status_code = ?self.status_code); + } + (self.status_code, self.message).into_response() + } } pub trait ErrorHandlingExt where Self: Sized, { - fn handle(self, code: StatusCode) -> Result; + fn handle( + self, + status_code: StatusCode, + message: impl Into>, + ) -> GeneralResult; - fn handle_internal(self) -> Result { - self.handle(StatusCode::INTERNAL_SERVER_ERROR) + fn handle_internal(self, message: impl Into>) -> GeneralResult { + self.handle(StatusCode::INTERNAL_SERVER_ERROR, message) } } impl> ErrorHandlingExt for Result { - fn handle(self, code: StatusCode) -> Result { - self.map_err(|err| { - handle_error(err); - code + fn handle( + self, + status_code: StatusCode, + message: impl Into>, + ) -> GeneralResult { + self.map_err(|err| GeneralError { + status_code, + message: message.into(), + error: Some(err.into()), + }) + } +} + +impl ErrorHandlingExt for Option { + fn handle( + self, + status_code: StatusCode, + message: impl Into>, + ) -> GeneralResult { + self.ok_or_else(|| GeneralError { + status_code, + message: message.into(), + error: None, }) } } diff --git a/src/prelude.rs b/src/prelude.rs index 6bf2b80..5be2406 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -1,4 +1,9 @@ -pub(crate) use crate::{auth::Claims, db, errors::ErrorHandlingExt as _, AppState, Pool}; +pub(crate) use crate::{ + auth::Claims, + db, + errors::{ErrorHandlingExt as _, GeneralError, GeneralResult}, + AppState, Pool, +}; pub use axum::{ extract::{Json, Query, State}, http::StatusCode,