diff --git a/crates/api_crud/src/site/create.rs b/crates/api_crud/src/site/create.rs index be8411e246..ed433ad9de 100644 --- 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 index 1e354d9cd4..d1820d177b 100644 --- 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 index cdb484722e..78590a6a74 100644 --- a/crates/utils/src/error.rs +++ b/crates/utils/src/error.rs @@ -10,7 +10,7 @@ use ts_rs::TS; pub type LemmyResult = Result; pub struct LemmyError { - pub error_type: Option, + pub error_type: LemmyErrorType, pub inner: anyhow::Error, pub context: SpanTrace, } @@ -20,9 +20,10 @@ where T: Into, { 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 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> { impl> LemmyErrorExt for Result { fn with_lemmy_type(self, error_type: LemmyErrorType) -> Result { 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 { impl LemmyErrorExt2 for Result { fn with_lemmy_type(self, error_type: LemmyErrorType) -> Result { 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 index 689c7d9c65..cc0b7c7074 100644 --- 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( .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( #[cfg(test)] mod tests { use super::*; - use crate::error::LemmyError; + use crate::error::{LemmyError, LemmyErrorType}; use actix_web::{ error::ErrorInternalServerError, middleware::ErrorHandlers, @@ -66,21 +64,21 @@ 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 { - 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 { Err(ErrorInternalServerError("This is not a LemmyError")) } @@ -88,7 +86,7 @@ mod tests { 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 index ec2d20b979..13d140c15a 100644 --- 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