Refactor: Migrate to thiserror for RESTError and QueryError

Author: lowhung

common/src/queries/errors.rs

@@ -1,22 +1,27 @@
 use serde::{Deserialize, Serialize};
-use std::fmt;
+use thiserror::Error;
 
 /// Common error type for all state query responses
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Error, Serialize, Deserialize)]
 pub enum QueryError {
     /// The requested resource was not found
+    #[error("Not found: {resource}")]
     NotFound { resource: String },
 
     /// An error occurred while processing the query
+    #[error("Internal error: {message}")]
     Internal { message: String },
 
     /// Storage backend is disabled in configuration
+    #[error("{storage_type} storage is not enabled")]
     StorageDisabled { storage_type: String },
 
     /// Invalid request parameters
+    #[error("Invalid request: {message}")]
     InvalidRequest { message: String },
 
     /// Query variant is not implemented yet
+    #[error("Query not implemented: {query}")]
     NotImplemented { query: String },
 }
 
@@ -51,19 +56,3 @@ impl QueryError {
         }
     }
 }
-
-impl fmt::Display for QueryError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            Self::NotFound { resource } => write!(f, "Not found: {}", resource),
-            Self::Internal { message } => write!(f, "Query failed: {}", message),
-            Self::StorageDisabled { storage_type } => {
-                write!(f, "{} storage is not enabled", storage_type)
-            }
-            Self::InvalidRequest { message } => write!(f, "Invalid request: {}", message),
-            Self::NotImplemented { query } => write!(f, "Query not implemented: {}", query),
-        }
-    }
-}
-
-impl std::error::Error for QueryError {}

common/src/queries/utils.rs

@@ -6,8 +6,6 @@ use crate::messages::{Message, RESTResponse};
 use crate::queries::errors::QueryError;
 use crate::rest_error::RESTError;
 
-/// Query state and get typed result or QueryError
-/// This is the low-level building block for handlers that need to do more processing
 pub async fn query_state<T, F>(
     context: &Arc<Context<Message>>,
     topic: &str,
@@ -21,7 +19,7 @@ where
         .message_bus
         .request(topic, request_msg)
         .await
-        .map_err(|e| QueryError::internal_error(e.to_string()))?;
+        .map_err(|e| QueryError::internal_error(format!("Failed to query '{topic}': {e:#}")))?;
 
     let message = Arc::try_unwrap(raw_msg).unwrap_or_else(|arc| (*arc).clone());
 

common/src/rest_error.rs

@@ -1,14 +1,21 @@
 use crate::queries::errors::QueryError;
 use anyhow::Error as AnyhowError;
 use caryatid_module_rest_server::messages::RESTResponse;
-use std::fmt;
+use thiserror::Error;
 
-/// Standard error types for the application
-#[derive(Debug)]
+/// Standard REST error types
+#[derive(Debug, Error)]
 pub enum RESTError {
+    #[error("{0}")]
     BadRequest(String),
+
+    #[error("{0}")]
     NotFound(String),
+
+    #[error("{0}")]
     InternalServerError(String),
+
+    #[error("{0}")]
     NotImplemented(String),
 }
 
@@ -24,12 +31,12 @@ impl RESTError {
     }
 
     /// Get the error message
-    pub fn message(&self) -> String {
+    pub fn message(&self) -> &str {
         match self {
-            RESTError::BadRequest(msg) => msg.clone(),
-            RESTError::NotFound(msg) => msg.clone(),
-            RESTError::InternalServerError(msg) => msg.clone(),
-            RESTError::NotImplemented(msg) => msg.clone(),
+            RESTError::BadRequest(msg) => msg,
+            RESTError::NotFound(msg) => msg,
+            RESTError::InternalServerError(msg) => msg,
+            RESTError::NotImplemented(msg) => msg,
         }
     }
 
@@ -79,18 +86,10 @@ impl RESTError {
     }
 }
 
-impl fmt::Display for RESTError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.message())
-    }
-}
-
-impl std::error::Error for RESTError {}
-
 /// Convert RESTError to RESTResponse
 impl From<RESTError> for RESTResponse {
     fn from(error: RESTError) -> Self {
-        RESTResponse::with_text(error.status_code(), &error.message())
+        RESTResponse::with_text(error.status_code(), error.message())
     }
 }
 
@@ -101,13 +100,6 @@ impl From<AnyhowError> for RESTError {
     }
 }
 
-// /// Convert ParseIntError to RESTError (400 Bad Request)
-// impl From<ParseIntError> for RESTError {
-//     fn from(error: ParseIntError) -> Self {
-//         RESTError::BadRequest(error.to_string())
-//     }
-// }
-
 /// Convert hex decode errors to RESTError (400 Bad Request)
 impl From<hex::FromHexError> for RESTError {
     fn from(error: hex::FromHexError) -> Self {