Fix #3366: Wrap plain-text error responses from the API in JSON (#3559)

* Fix #3366: API does return plain HTML errors

* Fix Clippy errors

* Improve api response times by doing send_activity asynchronously (#3493)

* do send_activity after http response

* move to util function

* format

* fix prometheus

* make synchronous federation configurable

* cargo fmt

* empty

* empty

---------

Co-authored-by: Dessalines <dessalines@users.noreply.github.com>

* Updating `login.rs` with generic `incorrect_login` response. (#3549)

* Adding v0.18.1 and v0.18.0 release notes. (#3530)

* Update RELEASES.md (#3556)

added instruction to find the location of your docker directory (especially useful for those who used ansible since they never had to setup docker manually)

* Use async email sender (#3554)

* Upgrade all dependencies (#3526)

* Upgrade all dependencies

* as base64

* Adding phiresky to codeowners. (#3576)

* Error enum fixed (#3487)

* Create error type enum

* Replace magic string slices with LemmyErrorTypes

* Remove unused enum

* Add rename snake case to error enum

* Rename functions

* clippy

* Fix merge errors

* Serialize in PascalCase instead of snake_case

* Revert src/lib

* Add serialization tests

* Update translations

* Fix compilation error in test

* Fix another compilation error

* Add code for generating typescript types

* Various fixes to avoid breaking api

* impl From<LemmyErrorType> for LemmyError

* with_lemmy_type

* trigger ci

---------

Co-authored-by: SleeplessOne1917 <abias1122@gmail.com>

* Only update site_aggregates for local site (#3516)

* Fix #3501 - Fix aggregation counts for elements removed and deleted (#3543)

Two bugs were found and fixed:
- previously elements removal and deletion were counted as two separate disappearances
- removing comments did not affect post aggregations

* Use LemmyErrorType also make error_type compulsory

* Add missing import for jsonify_plain_text_errors

* Fix formatting

* Trying to make woodpecker run again

---------

Co-authored-by: phiresky <phireskyde+git@gmail.com>
Co-authored-by: Dessalines <dessalines@users.noreply.github.com>
Co-authored-by: rosenjcb <rosenjcb@gmail.com>
Co-authored-by: nixoye <12674582+nixoye@users.noreply.github.com>
Co-authored-by: dullbananas <dull.bananas0@gmail.com>
Co-authored-by: Nutomic <me@nutomic.com>
Co-authored-by: SleeplessOne1917 <abias1122@gmail.com>
Co-authored-by: Sander Saarend <sander@saarend.com>
Co-authored-by: Piotr Juszczyk <74842304+pijuszczyk@users.noreply.github.com>
pull/3572/head^2
Pawan Hegde 11 months ago committed by GitHub
parent 9c2490d4f2
commit ef9dc5d0b6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -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,

@ -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,

@ -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
})
}

@ -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,127 @@
use crate::error::{LemmyError, LemmyErrorType};
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
.expect("http responses with 400-599 statuses should have an error object")
.as_error::<LemmyError>()
.is_some()
{
return Ok(ErrorHandlerResponse::Response(res.map_into_left_body()));
}
let (req, res) = res.into_parts();
let error = res
.error()
.expect("expected an error object in the response");
let response = HttpResponse::build(res.status())
.append_header(header::ContentType::json())
.json(LemmyErrorType::Unknown(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, LemmyErrorType};
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_modified() {
async fn lemmy_error_service() -> actix_web::Result<String, LemmyError> {
Err(LemmyError::from(LemmyErrorType::EmailAlreadyExists))
}
check_for_jsonification(
lemmy_error_service,
StatusCode::BAD_REQUEST,
"{\"error\":\"email_already_exists\"}",
)
.await;
}
#[actix_web::test]
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\":\"unknown\",\"message\":\"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\":\"unknown\",\"message\":\"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);
}
}

@ -431,10 +431,7 @@ mod tests {
assert!(result.is_err());
assert!(
result
.unwrap_err()
.error_type
.eq(&Some(expected_err.clone())),
result.unwrap_err().error_type.eq(&expected_err.clone()),
"Testing {}, expected error {}",
invalid_name,
expected_err
@ -454,7 +451,7 @@ mod tests {
&& invalid_result
.unwrap_err()
.error_type
.eq(&Some(LemmyErrorType::BioLengthOverflow))
.eq(&LemmyErrorType::BioLengthOverflow)
);
}
@ -478,7 +475,7 @@ mod tests {
&& invalid_result
.unwrap_err()
.error_type
.eq(&Some(LemmyErrorType::SiteDescriptionLengthOverflow))
.eq(&LemmyErrorType::SiteDescriptionLengthOverflow)
);
}
@ -508,10 +505,7 @@ mod tests {
assert!(result.is_err());
assert!(
result
.unwrap_err()
.error_type
.eq(&Some(expected_err.clone())),
result.unwrap_err().error_type.eq(&expected_err.clone()),
"Testing regex {:?}, expected error {}",
regex_str,
expected_err

@ -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,
@ -29,6 +35,7 @@ use lemmy_routes::{feeds, images, nodeinfo, webfinger};
use lemmy_utils::{
error::LemmyError,
rate_limit::RateLimitCell,
response::jsonify_plain_text_errors,
settings::SETTINGS,
SYNCHRONOUS_FEDERATION,
};
@ -181,6 +188,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()));

Loading…
Cancel
Save