From 4b250cfe08181884032a97bc2d9da0fffd04a907 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Thu, 19 Sep 2024 15:21:15 +0200 Subject: [PATCH] Simplify handling of NotFound SQL errors (fixes #4633) --- crates/api_crud/src/post/read.rs | 5 +---- crates/db_schema/src/impls/post.rs | 4 ++++ crates/utils/src/error.rs | 17 ++++++++++++++++- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/crates/api_crud/src/post/read.rs b/crates/api_crud/src/post/read.rs index 4ecf24e0c..671dc4fd5 100644 --- a/crates/api_crud/src/post/read.rs +++ b/crates/api_crud/src/post/read.rs @@ -40,10 +40,7 @@ pub async fn get_post( }; // Check to see if the person is a mod or admin, to show deleted / removed - let community_id = Post::read(&mut context.pool(), post_id) - .await? - .ok_or(LemmyErrorType::CouldntFindPost)? - .community_id; + let community_id = Post::read_xx(&mut context.pool(), post_id).await?.community_id; let is_mod_or_admin = is_mod_or_admin_opt( &mut context.pool(), diff --git a/crates/db_schema/src/impls/post.rs b/crates/db_schema/src/impls/post.rs index 8e14bee9f..e736e0c9b 100644 --- a/crates/db_schema/src/impls/post.rs +++ b/crates/db_schema/src/impls/post.rs @@ -68,6 +68,10 @@ impl Crud for Post { } impl Post { + pub async fn read_xx(pool: &mut DbPool<'_>, id: PostId) -> Result { + let conn = &mut *get_conn(pool).await?; + post::table.find(id).first(conn).await + } pub async fn insert_apub( pool: &mut DbPool<'_>, timestamp: DateTime, diff --git a/crates/utils/src/error.rs b/crates/utils/src/error.rs index 1935e4132..528342709 100644 --- a/crates/utils/src/error.rs +++ b/crates/utils/src/error.rs @@ -211,8 +211,12 @@ cfg_if! { { fn from(t: T) -> Self { let cause = t.into(); + let error_type = match cause.downcast_ref::() { + Some(&diesel::NotFound) => LemmyErrorType::CouldntFindPost, + _ => LemmyErrorType::Unknown(format!("{}", &cause)) + }; LemmyError { - error_type: LemmyErrorType::Unknown(format!("{}", &cause)), + error_type, inner: cause, context: Backtrace::capture(), } @@ -323,6 +327,17 @@ cfg_if! { ) } + #[test] + fn test_convert_diesel_errors() { + let not_found_error = LemmyError::from(diesel::NotFound); + assert_eq!(LemmyErrorType::CouldntFindPost, not_found_error.error_type); + assert_eq!(404, not_found_error.status_code()); + + let other_error = LemmyError::from(diesel::result::Error::NotInTransaction); + assert!(matches!(other_error.error_type, LemmyErrorType::Unknown{..})); + assert_eq!(400, other_error.status_code()); + } + /// Check if errors match translations. Disabled because many are not translated at all. #[test] #[ignore]