Use LemmyErrorType also make error_type compulsory

LemmyNet · Jul 10, 2023 · 60ce6f2 · 60ce6f2
1 parent 1026627
commit 60ce6f2
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 34 deletions.
diff --git a/crates/api_crud/src/site/create.rs b/crates/api_crud/src/site/create.rs
@@ -371,7 +371,7 @@ mod tests {
           }
           Err(error) => {
             assert!(
-              error.error_type.eq(&Some(expected_err.clone())),
+              error.error_type.eq(&expected_err.clone()),
               "Got Err {:?}, but should have failed with message: {} for reason: {}. invalid_payloads.nth({})",
               error.error_type,
               expected_err,

diff --git a/crates/api_crud/src/site/update.rs b/crates/api_crud/src/site/update.rs
@@ -373,7 +373,7 @@ mod tests {
           }
           Err(error) => {
             assert!(
-              error.error_type.eq(&Some(expected_err.clone())),
+              error.error_type.eq(&expected_err.clone()),
               "Got Err {:?}, but should have failed with message: {} for reason: {}. invalid_payloads.nth({})",
               error.error_type,
               expected_err,

diff --git a/crates/utils/src/error.rs b/crates/utils/src/error.rs
@@ -10,7 +10,7 @@ use ts_rs::TS;
 pub type LemmyResult<T> = Result<T, LemmyError>;
 
 pub struct LemmyError {
-  pub error_type: Option<LemmyErrorType>,
+  pub error_type: LemmyErrorType,
   pub inner: anyhow::Error,
   pub context: SpanTrace,
 }
@@ -20,9 +20,10 @@ where
   T: Into<anyhow::Error>,
 {
   fn from(t: T) -> Self {
+    let cause = t.into();
     LemmyError {
-      error_type: None,
-      inner: t.into(),
+      error_type: LemmyErrorType::Unknown(format!("{}", &cause)),
+      inner: cause,
       context: SpanTrace::capture(),
     }
   }
@@ -40,9 +41,7 @@ impl Debug for LemmyError {
 
 impl Display for LemmyError {
   fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-    if let Some(message) = &self.error_type {
-      write!(f, "{message}: ")?;
-    }
+    write!(f, "{}: ", &self.error_type)?;
     // print anyhow including trace
     // https://docs.rs/anyhow/latest/anyhow/struct.Error.html#display-representations
     // this will print the anyhow trace (only if it exists)
@@ -61,13 +60,7 @@ impl actix_web::error::ResponseError for LemmyError {
   }
 
   fn error_response(&self) -> actix_web::HttpResponse {
-    if let Some(message) = &self.error_type {
-      actix_web::HttpResponse::build(self.status_code()).json(message)
-    } else {
-      actix_web::HttpResponse::build(self.status_code())
-        .content_type("text/plain")
-        .body(self.inner.to_string())
-    }
+    actix_web::HttpResponse::build(self.status_code()).json(&self.error_type)
   }
 }
 
@@ -214,14 +207,14 @@ pub enum LemmyErrorType {
   CouldntCreateAudioCaptcha,
   InvalidUrlScheme,
   CouldntSendWebmention,
-  Unknown,
+  Unknown(String),
 }
 
 impl From<LemmyErrorType> for LemmyError {
   fn from(error_type: LemmyErrorType) -> Self {
     let inner = anyhow::anyhow!("{}", error_type);
     LemmyError {
-      error_type: Some(error_type),
+      error_type,
       inner,
       context: SpanTrace::capture(),
     }
@@ -235,7 +228,7 @@ pub trait LemmyErrorExt<T, E: Into<anyhow::Error>> {
 impl<T, E: Into<anyhow::Error>> LemmyErrorExt<T, E> for Result<T, E> {
   fn with_lemmy_type(self, error_type: LemmyErrorType) -> Result<T, LemmyError> {
     self.map_err(|error| LemmyError {
-      error_type: Some(error_type),
+      error_type,
       inner: error.into(),
       context: SpanTrace::capture(),
     })
@@ -248,7 +241,7 @@ pub trait LemmyErrorExt2<T> {
 impl<T> LemmyErrorExt2<T> for Result<T, LemmyError> {
   fn with_lemmy_type(self, error_type: LemmyErrorType) -> Result<T, LemmyError> {
     self.map_err(|mut e| {
-      e.error_type = Some(error_type);
+      e.error_type = error_type;
       e
     })
   }

diff --git a/crates/utils/src/response.rs b/crates/utils/src/response.rs
@@ -1,4 +1,4 @@
-use crate::error::{ApiError, LemmyError};
+use crate::error::{LemmyError, LemmyErrorType};
 use actix_web::{
   dev::ServiceResponse,
   http::header,
@@ -30,9 +30,7 @@ pub fn jsonify_plain_text_errors<BODY>(
     .expect("expected an error object in the response");
   let response = HttpResponse::build(res.status())
     .append_header(header::ContentType::json())
-    .json(ApiError {
-      error: error.to_string(),
-    });
+    .json(LemmyErrorType::Unknown(error.to_string()));
 
   let service_response = ServiceResponse::new(req, response);
   Ok(ErrorHandlerResponse::Response(
@@ -43,7 +41,7 @@ pub fn jsonify_plain_text_errors<BODY>(
 #[cfg(test)]
 mod tests {
   use super::*;
-  use crate::error::LemmyError;
+  use crate::error::{LemmyError, LemmyErrorType};
   use actix_web::{
     error::ErrorInternalServerError,
     middleware::ErrorHandlers,
@@ -66,29 +64,29 @@ mod tests {
   }
 
   #[actix_web::test]
-  async fn test_lemmy_errors_are_not_double_jsonified() {
+  async fn test_lemmy_errors_are_not_modified() {
     async fn lemmy_error_service() -> actix_web::Result<String, LemmyError> {
-      Err(LemmyError::from_message("Something the matter"))
+      Err(LemmyError::from(LemmyErrorType::EmailAlreadyExists))
     }
 
     check_for_jsonification(
       lemmy_error_service,
       StatusCode::BAD_REQUEST,
-      "{\"error\":\"Something the matter\"}",
+      "{\"error\":\"email_already_exists\"}",
     )
     .await;
   }
 
   #[actix_web::test]
-  async fn test_generic_errors_are_jsonified() {
+  async fn test_generic_errors_are_jsonified_as_unknown_errors() {
     async fn generic_error_service() -> actix_web::Result<String, Error> {
       Err(ErrorInternalServerError("This is not a LemmyError"))
     }
 
     check_for_jsonification(
       generic_error_service,
       StatusCode::INTERNAL_SERVER_ERROR,
-      "{\"error\":\"This is not a LemmyError\"}",
+      "{\"error\":\"unknown\",\"message\":\"This is not a LemmyError\"}",
     )
     .await;
   }
@@ -102,7 +100,7 @@ mod tests {
     check_for_jsonification(
       anyhow_error_service,
       StatusCode::BAD_REQUEST,
-      "{\"error\":\"This is the inner error\"}",
+      "{\"error\":\"unknown\",\"message\":\"This is the inner error\"}",
     )
     .await;
   }

diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs
@@ -434,7 +434,7 @@ mod tests {
           result
             .unwrap_err()
             .error_type
-            .eq(&Some(expected_err.clone())),
+            .eq(&expected_err.clone()),
           "Testing {}, expected error {}",
           invalid_name,
           expected_err
@@ -454,7 +454,7 @@ mod tests {
         && invalid_result
           .unwrap_err()
           .error_type
-          .eq(&Some(LemmyErrorType::BioLengthOverflow))
+          .eq(&LemmyErrorType::BioLengthOverflow)
     );
   }
 
@@ -478,7 +478,7 @@ mod tests {
         && invalid_result
           .unwrap_err()
           .error_type
-          .eq(&Some(LemmyErrorType::SiteDescriptionLengthOverflow))
+          .eq(&LemmyErrorType::SiteDescriptionLengthOverflow)
     );
   }
 
@@ -511,7 +511,7 @@ mod tests {
           result
             .unwrap_err()
             .error_type
-            .eq(&Some(expected_err.clone())),
+            .eq(&expected_err.clone()),
           "Testing regex {:?}, expected error {}",
           regex_str,
           expected_err