From b30d6d7aaf30ae8485acd851bef0e85b55c60e82 Mon Sep 17 00:00:00 2001 From: TheKrol Date: Fri, 21 Feb 2025 07:24:56 -0500 Subject: [PATCH 1/9] Update errors on group and hook --- backend/src/handlers_prelude/github_hook.rs | 21 ++-- backend/src/handlers_prelude/groups.rs | 117 +++++++------------- backend/src/handlers_prelude/mod.rs | 6 + frontend/static/css/theme.css | 5 +- 4 files changed, 57 insertions(+), 92 deletions(-) diff --git a/backend/src/handlers_prelude/github_hook.rs b/backend/src/handlers_prelude/github_hook.rs index 2a67df72..8a8df9b3 100644 --- a/backend/src/handlers_prelude/github_hook.rs +++ b/backend/src/handlers_prelude/github_hook.rs @@ -2,22 +2,25 @@ use axum::routing::post; use axum::{extract::State, http::HeaderMap, Router}; -use tracing::{debug, error, info}; +use tracing::{debug, info}; +use crate::handlers_prelude::ApiError; use crate::AppState; -pub async fn github_hook_handler(State(state): State, headers: HeaderMap) { +pub async fn github_hook_handler( + State(state): State, + headers: HeaderMap, +) -> Result<(), ApiError> { let event_type = headers.get("x-github-event").unwrap().to_str().unwrap(); - debug!("Received Github webhook event of type {event_type:?}"); + + debug!("Received Github webhook event of type {:?}", event_type); + if event_type == "push" { info!("New changes pushed to Github, pulling changes..."); - match state.git.pull() { - Ok(_) => {} - Err(e) => { - error!("Failed to auto-pull changes with error: {e:?}"); - } - } + state.git.pull()?; } + + Ok(()) } pub async fn create_github_route() -> Router { diff --git a/backend/src/handlers_prelude/groups.rs b/backend/src/handlers_prelude/groups.rs index ce8349a7..95674007 100644 --- a/backend/src/handlers_prelude/groups.rs +++ b/backend/src/handlers_prelude/groups.rs @@ -6,11 +6,10 @@ use axum::{ }; use reqwest::StatusCode; use serde::{Deserialize, Serialize}; -use tracing::error; use crate::{ db::{Database, Group}, - eyre_to_axum_err, + handlers_prelude::ApiError, perms::Permission, require_perms, AppState, }; @@ -33,16 +32,9 @@ pub struct GroupResponse { pub async fn create_group_response( db: &Database, group: Group, -) -> Result { - let permissions = db - .get_group_permissions(group.id) - .await - .map_err(eyre_to_axum_err)?; - - let members = db - .get_group_members(group.id) - .await - .map_err(eyre_to_axum_err)?; +) -> Result { + let permissions = db.get_group_permissions(group.id).await?; + let members = db.get_group_members(group.id).await?; Ok(GroupResponse { id: group.id, @@ -55,36 +47,24 @@ pub async fn create_group_response( username: m.username, avatar_url: m.avatar_url, }) - .collect::>(), + .collect(), }) } pub async fn get_groups_handler( State(state): State, headers: HeaderMap, -) -> Result>, (StatusCode, String)> { +) -> Result>, ApiError> { require_perms(State(&state), headers, &[Permission::ManageUsers]).await?; - match state.db.get_all_groups().await { - Ok(groups) => { - let mut get_groups_response = Vec::new(); - - for group in groups { - get_groups_response.push(create_group_response(&state.db, group).await?); - } - - Ok(Json(get_groups_response)) - } - Err(e) => { - error!("An error was encountered fetching all groups: {e:?}"); - Err(( - StatusCode::INTERNAL_SERVER_ERROR, - "An internal error was encountered fetching all groups, \ - check server logs for more info" - .to_owned(), - )) - } + let groups = state.db.get_all_groups().await?; + let mut get_groups_response = Vec::with_capacity(groups.len()); + + for group in groups { + get_groups_response.push(create_group_response(&state.db, group).await?); } + + Ok(Json(get_groups_response)) } #[derive(Serialize, Deserialize)] @@ -96,20 +76,13 @@ pub async fn post_group_handler( State(state): State, headers: HeaderMap, Json(body): Json, -) -> Result, (StatusCode, String)> { +) -> Result, ApiError> { require_perms(State(&state), headers, &[Permission::ManageUsers]).await?; - Ok(Json( - create_group_response( - &state.db, - state - .db - .create_group(body.group_name) - .await - .map_err(eyre_to_axum_err)?, - ) - .await?, - )) + let group = state.db.create_group(body.group_name).await?; + let response = create_group_response(&state.db, group).await?; + + Ok(Json(response)) } #[derive(Serialize, Deserialize)] @@ -122,17 +95,15 @@ pub async fn put_group_permissions_handler( headers: HeaderMap, Path(group_id): Path, Json(body): Json, -) -> Result, (StatusCode, String)> { +) -> Result, ApiError> { + // Ensure the user has the necessary permissions require_perms(State(&state), headers, &[Permission::ManageUsers]).await?; - let current_permissions = state - .db - .get_group_permissions(group_id) - .await - .map_err(eyre_to_axum_err)?; - + // Fetch current permissions for the group + let current_permissions = state.db.get_group_permissions(group_id).await?; let new_permissions = body.permissions; + // Identify permissions to remove and add let permissions_to_remove = current_permissions .iter() .filter(|perm| !new_permissions.contains(perm)) @@ -143,48 +114,34 @@ pub async fn put_group_permissions_handler( .filter(|perm| !current_permissions.contains(perm)) .collect::>(); + // Remove permissions for perm in permissions_to_remove { - state - .db - .remove_group_permission(group_id, *perm) - .await - .map_err(eyre_to_axum_err)?; + state.db.remove_group_permission(group_id, *perm).await?; } + // Add permissions for perm in permissions_to_add { - state - .db - .add_group_permission(group_id, *perm) - .await - .map_err(eyre_to_axum_err)?; + state.db.add_group_permission(group_id, *perm).await?; } - Ok(Json( - create_group_response( - &state.db, - state - .db - .get_group(group_id) - .await - .map_err(eyre_to_axum_err)? - .unwrap(), - ) - .await?, - )) + // Fetch updated group and return the response + let updated_group = state.db.get_group(group_id).await?.ok_or_else(|| { + ApiError::from((StatusCode::NOT_FOUND, "Group not found in the database".to_string())) + })?; + + Ok(Json(create_group_response(&state.db, updated_group).await?)) } pub async fn delete_group_handler( State(state): State, headers: HeaderMap, Path(group_id): Path, -) -> Result<(), (StatusCode, String)> { +) -> Result<(), ApiError> { require_perms(State(&state), headers, &[Permission::ManageUsers]).await?; - state - .db - .delete_group(group_id) - .await - .map_err(eyre_to_axum_err) + state.db.delete_group(group_id).await?; + + Ok(()) } pub async fn create_group_route() -> Router { diff --git a/backend/src/handlers_prelude/mod.rs b/backend/src/handlers_prelude/mod.rs index 75f3a478..2c9bf9b0 100644 --- a/backend/src/handlers_prelude/mod.rs +++ b/backend/src/handlers_prelude/mod.rs @@ -55,6 +55,12 @@ impl From for ApiError { } } +impl From<(StatusCode, String)> for ApiError { + fn from(err: (StatusCode, String)) -> Self { + Self(eyre::eyre!("{}: {}", err.0, err.1)) + } +} + /// Quick and dirty way to convert an eyre error to a (StatusCode, message) response, meant for use with `map_err`, so that errors can be propagated out of /// axum handlers with `?`. pub fn eyre_to_axum_err(e: Report) -> (StatusCode, String) { diff --git a/frontend/static/css/theme.css b/frontend/static/css/theme.css index ad4fbf58..965ee64a 100644 --- a/frontend/static/css/theme.css +++ b/frontend/static/css/theme.css @@ -4,9 +4,8 @@ https://primer.style/primitives/storybook/?path=/story/color-base-display-scales--all-scales */ :root { - --font-family: - system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, Cantarell, - 'Open Sans', 'Helvetica Neue', sans-serif; + --font-family: system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, + Cantarell, 'Open Sans', 'Helvetica Neue', sans-serif; --green: #329932; --red: #993232; From 982eaa6d881ac179564fb4436855b36b57c182e7 Mon Sep 17 00:00:00 2001 From: TheKrol Date: Fri, 21 Feb 2025 07:29:36 -0500 Subject: [PATCH 2/9] Removed comments --- backend/src/handlers_prelude/groups.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/backend/src/handlers_prelude/groups.rs b/backend/src/handlers_prelude/groups.rs index 95674007..dedcfbd7 100644 --- a/backend/src/handlers_prelude/groups.rs +++ b/backend/src/handlers_prelude/groups.rs @@ -47,7 +47,7 @@ pub async fn create_group_response( username: m.username, avatar_url: m.avatar_url, }) - .collect(), + .collect::>(), }) } @@ -96,14 +96,11 @@ pub async fn put_group_permissions_handler( Path(group_id): Path, Json(body): Json, ) -> Result, ApiError> { - // Ensure the user has the necessary permissions require_perms(State(&state), headers, &[Permission::ManageUsers]).await?; - // Fetch current permissions for the group let current_permissions = state.db.get_group_permissions(group_id).await?; let new_permissions = body.permissions; - // Identify permissions to remove and add let permissions_to_remove = current_permissions .iter() .filter(|perm| !new_permissions.contains(perm)) @@ -114,17 +111,14 @@ pub async fn put_group_permissions_handler( .filter(|perm| !current_permissions.contains(perm)) .collect::>(); - // Remove permissions for perm in permissions_to_remove { state.db.remove_group_permission(group_id, *perm).await?; } - // Add permissions for perm in permissions_to_add { state.db.add_group_permission(group_id, *perm).await?; } - // Fetch updated group and return the response let updated_group = state.db.get_group(group_id).await?.ok_or_else(|| { ApiError::from((StatusCode::NOT_FOUND, "Group not found in the database".to_string())) })?; From d9770224e3004ac49610ce40ebb0ca0472fa8aeb Mon Sep 17 00:00:00 2001 From: TheKrol Date: Fri, 21 Feb 2025 07:36:38 -0500 Subject: [PATCH 3/9] I don't know why npm run format did that, but I guess it is an issue --- frontend/static/css/theme.css | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frontend/static/css/theme.css b/frontend/static/css/theme.css index 965ee64a..b170d84e 100644 --- a/frontend/static/css/theme.css +++ b/frontend/static/css/theme.css @@ -4,9 +4,9 @@ https://primer.style/primitives/storybook/?path=/story/color-base-display-scales--all-scales */ :root { - --font-family: system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, - Cantarell, 'Open Sans', 'Helvetica Neue', sans-serif; - + --font-family: + system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, Cantarell, + 'Open Sans', 'Helvetica Neue', sans-serif; --green: #329932; --red: #993232; --toast-success: #36a331; From 3abcaae67d17ba55339e3809546f28208f387fd7 Mon Sep 17 00:00:00 2001 From: TheKrol Date: Sat, 22 Feb 2025 11:40:03 -0500 Subject: [PATCH 4/9] Finish all handler with update error codes --- backend/src/handlers_prelude/mod.rs | 99 ++++++++++++++++--------- backend/src/handlers_prelude/reclone.rs | 7 +- backend/src/handlers_prelude/repo_fs.rs | 82 +++++++++++--------- backend/src/handlers_prelude/users.rs | 68 +++++++++-------- 4 files changed, 146 insertions(+), 110 deletions(-) diff --git a/backend/src/handlers_prelude/mod.rs b/backend/src/handlers_prelude/mod.rs index 2c9bf9b0..bd71e7e2 100644 --- a/backend/src/handlers_prelude/mod.rs +++ b/backend/src/handlers_prelude/mod.rs @@ -27,48 +27,74 @@ use color_eyre::{ Report, }; use reqwest::StatusCode; -use tracing::{debug, error, trace}; +use tracing::{debug, trace}; use crate::{db::User, perms::Permission, AppState}; -pub struct ApiError(eyre::Error); +pub struct ApiError { + status: Option, + error: Report, +} -impl IntoResponse for ApiError { - fn into_response(self) -> Response { - ( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Something went wrong: {}", self.0), - ) - .into_response() +impl ApiError { + pub fn new(status: Option, error: impl Into) -> Self { + Self { + status, + error: error.into(), + } } } -impl From for ApiError { - fn from(err: eyre::Error) -> Self { - Self(err) +impl IntoResponse for ApiError { + fn into_response(self) -> Response { + let status = self.status.unwrap_or(StatusCode::INTERNAL_SERVER_ERROR); + (status, self.error.to_string()).into_response() } } impl From for ApiError { - fn from(err: String) -> Self { - Self(eyre::eyre!(err)) + fn from(message: String) -> Self { + Self { + status: Some(StatusCode::INTERNAL_SERVER_ERROR), + error: eyre::eyre!(message), + } } } impl From<(StatusCode, String)> for ApiError { - fn from(err: (StatusCode, String)) -> Self { - Self(eyre::eyre!("{}: {}", err.0, err.1)) + fn from((status, message): (StatusCode, String)) -> Self { + Self { + status: Some(status), + error: eyre::eyre!(message), + } + } +} + +impl From<&str> for ApiError { + fn from(message: &str) -> Self { + Self { + status: Some(StatusCode::INTERNAL_SERVER_ERROR), + error: eyre::eyre!(message.to_string()), + } } } -/// Quick and dirty way to convert an eyre error to a (StatusCode, message) response, meant for use with `map_err`, so that errors can be propagated out of -/// axum handlers with `?`. -pub fn eyre_to_axum_err(e: Report) -> (StatusCode, String) { - error!("An error was encountered in an axum handler: {e:?}"); - ( - StatusCode::INTERNAL_SERVER_ERROR, - format!("An error was encountered, check server logs for more info: {e}"), - ) +impl From<(StatusCode, &str)> for ApiError { + fn from((status, message): (StatusCode, &str)) -> Self { + Self { + status: Some(status), + error: eyre::eyre!(message.to_string()), + } + } +} + +impl From for ApiError { + fn from(error: Report) -> Self { + Self { + status: Some(StatusCode::INTERNAL_SERVER_ERROR), + error, + } + } } /// The output of a find_user call, used to differentiate between expired users and valid users @@ -124,13 +150,13 @@ pub async fn require_perms( State(state): State<&AppState>, headers: HeaderMap, perms: &[Permission], -) -> Result { - let maybe_user = find_user(state, headers).await.map_err(eyre_to_axum_err)?; +) -> Result { + let maybe_user = find_user(state, headers).await?; match maybe_user { Some(user) => match user { - FoundUser::ExpiredUser(u) => Err(( - StatusCode::UNAUTHORIZED, - format!( + FoundUser::ExpiredUser(u) => Err(ApiError::new( + Some(StatusCode::UNAUTHORIZED), + eyre::eyre!( "The access token has expired for the user {}, they must authenticate again.", u.username ), @@ -139,15 +165,14 @@ pub async fn require_perms( let user_perms = &state .db .get_user_permissions(u.id) - .await - .map_err(eyre_to_axum_err)?; + .await?; let has_permissions = perms.iter().all(|perm| user_perms.contains(perm)); if has_permissions { Ok(u) } else { - Err(( - StatusCode::FORBIDDEN, - format!( + Err(ApiError::new( + Some(StatusCode::FORBIDDEN), + eyre::eyre!( "User {:?} lacks the permission to edit documents.", u.username ), @@ -155,9 +180,9 @@ pub async fn require_perms( } } }, - None => Err(( - StatusCode::UNAUTHORIZED, - "No valid user is authenticated, perhaps you forgot to add `{credentials: \"include\"}` in your fetch options?.".to_string(), + None => Err(ApiError::new( + Some(StatusCode::UNAUTHORIZED), + eyre::eyre!("No valid user is authenticated, perhaps you forgot to add `{{credentials: \"include\"}}` in your fetch options?"), )), } } diff --git a/backend/src/handlers_prelude/reclone.rs b/backend/src/handlers_prelude/reclone.rs index ddcce336..88162283 100644 --- a/backend/src/handlers_prelude/reclone.rs +++ b/backend/src/handlers_prelude/reclone.rs @@ -1,17 +1,16 @@ use axum::routing::post; use axum::{extract::State, http::HeaderMap, Router}; -use reqwest::StatusCode; use crate::{perms::Permission, AppState}; -use super::{eyre_to_axum_err, require_perms}; +use super::{ApiError, require_perms}; pub async fn post_reclone_handler( State(state): State, headers: HeaderMap, -) -> Result<(), (StatusCode, String)> { +) -> Result<(), ApiError> { require_perms(State(&state), headers, &[Permission::ManageUsers]).await?; - state.git.reclone().map_err(eyre_to_axum_err)?; + state.git.reclone()?; Ok(()) } diff --git a/backend/src/handlers_prelude/repo_fs.rs b/backend/src/handlers_prelude/repo_fs.rs index 7fd1cf3b..2563f466 100644 --- a/backend/src/handlers_prelude/repo_fs.rs +++ b/backend/src/handlers_prelude/repo_fs.rs @@ -13,10 +13,9 @@ use reqwest::header::{CONTENT_DISPOSITION, CONTENT_TYPE}; use serde::{Deserialize, Serialize}; use tracing::{error, warn}; +use crate::handlers_prelude::ApiError; use crate::{perms::Permission, require_perms, AppState}; -use super::eyre_to_axum_err; - #[derive(Debug, Deserialize, Serialize)] pub struct GetDocQuery { pub path: String, @@ -27,11 +26,14 @@ pub struct GetDocResponse { pub contents: String, } -async fn get_gh_token(state: &AppState) -> Result { - state.gh_client.get_token().await.map_err(|e| { - error!("Failed to retrieve GitHub token: {e}"); - (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()) - }) +async fn get_gh_token(state: &AppState) -> Result { + match state.gh_client.get_token().await { + Ok(token) => Ok(token), + Err(e) => { + error!("Failed to retrieve GitHub token: {e}"); + Err(ApiError::from((StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))) + } + } } /// This handler accepts a `GET` request to `/api/doc?path=`. @@ -39,13 +41,13 @@ async fn get_gh_token(state: &AppState) -> Result pub async fn get_doc_handler( State(state): State, Query(query): Query, -) -> Result, (StatusCode, &'static str)> { +) -> Result, ApiError> { match state.git.get_doc(&query.path) { Ok(maybe_doc) => maybe_doc.map_or( - Err(( + Err(ApiError::from(( StatusCode::NOT_FOUND, - "The file at the provided path was not found.", - )), + "The file at the provided path was not found.".to_string(), + ))), |doc| Ok(Json(GetDocResponse { contents: doc })), ), Err(e) => { @@ -53,10 +55,10 @@ pub async fn get_doc_handler( "Failed to fetch doc with path: {:?}; error: {:?}", query.path, e ); - Err(( + Err(ApiError::from(( StatusCode::INTERNAL_SERVER_ERROR, - "Fetch failed, check server logs for more info", - )) + "Fetch failed, check server logs for more info".to_string(), + ))) } } } @@ -74,7 +76,7 @@ pub async fn put_doc_handler( State(state): State, headers: HeaderMap, Json(body): Json, -) -> Result { +) -> Result { let author = require_perms( axum::extract::State(&state), headers, @@ -99,10 +101,10 @@ pub async fn put_doc_handler( Ok(_) => Ok(StatusCode::CREATED), Err(e) => { error!("Failed to complete put_doc call with error: {e:?}"); - Err(( + Err(ApiError::from(( StatusCode::INTERNAL_SERVER_ERROR, "Failed to create document, check server logs for more info".to_string(), - )) + ))) } } } @@ -112,7 +114,7 @@ pub async fn delete_doc_handler( State(state): State, headers: HeaderMap, Query(query): Query, -) -> Result { +) -> Result { let author = require_perms( axum::extract::State(&state), headers, @@ -127,7 +129,10 @@ pub async fn delete_doc_handler( &format!("{} deleted {}", author.username, query.path), &get_gh_token(&state).await?, ) - .map_err(eyre_to_axum_err)?; + .map_err(|e| { + error!("Failed to delete doc: {:?}", e); + ApiError::from((StatusCode::INTERNAL_SERVER_ERROR, e.to_string())) + })?; Ok(StatusCode::NO_CONTENT) } @@ -136,16 +141,15 @@ pub async fn delete_doc_handler( /// representing the state of the tree. This is used in the viewer for directory navigation. pub async fn get_doc_tree_handler( State(state): State, -) -> Result, (StatusCode, &'static str)> { +) -> Result, ApiError> { match state.git.get_doc_tree() { Ok(t) => Ok(Json(t)), Err(e) => { error!("An error was encountered fetching the document tree: {e:?}"); - Err(( + Err(ApiError::from(( StatusCode::INTERNAL_SERVER_ERROR, - "An internal error was encountered fetching the doc tree, \ - check server logs for more info", - )) + "An internal error was encountered fetching the doc tree, check server logs for more info", + ))) } } } @@ -172,14 +176,21 @@ pub async fn get_asset_tree_handler( pub async fn get_asset_handler( State(state): State, Path(path): Path>, -) -> impl IntoResponse { +) -> Result { let file_name = path.last().unwrap().clone(); let path = path.join("/"); - // https://github.com/tokio-rs/axum/discussions/608#discussioncomment-1789020 - let file = match state.git.get_asset(&path).map_err(eyre_to_axum_err)? { + + // Attempt to retrieve the asset, returning an ApiError on failure + let file = match state.git.get_asset(&path)? { Some(file) => file, - None => return Err((StatusCode::NOT_FOUND, format!("File not found: {}", path))), + None => { + return Err(ApiError::from(( + StatusCode::NOT_FOUND, + format!("File not found: {}", path), + ))); + } }; + let mut headers = HeaderMap::new(); headers.insert( CONTENT_TYPE, @@ -191,9 +202,11 @@ pub async fn get_asset_handler( CONTENT_DISPOSITION, format!("inline; filename={file_name:?}").parse().unwrap(), ); + Ok((headers, file)) } + /// This handler creates or replaces the asset at the provided path /// with a new asset pub async fn put_asset_handler( @@ -201,7 +214,7 @@ pub async fn put_asset_handler( headers: HeaderMap, Path(path): Path>, body: Bytes, -) -> Result { +) -> Result { let path = path.join("/"); let author = require_perms( axum::extract::State(&state), @@ -215,11 +228,7 @@ pub async fn put_asset_handler( // Call put_asset to update the asset, passing the required parameters state .git - .put_asset(&path, &body, &message, &get_gh_token(&state).await?) - .map_err(|e| { - error!("Failed to update asset: {e}"); - (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()) - })?; + .put_asset(&path, &body, &message, &get_gh_token(&state).await?)?; Ok(StatusCode::CREATED) } @@ -230,15 +239,14 @@ pub async fn delete_asset_handler( State(state): State, headers: HeaderMap, Path(path): Path>, -) -> Result { +) -> Result { let path = path.join("/"); let author = require_perms(State(&state), headers, &[Permission::ManageContent]).await?; // Generate commit message combining author and default update message let message = format!("{} deleted {}", author.username, path); state .git - .delete_asset(&path, &message, &get_gh_token(&state).await?) - .map_err(eyre_to_axum_err)?; + .delete_asset(&path, &message, &get_gh_token(&state).await?)?; Ok(StatusCode::OK) } diff --git a/backend/src/handlers_prelude/users.rs b/backend/src/handlers_prelude/users.rs index edf3b0f7..1baa6d44 100644 --- a/backend/src/handlers_prelude/users.rs +++ b/backend/src/handlers_prelude/users.rs @@ -8,9 +8,9 @@ use reqwest::StatusCode; use serde::{Deserialize, Serialize}; use tracing::error; +use crate::handlers_prelude::ApiError; use crate::{ db::{Database, Group, User}, - eyre_to_axum_err, perms::Permission, require_perms, AppState, }; @@ -27,16 +27,14 @@ pub struct UserResponse { pub async fn create_user_response( db: &Database, user: User, -) -> Result { +) -> Result { let groups = db .get_user_groups(user.id) - .await - .map_err(eyre_to_axum_err)?; + .await?; let permissions = db .get_user_permissions(user.id) - .await - .map_err(eyre_to_axum_err)?; + .await?; Ok(UserResponse { id: user.id, @@ -50,7 +48,7 @@ pub async fn create_user_response( pub async fn get_users_handler( State(state): State, headers: HeaderMap, -) -> Result>, (StatusCode, String)> { +) -> Result>, ApiError> { require_perms(State(&state), headers, &[Permission::ManageUsers]).await?; match state.db.get_all_users().await { @@ -65,12 +63,12 @@ pub async fn get_users_handler( } Err(e) => { error!("An error was encountered fetching all users: {e:?}"); - Err(( + Err(ApiError::from(( StatusCode::INTERNAL_SERVER_ERROR, "An internal error was encountered fetching all users, \ check server logs for more info" .to_owned(), - )) + ))) } } } @@ -78,7 +76,7 @@ pub async fn get_users_handler( pub async fn get_current_user_handler( State(state): State, headers: HeaderMap, -) -> Result, (StatusCode, String)> { +) -> Result, ApiError> { let user = require_perms(axum::extract::State(&state), headers, &[]).await?; Ok(Json(create_user_response(&state.db, user).await?)) } @@ -93,22 +91,20 @@ pub async fn post_user_membership_handler( headers: HeaderMap, Path(user_id): Path, Json(body): Json, -) -> Result, (StatusCode, String)> { +) -> Result, ApiError> { require_perms(State(&state), headers, &[Permission::ManageUsers]).await?; for group_id in body.group_ids { state .db .add_group_membership(group_id, user_id) - .await - .map_err(eyre_to_axum_err)?; + .await?; } let user = state .db .get_user(user_id) - .await - .map_err(eyre_to_axum_err)? + .await? .unwrap(); Ok(Json(create_user_response(&state.db, user).await?)) @@ -119,22 +115,20 @@ pub async fn delete_user_membership_handler( headers: HeaderMap, Path(user_id): Path, Json(body): Json, -) -> Result, (StatusCode, String)> { +) -> Result, ApiError> { require_perms(State(&state), headers, &[Permission::ManageUsers]).await?; for group_id in body.group_ids { state .db .remove_group_membership(group_id, user_id) - .await - .map_err(eyre_to_axum_err)?; + .await?; } let user = state .db .get_user(user_id) - .await - .map_err(eyre_to_axum_err)? + .await? .unwrap(); Ok(Json(create_user_response(&state.db, user).await?)) @@ -144,27 +138,37 @@ pub async fn delete_user_handler( State(state): State, headers: HeaderMap, Path(user_id): Path, -) -> Result<(), (StatusCode, String)> { +) -> Result<(), ApiError> { require_perms(State(&state), headers, &[Permission::ManageUsers]).await?; - state - .db - .delete_user(user_id) - .await - .map_err(eyre_to_axum_err) + match state.db.delete_user(user_id).await { + Ok(_) => Ok(()), + Err(e) => { + error!("Failed to delete user with ID {}: {}", user_id, e); + Err(ApiError::from(( + StatusCode::INTERNAL_SERVER_ERROR, + "An internal error occurred while deleting the user, check server logs for more info", + ))) + } + } } pub async fn delete_current_user( State(state): State, headers: HeaderMap, -) -> Result<(), (StatusCode, String)> { +) -> Result<(), ApiError> { let user = require_perms(axum::extract::State(&state), headers, &[]).await?; - state - .db - .delete_user(user.id) - .await - .map_err(eyre_to_axum_err) + match state.db.delete_user(user.id).await { + Ok(_) => Ok(()), + Err(e) => { + error!("Failed to delete current user with ID {}: {}", user.id, e); + Err(ApiError::from(( + StatusCode::INTERNAL_SERVER_ERROR, + "An internal error occurred while deleting the user, check server logs for more info", + ))) + } + } } pub async fn create_user_route() -> Router { From 1532a2d4f96cba69b64222f251d79491adc14fe9 Mon Sep 17 00:00:00 2001 From: TheKrol Date: Sat, 22 Feb 2025 11:42:57 -0500 Subject: [PATCH 5/9] Update frontend format? --- frontend/static/css/theme.css | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frontend/static/css/theme.css b/frontend/static/css/theme.css index b170d84e..f6fab130 100644 --- a/frontend/static/css/theme.css +++ b/frontend/static/css/theme.css @@ -4,9 +4,8 @@ https://primer.style/primitives/storybook/?path=/story/color-base-display-scales--all-scales */ :root { - --font-family: - system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, Cantarell, - 'Open Sans', 'Helvetica Neue', sans-serif; + --font-family: system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, + Cantarell, 'Open Sans', 'Helvetica Neue', sans-serif; --green: #329932; --red: #993232; --toast-success: #36a331; From 93f02be8cb0e412d2cd82ada303a385335451ad8 Mon Sep 17 00:00:00 2001 From: TheKrol Date: Sat, 22 Feb 2025 13:16:49 -0500 Subject: [PATCH 6/9] Update frontend format --- frontend/static/css/theme.css | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frontend/static/css/theme.css b/frontend/static/css/theme.css index f6fab130..b170d84e 100644 --- a/frontend/static/css/theme.css +++ b/frontend/static/css/theme.css @@ -4,8 +4,9 @@ https://primer.style/primitives/storybook/?path=/story/color-base-display-scales--all-scales */ :root { - --font-family: system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, - Cantarell, 'Open Sans', 'Helvetica Neue', sans-serif; + --font-family: + system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, Cantarell, + 'Open Sans', 'Helvetica Neue', sans-serif; --green: #329932; --red: #993232; --toast-success: #36a331; From 29a5e7573f019a46ec2260f4e38070eb0db7ac8f Mon Sep 17 00:00:00 2001 From: TheKrol Date: Sat, 22 Feb 2025 15:09:04 -0500 Subject: [PATCH 7/9] Added logging for errors --- backend/src/handlers_prelude/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/src/handlers_prelude/mod.rs b/backend/src/handlers_prelude/mod.rs index bd71e7e2..668f9eb7 100644 --- a/backend/src/handlers_prelude/mod.rs +++ b/backend/src/handlers_prelude/mod.rs @@ -27,7 +27,7 @@ use color_eyre::{ Report, }; use reqwest::StatusCode; -use tracing::{debug, trace}; +use tracing::{debug, error, trace}; use crate::{db::User, perms::Permission, AppState}; @@ -47,6 +47,7 @@ impl ApiError { impl IntoResponse for ApiError { fn into_response(self) -> Response { + error!(error = %self.error, "Error returned from handler"); let status = self.status.unwrap_or(StatusCode::INTERNAL_SERVER_ERROR); (status, self.error.to_string()).into_response() } From 32decd958a730e8939a0c5d7f8a8b6f59849d13d Mon Sep 17 00:00:00 2001 From: TheKrol Date: Sat, 22 Feb 2025 15:34:33 -0500 Subject: [PATCH 8/9] update from feedback --- backend/src/handlers_prelude/github_hook.rs | 3 +- backend/src/handlers_prelude/repo_fs.rs | 42 ++++++++++----------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/backend/src/handlers_prelude/github_hook.rs b/backend/src/handlers_prelude/github_hook.rs index 8a8df9b3..787f3d32 100644 --- a/backend/src/handlers_prelude/github_hook.rs +++ b/backend/src/handlers_prelude/github_hook.rs @@ -4,6 +4,7 @@ use axum::routing::post; use axum::{extract::State, http::HeaderMap, Router}; use tracing::{debug, info}; +use color_eyre::eyre::WrapErr; use crate::handlers_prelude::ApiError; use crate::AppState; @@ -17,7 +18,7 @@ pub async fn github_hook_handler( if event_type == "push" { info!("New changes pushed to Github, pulling changes..."); - state.git.pull()?; + state.git.pull().context("Failed during automatic pull triggered by GitHub push event")?; } Ok(()) diff --git a/backend/src/handlers_prelude/repo_fs.rs b/backend/src/handlers_prelude/repo_fs.rs index 2563f466..6422202f 100644 --- a/backend/src/handlers_prelude/repo_fs.rs +++ b/backend/src/handlers_prelude/repo_fs.rs @@ -11,7 +11,7 @@ use axum::{ }; use reqwest::header::{CONTENT_DISPOSITION, CONTENT_TYPE}; use serde::{Deserialize, Serialize}; -use tracing::{error, warn}; +use tracing::error; use crate::handlers_prelude::ApiError; use crate::{perms::Permission, require_perms, AppState}; @@ -27,13 +27,12 @@ pub struct GetDocResponse { } async fn get_gh_token(state: &AppState) -> Result { - match state.gh_client.get_token().await { - Ok(token) => Ok(token), - Err(e) => { - error!("Failed to retrieve GitHub token: {e}"); - Err(ApiError::from((StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))) - } - } + let token = state + .gh_client + .get_token() + .await?; + + Ok(token) } /// This handler accepts a `GET` request to `/api/doc?path=`. @@ -51,7 +50,7 @@ pub async fn get_doc_handler( |doc| Ok(Json(GetDocResponse { contents: doc })), ), Err(e) => { - warn!( + error!( "Failed to fetch doc with path: {:?}; error: {:?}", query.path, e ); @@ -122,19 +121,20 @@ pub async fn delete_doc_handler( ) .await?; - state - .git - .delete_doc( - &query.path, - &format!("{} deleted {}", author.username, query.path), - &get_gh_token(&state).await?, - ) - .map_err(|e| { + match state.git.delete_doc( + &query.path, + &format!("{} deleted {}", author.username, query.path), + &get_gh_token(&state).await?, + ) { + Ok(_) => Ok(StatusCode::NO_CONTENT), + Err(e) => { error!("Failed to delete doc: {:?}", e); - ApiError::from((StatusCode::INTERNAL_SERVER_ERROR, e.to_string())) - })?; - - Ok(StatusCode::NO_CONTENT) + Err(ApiError::from(( + StatusCode::INTERNAL_SERVER_ERROR, + "An error occurred while deleting the document. Please check the server logs.", + ))) + } + } } /// This handler reads the document folder and builds a tree style object From 3eea8ae44fcb5ad59308ae43946723b842ea95ea Mon Sep 17 00:00:00 2001 From: TheKrol Date: Tue, 4 Mar 2025 10:21:42 -0500 Subject: [PATCH 9/9] Update errors --- backend/src/handlers_prelude/mod.rs | 1 - backend/src/handlers_prelude/repo_fs.rs | 114 +++++++----------------- 2 files changed, 34 insertions(+), 81 deletions(-) diff --git a/backend/src/handlers_prelude/mod.rs b/backend/src/handlers_prelude/mod.rs index 668f9eb7..d3284b5f 100644 --- a/backend/src/handlers_prelude/mod.rs +++ b/backend/src/handlers_prelude/mod.rs @@ -1,7 +1,6 @@ //! All Axum handlers are exported from this module use std::collections::HashMap; - use axum::response::{IntoResponse, Response}; use axum::{extract::State, http::HeaderMap}; use chrono::{DateTime, Utc}; diff --git a/backend/src/handlers_prelude/repo_fs.rs b/backend/src/handlers_prelude/repo_fs.rs index 6422202f..b44404a9 100644 --- a/backend/src/handlers_prelude/repo_fs.rs +++ b/backend/src/handlers_prelude/repo_fs.rs @@ -11,7 +11,6 @@ use axum::{ }; use reqwest::header::{CONTENT_DISPOSITION, CONTENT_TYPE}; use serde::{Deserialize, Serialize}; -use tracing::error; use crate::handlers_prelude::ApiError; use crate::{perms::Permission, require_perms, AppState}; @@ -41,27 +40,16 @@ pub async fn get_doc_handler( State(state): State, Query(query): Query, ) -> Result, ApiError> { - match state.git.get_doc(&query.path) { - Ok(maybe_doc) => maybe_doc.map_or( - Err(ApiError::from(( - StatusCode::NOT_FOUND, - "The file at the provided path was not found.".to_string(), - ))), - |doc| Ok(Json(GetDocResponse { contents: doc })), - ), - Err(e) => { - error!( - "Failed to fetch doc with path: {:?}; error: {:?}", - query.path, e - ); - Err(ApiError::from(( - StatusCode::INTERNAL_SERVER_ERROR, - "Fetch failed, check server logs for more info".to_string(), - ))) - } - } + let maybe_doc = state.git.get_doc(&query.path)?; + + let doc = maybe_doc.ok_or_else(|| { + ApiError::from("The file at the provided path was not found.".to_string()) + })?; + + Ok(Json(GetDocResponse { contents: doc })) } + #[derive(Serialize, Deserialize)] pub struct PutDocRequestBody { contents: String, @@ -90,22 +78,15 @@ pub async fn put_doc_handler( // Use the branch name from the request body let branch_name = &body.branch_name; - match state.git.put_doc( + state.git.put_doc( &body.path, &body.contents, &final_commit_message, &get_gh_token(&state).await?, branch_name, - ) { - Ok(_) => Ok(StatusCode::CREATED), - Err(e) => { - error!("Failed to complete put_doc call with error: {e:?}"); - Err(ApiError::from(( - StatusCode::INTERNAL_SERVER_ERROR, - "Failed to create document, check server logs for more info".to_string(), - ))) - } - } + )?; + + Ok(StatusCode::CREATED) } /// Deletes the document at the provided path, if the user has perms. @@ -114,62 +95,37 @@ pub async fn delete_doc_handler( headers: HeaderMap, Query(query): Query, ) -> Result { - let author = require_perms( - axum::extract::State(&state), - headers, - &[Permission::ManageContent], - ) - .await?; + let author = require_perms(axum::extract::State(&state), headers, &[Permission::ManageContent]) + .await?; - match state.git.delete_doc( + state.git.delete_doc( &query.path, &format!("{} deleted {}", author.username, query.path), &get_gh_token(&state).await?, - ) { - Ok(_) => Ok(StatusCode::NO_CONTENT), - Err(e) => { - error!("Failed to delete doc: {:?}", e); - Err(ApiError::from(( - StatusCode::INTERNAL_SERVER_ERROR, - "An error occurred while deleting the document. Please check the server logs.", - ))) - } - } + )?; + + Ok(StatusCode::NO_CONTENT) } + /// This handler reads the document folder and builds a tree style object /// representing the state of the tree. This is used in the viewer for directory navigation. pub async fn get_doc_tree_handler( State(state): State, ) -> Result, ApiError> { - match state.git.get_doc_tree() { - Ok(t) => Ok(Json(t)), - Err(e) => { - error!("An error was encountered fetching the document tree: {e:?}"); - Err(ApiError::from(( - StatusCode::INTERNAL_SERVER_ERROR, - "An internal error was encountered fetching the doc tree, check server logs for more info", - ))) - } - } + let tree = state.git.get_doc_tree()?; + + Ok(Json(tree)) } /// This handler reads the assets folder and builds a tree style object /// representing the state of the tree. This is used in the viewer for directory navigation. pub async fn get_asset_tree_handler( State(state): State, -) -> Result, (StatusCode, &'static str)> { - match state.git.get_asset_tree() { - Ok(t) => Ok(Json(t)), - Err(e) => { - error!("An error was encountered fetching the asset tree: {e:?}"); - Err(( - StatusCode::INTERNAL_SERVER_ERROR, - "An internal error was encountered fetching the asset tree, \ - check server logs for more info", - )) - } - } +) -> Result, ApiError> { + let tree = state.git.get_asset_tree()?; + + Ok(Json(tree)) } /// This handler fetches an asset from the repo's asset folder @@ -180,16 +136,13 @@ pub async fn get_asset_handler( let file_name = path.last().unwrap().clone(); let path = path.join("/"); - // Attempt to retrieve the asset, returning an ApiError on failure - let file = match state.git.get_asset(&path)? { - Some(file) => file, - None => { - return Err(ApiError::from(( - StatusCode::NOT_FOUND, - format!("File not found: {}", path), - ))); - } - }; + let file = state + .git + .get_asset(&path)? + .ok_or_else(|| ApiError::from(( + StatusCode::NOT_FOUND, + format!("File not found: {}", path), + )))?; let mut headers = HeaderMap::new(); headers.insert( @@ -207,6 +160,7 @@ pub async fn get_asset_handler( } + /// This handler creates or replaces the asset at the provided path /// with a new asset pub async fn put_asset_handler(