LemmyNet · Nutomic · Jul 10, 2023 · Jul 9, 2023 · Jul 10, 2023 · Jul 10, 2023
@@ -5,8 +5,8 @@ use std::{
 use tracing_error::SpanTrace;
 
 #[derive(serde::Serialize)]
-struct ApiError {
-  error: String,
+pub struct ApiError {
+  pub error: String,
 }
 
 pub type LemmyResult<T> = Result<T, LemmyError>;
@@ -108,14 +108,13 @@ impl actix_web::error::ResponseError for LemmyError {
   }
 
   fn error_response(&self) -> actix_web::HttpResponse {
-    if let Some(message) = &self.message {
-      actix_web::HttpResponse::build(self.status_code()).json(ApiError {
-        error: message.into(),
-      })
-    } else {
-      actix_web::HttpResponse::build(self.status_code())
-        .content_type("text/plain")
-        .body(self.inner.to_string())
-    }
+    let error_message = match &self.message {
+      Some(message) => message.into(),
+      None => format!("{:#}", self.inner),
+    };
+
+    actix_web::HttpResponse::build(self.status_code()).json(ApiError {
+      error: error_message.into(),
+    })
   }
 }
@@ -11,6 +11,7 @@ pub mod settings;
 pub mod claims;
 pub mod error;
 pub mod request;
+pub mod response;
 pub mod utils;
 pub mod version;
 

@@ -0,0 +1,123 @@
+use crate::error::{ApiError, LemmyError};
+use actix_web::{
+  dev::ServiceResponse,
+  http::header,
+  middleware::ErrorHandlerResponse,
+  HttpResponse,
+};
+
+pub fn jsonify_plain_text_errors<BODY>(
+  res: ServiceResponse<BODY>,
+) -> actix_web::Result<ErrorHandlerResponse<BODY>> {
+  let maybe_error = res.response().error();
+
+  // This function is only expected to be called for errors, so if there is no error, return
+  if maybe_error.is_none() {
+    return Ok(ErrorHandlerResponse::Response(res.map_into_left_body()));
+  }
+  // We're assuming that any LemmyError is already in JSON format, so we don't need to do anything
+  if maybe_error.unwrap().as_error::<LemmyError>().is_some() {
+    return Ok(ErrorHandlerResponse::Response(res.map_into_left_body()));
+  }
+
+  let (req, res) = res.into_parts();
+  let error = res.error().unwrap();
+  let response = HttpResponse::build(res.status())
+    .append_header(header::ContentType::json())
+    .json(ApiError {
+      error: error.to_string(),
+    });
+
+  let service_response = ServiceResponse::new(req, response);
+  Ok(ErrorHandlerResponse::Response(
+    service_response.map_into_right_body(),
+  ))
+}
+
+#[cfg(test)]
+mod tests {
+  use super::*;
+  use crate::error::LemmyError;
+  use actix_web::{
+    error::ErrorInternalServerError,
+    middleware::ErrorHandlers,
+    test,
+    web,
+    App,
+    Error,
+    Handler,
+    Responder,
+  };
+  use http::StatusCode;
+
+  #[actix_web::test]
+  async fn test_non_error_responses_are_not_modified() {
+    async fn ok_service() -> actix_web::Result<String, Error> {
+      Ok("Oll Korrect".to_string())
+    }
+
+    check_for_jsonification(ok_service, StatusCode::OK, "Oll Korrect").await;
+  }
+
+  #[actix_web::test]
+  async fn test_lemmy_errors_are_not_double_jsonified() {
+    async fn lemmy_error_service() -> actix_web::Result<String, LemmyError> {
+      Err(LemmyError::from_message("Something the matter"))
+    }
+
+    check_for_jsonification(
+      lemmy_error_service,
+      StatusCode::BAD_REQUEST,
+      "{\"error\":\"Something the matter\"}",
+    )
+    .await;
+  }
+
+  #[actix_web::test]
+  async fn test_generic_errors_are_jsonified() {
+    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\"}",
+    )
+    .await;
+  }
+
+  #[actix_web::test]
+  async fn test_anyhow_errors_wrapped_in_lemmy_errors_are_jsonified_correctly() {
+    async fn anyhow_error_service() -> actix_web::Result<String, LemmyError> {
+      Err(LemmyError::from(anyhow::anyhow!("This is the inner error")))
+    }
+
+    check_for_jsonification(
+      anyhow_error_service,
+      StatusCode::BAD_REQUEST,
+      "{\"error\":\"This is the inner error\"}",
+    )
+    .await;
+  }
+
+  async fn check_for_jsonification(
+    service: impl Handler<(), Output = impl Responder + 'static>,
+    expected_status_code: StatusCode,
+    expected_body: &str,
+  ) {
+    let app = test::init_service(
+      App::new()
+        .wrap(ErrorHandlers::new().default_handler(jsonify_plain_text_errors))
+        .route("/", web::get().to(service)),
+    )
+    .await;
+    let req = test::TestRequest::default().to_request();
+    let res = test::call_service(&app, req).await;
+
+    assert_eq!(res.status(), expected_status_code);
+
+    let body = test::read_body(res).await;
+    assert_eq!(body, expected_body);
+  }
+}
@@ -10,7 +10,13 @@ pub mod telemetry;
 use crate::{code_migrations::run_advanced_migrations, root_span_builder::QuieterRootSpanBuilder};
 use activitypub_federation::config::{FederationConfig, FederationMiddleware};
 use actix_cors::Cors;
-use actix_web::{middleware, web::Data, App, HttpServer, Result};
+use actix_web::{
+  middleware::{self, ErrorHandlers},
+  web::Data,
+  App,
+  HttpServer,
+  Result,
+};
 use lemmy_api_common::{
   context::LemmyContext,
   lemmy_db_views::structs::SiteView,
@@ -26,7 +32,12 @@ use lemmy_db_schema::{
   utils::{build_db_pool, get_database_url, run_migrations},
 };
 use lemmy_routes::{feeds, images, nodeinfo, webfinger};
-use lemmy_utils::{error::LemmyError, rate_limit::RateLimitCell, settings::SETTINGS};
+use lemmy_utils::{
+  error::LemmyError,
+  rate_limit::RateLimitCell,
+  response::jsonify_plain_text_errors,
+  settings::SETTINGS,
+};
 use reqwest::Client;
 use reqwest_middleware::ClientBuilder;
 use reqwest_tracing::TracingMiddleware;
@@ -176,6 +187,7 @@ pub async fn start_lemmy_server() -> Result<(), LemmyError> {
       .wrap(middleware::Compress::default())
       .wrap(cors_config)
       .wrap(TracingLogger::<QuieterRootSpanBuilder>::new())
+      .wrap(ErrorHandlers::new().default_handler(jsonify_plain_text_errors))
       .app_data(Data::new(context.clone()))
       .app_data(Data::new(rate_limit_cell.clone()))
       .wrap(FederationMiddleware::new(federation_config.clone()));