diff --git a/crates/application/src/diary/delete_review.rs b/crates/application/src/diary/delete_review.rs index cdc0d1f..9cdad14 100644 --- a/crates/application/src/diary/delete_review.rs +++ b/crates/application/src/diary/delete_review.rs @@ -17,7 +17,7 @@ pub async fn execute(ctx: &AppContext, cmd: DeleteReviewCommand) -> Result<(), D .ok_or_else(|| DomainError::NotFound(format!("review {}", cmd.review_id)))?; if review.user_id() != &requesting_user_id { - return Err(DomainError::Unauthorized("not your review".into())); + return Err(DomainError::Forbidden("not your review".into())); } let movie_id = review.movie_id().clone(); diff --git a/crates/application/src/integrations/confirm.rs b/crates/application/src/integrations/confirm.rs index 3be4a0f..acc66f1 100644 --- a/crates/application/src/integrations/confirm.rs +++ b/crates/application/src/integrations/confirm.rs @@ -25,7 +25,7 @@ pub async fn execute(ctx: &AppContext, cmd: ConfirmWatchEventsCommand) -> Result .ok_or_else(|| DomainError::NotFound(format!("WatchEvent {}", c.watch_event_id)))?; if event.user_id() != &user_id { - return Err(DomainError::Unauthorized("not your watch event".into())); + return Err(DomainError::Forbidden("not your watch event".into())); } let input = if let Some(movie_id) = event.movie_id() { diff --git a/crates/application/src/integrations/dismiss.rs b/crates/application/src/integrations/dismiss.rs index e13c611..8b6288b 100644 --- a/crates/application/src/integrations/dismiss.rs +++ b/crates/application/src/integrations/dismiss.rs @@ -27,7 +27,7 @@ pub async fn execute(ctx: &AppContext, cmd: DismissWatchEventsCommand) -> Result } for event in &events { if event.user_id() != &user_id { - return Err(DomainError::Unauthorized("not your watch event".into())); + return Err(DomainError::Forbidden("not your watch event".into())); } } diff --git a/crates/domain/src/errors.rs b/crates/domain/src/errors.rs index cfbcd15..611dca7 100644 --- a/crates/domain/src/errors.rs +++ b/crates/domain/src/errors.rs @@ -16,4 +16,7 @@ pub enum DomainError { #[error("Unauthorized: {0}")] Unauthorized(String), + + #[error("Forbidden: {0}")] + Forbidden(String), } diff --git a/crates/presentation/src/errors.rs b/crates/presentation/src/errors.rs index 099d7e3..968aec8 100644 --- a/crates/presentation/src/errors.rs +++ b/crates/presentation/src/errors.rs @@ -4,6 +4,28 @@ use axum::{ }; use domain::errors::DomainError; +pub fn domain_error_status(e: &DomainError) -> StatusCode { + match e { + DomainError::InvalidRating { .. } | DomainError::ValidationError(_) => { + StatusCode::BAD_REQUEST + } + DomainError::NotFound(_) => StatusCode::NOT_FOUND, + DomainError::Unauthorized(_) => StatusCode::UNAUTHORIZED, + DomainError::Forbidden(_) => StatusCode::FORBIDDEN, + DomainError::InfrastructureError(_) => StatusCode::INTERNAL_SERVER_ERROR, + } +} + +pub fn domain_error_response(e: DomainError) -> Response { + match &e { + DomainError::InfrastructureError(_) => { + tracing::error!("Internal error: {:?}", e); + StatusCode::INTERNAL_SERVER_ERROR.into_response() + } + _ => (domain_error_status(&e), e.to_string()).into_response(), + } +} + pub struct ApiError(pub DomainError); impl From for ApiError { @@ -14,20 +36,6 @@ impl From for ApiError { impl IntoResponse for ApiError { fn into_response(self) -> Response { - let (status, error_message) = match self.0 { - DomainError::InvalidRating { .. } => (StatusCode::BAD_REQUEST, self.0.to_string()), - DomainError::ValidationError(msg) => (StatusCode::BAD_REQUEST, msg), - DomainError::NotFound(msg) => (StatusCode::NOT_FOUND, msg), - DomainError::Unauthorized(msg) => (StatusCode::UNAUTHORIZED, msg), - DomainError::InfrastructureError(_) => { - tracing::error!("Internal Infrastructure Error: {:?}", self.0); - ( - StatusCode::INTERNAL_SERVER_ERROR, - "Internal server error".to_string(), - ) - } - }; - - (status, error_message).into_response() + domain_error_response(self.0) } } diff --git a/crates/presentation/src/handlers/api.rs b/crates/presentation/src/handlers/api.rs index 53a6a96..fc06c3e 100644 --- a/crates/presentation/src/handlers/api.rs +++ b/crates/presentation/src/handlers/api.rs @@ -37,7 +37,6 @@ use application::{ }, }; use domain::{ - errors::DomainError, models::{ExportFormat, PersonId, collections::PageParams}, services::review_history::Trend, value_objects::UserId, @@ -257,20 +256,13 @@ pub async fn delete_review( State(state): State, AuthenticatedUser(user_id): AuthenticatedUser, Path(review_id): Path, -) -> impl IntoResponse { +) -> Result { let cmd = DeleteReviewCommand { review_id, requesting_user_id: user_id.value(), }; - match delete_review::execute(&state.app_ctx, cmd).await { - Ok(()) => StatusCode::NO_CONTENT.into_response(), - Err(DomainError::NotFound(_)) => StatusCode::NOT_FOUND.into_response(), - Err(DomainError::Unauthorized(_)) => StatusCode::FORBIDDEN.into_response(), - Err(e) => { - tracing::error!("delete_review error: {:?}", e); - StatusCode::INTERNAL_SERVER_ERROR.into_response() - } - } + delete_review::execute(&state.app_ctx, cmd).await?; + Ok(StatusCode::NO_CONTENT) } #[utoipa::path( @@ -395,10 +387,7 @@ pub async fn get_movie_profile( }) .into_response(), Ok(None) => StatusCode::NOT_FOUND.into_response(), - Err(e) => { - tracing::error!("get_movie_profile: {:?}", e); - StatusCode::INTERNAL_SERVER_ERROR.into_response() - } + Err(e) => crate::errors::domain_error_response(e), } } @@ -414,27 +403,19 @@ pub async fn get_movie_profile( pub async fn get_profile( State(state): State, AuthenticatedUser(user_id): AuthenticatedUser, -) -> impl IntoResponse { - match application::users::get_current_profile::execute( +) -> Result, ApiError> { + let profile = application::users::get_current_profile::execute( &state.app_ctx, application::users::queries::GetCurrentProfileQuery { user_id: user_id.value(), }, ) - .await - { - Ok(profile) => Json(ProfileResponse { - username: profile.username, - bio: profile.bio, - avatar_url: profile.avatar_url, - }) - .into_response(), - Err(DomainError::NotFound(_)) => StatusCode::NOT_FOUND.into_response(), - Err(e) => { - tracing::error!("get_profile error: {:?}", e); - StatusCode::INTERNAL_SERVER_ERROR.into_response() - } - } + .await?; + Ok(Json(ProfileResponse { + username: profile.username, + bio: profile.bio, + avatar_url: profile.avatar_url, + })) } #[utoipa::path( @@ -513,14 +494,7 @@ pub async fn update_profile_handler( match update_profile::execute(&state.app_ctx, cmd).await { Ok(()) => StatusCode::NO_CONTENT.into_response(), - Err(domain::errors::DomainError::ValidationError(msg)) => { - tracing::warn!("update_profile validation: {}", msg); - StatusCode::BAD_REQUEST.into_response() - } - Err(e) => { - tracing::error!("update_profile error: {:?}", e); - StatusCode::INTERNAL_SERVER_ERROR.into_response() - } + Err(e) => crate::errors::domain_error_response(e), } } @@ -561,13 +535,7 @@ pub async fn update_profile_fields_handler( match update_profile_fields::execute(&state.app_ctx, cmd).await { Ok(()) => StatusCode::NO_CONTENT.into_response(), - Err(domain::errors::DomainError::ValidationError(msg)) => { - (StatusCode::BAD_REQUEST, msg).into_response() - } - Err(e) => { - tracing::error!("update_profile_fields error: {:?}", e); - StatusCode::INTERNAL_SERVER_ERROR.into_response() - } + Err(e) => crate::errors::domain_error_response(e), } } @@ -1034,8 +1002,7 @@ pub async fn get_user_profile( Ok(Some(u)) => u, Ok(None) => return StatusCode::NOT_FOUND.into_response(), Err(e) => { - tracing::error!("user lookup: {:?}", e); - return StatusCode::INTERNAL_SERVER_ERROR.into_response(); + return crate::errors::domain_error_response(e); } }; @@ -1054,10 +1021,7 @@ pub async fn get_user_profile( .await { Ok(p) => p, - Err(e) => { - tracing::error!("profile: {:?}", e); - return StatusCode::INTERNAL_SERVER_ERROR.into_response(); - } + Err(e) => return crate::errors::domain_error_response(e), }; let entries = profile.entries.map(|p| DiaryResponse { @@ -1236,10 +1200,7 @@ pub async fn get_search( }, }) .into_response(), - Err(e) => { - tracing::error!("search failed: {e}"); - StatusCode::INTERNAL_SERVER_ERROR.into_response() - } + Err(e) => crate::errors::domain_error_response(e), } } @@ -1266,10 +1227,7 @@ pub async fn get_person_handler( }) .into_response(), Ok(None) => StatusCode::NOT_FOUND.into_response(), - Err(e) => { - tracing::error!("get_person failed: {e}"); - StatusCode::INTERNAL_SERVER_ERROR.into_response() - } + Err(e) => crate::errors::domain_error_response(e), } } @@ -1320,11 +1278,7 @@ pub async fn get_person_credits_handler( .collect(), }) .into_response(), - Err(DomainError::NotFound(_)) => StatusCode::NOT_FOUND.into_response(), - Err(e) => { - tracing::error!("get_person_credits failed: {e}"); - StatusCode::INTERNAL_SERVER_ERROR.into_response() - } + Err(e) => crate::errors::domain_error_response(e), } } diff --git a/crates/presentation/src/handlers/html.rs b/crates/presentation/src/handlers/html.rs index 87e9211..b914ee9 100644 --- a/crates/presentation/src/handlers/html.rs +++ b/crates/presentation/src/handlers/html.rs @@ -301,12 +301,7 @@ pub async fn post_delete_review( .unwrap_or_else(|| "/".to_string()); Redirect::to(&redirect_url).into_response() } - Err(DomainError::NotFound(_)) => StatusCode::NOT_FOUND.into_response(), - Err(DomainError::Unauthorized(_)) => StatusCode::FORBIDDEN.into_response(), - Err(e) => { - tracing::error!("delete_review html error: {:?}", e); - StatusCode::INTERNAL_SERVER_ERROR.into_response() - } + Err(e) => crate::errors::domain_error_response(e), } } @@ -341,11 +336,7 @@ pub async fn get_export( bytes, ) .into_response(), - Err(DomainError::Unauthorized(_)) => StatusCode::FORBIDDEN.into_response(), - Err(e) => { - tracing::error!("export error: {:?}", e); - StatusCode::INTERNAL_SERVER_ERROR.into_response() - } + Err(e) => crate::errors::domain_error_response(e), } } @@ -408,7 +399,7 @@ pub async fn get_activity_feed( }) .into_response() } - Err(e) => (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()).into_response(), + Err(e) => crate::errors::domain_error_response(e), } } @@ -445,7 +436,7 @@ pub async fn get_users_list( }) .into_response() } - Err(e) => (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()).into_response(), + Err(e) => crate::errors::domain_error_response(e), } } @@ -521,8 +512,8 @@ pub async fn get_user_profile( .await { Ok(Some(u)) => u, - Ok(None) => return (StatusCode::NOT_FOUND, "User not found").into_response(), - Err(e) => return (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()).into_response(), + Ok(None) => return StatusCode::NOT_FOUND.into_response(), + Err(e) => return crate::errors::domain_error_response(e), }; let display_name = profile_user.username().value(); @@ -650,7 +641,7 @@ pub async fn get_user_profile( }) .into_response() } - Err(e) => (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()).into_response(), + Err(e) => crate::errors::domain_error_response(e), } } @@ -993,12 +984,7 @@ pub async fn get_movie_detail( ) .await { - Err(DomainError::NotFound(_)) => StatusCode::NOT_FOUND.into_response(), - Err(DomainError::ValidationError(_)) => StatusCode::BAD_REQUEST.into_response(), - Err(e) => { - tracing::error!("movie detail error: {:?}", e); - StatusCode::INTERNAL_SERVER_ERROR.into_response() - } + Err(e) => crate::errors::domain_error_response(e), Ok(result) => { let histogram_max = result .stats @@ -1062,10 +1048,7 @@ pub async fn get_watchlist_page( .await { Ok(r) => r, - Err(e) => { - tracing::error!("watchlist error: {:?}", e); - return StatusCode::INTERNAL_SERVER_ERROR.into_response(); - } + Err(e) => return crate::errors::domain_error_response(e), }; render_page(WatchlistTemplate { @@ -1151,10 +1134,7 @@ pub async fn post_watchlist_add( let url = format!("{}{}error={}", redirect_base, sep, encode_error(&msg)); Redirect::to(&url).into_response() } - Err(e) => { - tracing::error!("watchlist add error: {:?}", e); - StatusCode::INTERNAL_SERVER_ERROR.into_response() - } + Err(e) => crate::errors::domain_error_response(e), } } @@ -1184,10 +1164,7 @@ pub async fn post_watchlist_remove( .unwrap_or_else(|| "/".to_string()); Redirect::to(&redirect_url).into_response() } - Err(e) => { - tracing::error!("watchlist remove error: {:?}", e); - StatusCode::INTERNAL_SERVER_ERROR.into_response() - } + Err(e) => crate::errors::domain_error_response(e), } } @@ -1209,10 +1186,7 @@ pub async fn get_profile_settings( let user = match state.app_ctx.repos.user.find_by_id(&user_id).await { Ok(Some(u)) => u, Ok(None) => return StatusCode::NOT_FOUND.into_response(), - Err(e) => { - tracing::error!("get_profile_settings user lookup: {:?}", e); - return StatusCode::INTERNAL_SERVER_ERROR.into_response(); - } + Err(e) => return crate::errors::domain_error_response(e), }; let base_url = &state.app_ctx.config.base_url; diff --git a/crates/presentation/src/handlers/webhook.rs b/crates/presentation/src/handlers/webhook.rs index 2b5b6a7..335c8a8 100644 --- a/crates/presentation/src/handlers/webhook.rs +++ b/crates/presentation/src/handlers/webhook.rs @@ -128,12 +128,7 @@ async fn run_ingest( ) -> StatusCode { match ingest_watch_event::execute(&state.app_ctx, cmd, parser).await { Ok(()) => StatusCode::OK, - Err(domain::errors::DomainError::Unauthorized(_)) => StatusCode::UNAUTHORIZED, - Err(domain::errors::DomainError::ValidationError(_)) => StatusCode::BAD_REQUEST, - Err(e) => { - tracing::error!("webhook ingestion failed: {e:?}"); - StatusCode::INTERNAL_SERVER_ERROR - } + Err(e) => crate::errors::domain_error_status(&e), } }