From f68f2155c3b821f61ebff94eaaf449e2f19f65c2 Mon Sep 17 00:00:00 2001 From: ElnuDev Date: Sat, 30 Jul 2022 13:19:11 -0700 Subject: [PATCH] Improve error handling --- src/main.rs | 204 +++++++++++++++++++++++++--------------------------- 1 file changed, 99 insertions(+), 105 deletions(-) diff --git a/src/main.rs b/src/main.rs index 0e6a9ee..f397f37 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,7 +5,7 @@ pub use comment::*; mod database; pub use database::Database; -use actix_web::{get, post, web, App, HttpRequest, HttpResponse, HttpServer, Responder}; +use actix_web::{get, post, web, App, HttpRequest, HttpResponse, HttpServer}; use clap::Parser; use sanitize_html::{errors::SanitizeError, rules::predefined::DEFAULT, sanitize_str}; use scraper::{Html, Selector}; @@ -19,69 +19,68 @@ struct AppState { arguments: Arguments, } -// TODO: Make error handling system not bad. -// Currently, it's a horrible mix of custom error types a direct HttpResponses, -// due to not being able to pass HttpResponse out of web::block... -// Well, it works, at least. - -enum DatabaseAccessError { - BadOrigin, - AccessError, - DatabaseError, -} - -enum CommentCreationError { - DatabaseAccessError(DatabaseAccessError), - BadParent, -} - -impl DatabaseAccessError { - fn to_http_response(&self) -> HttpResponse { - match self { - Self::BadOrigin => HttpResponse::BadRequest().reason("bad origin").finish(), - Self::AccessError => HttpResponse::InternalServerError() - .reason("database access error") - .finish(), // e.g. PoisonError - Self::DatabaseError => HttpResponse::InternalServerError() - .reason("database error") - .finish(), - } - } +enum Error { + InvalidOrigin, + InvalidBody, + InvalidUrl, + InvalidFields, + InvalidContentId, + InvalidParent, + EmailRequired, + NameRequired, + DatabaseAccessError, + DatabaseInternalError, + SanitizationError, + PageFetchError, } -impl CommentCreationError { +impl Error { fn to_http_response(&self) -> HttpResponse { match self { - Self::DatabaseAccessError(error) => error.to_http_response(), - Self::BadParent => HttpResponse::BadRequest() - .reason("invalid comment parent") - .finish(), - } + Self::InvalidOrigin | + Self::InvalidBody | + Self::InvalidUrl | + Self::InvalidFields | + Self::InvalidContentId | + Self::InvalidParent | + Self::EmailRequired | + Self::NameRequired => HttpResponse::BadRequest(), + Self::DatabaseAccessError | + Self::DatabaseInternalError | + Self::SanitizationError | + Self::PageFetchError => HttpResponse::InternalServerError(), + }.reason(match self { + Self::InvalidOrigin => "invalid request origin", + Self::InvalidBody => "invalid request body", + Self::InvalidUrl => "invalid request url", + Self::InvalidFields => "invalid request field", + Self::InvalidContentId => "invalid request content id", + Self::InvalidParent => "invalid comment parent", + Self::EmailRequired => "comment email required", + Self::NameRequired => "comment name required", + Self::DatabaseAccessError => "database access error", + Self::DatabaseInternalError => "database internal error", + Self::SanitizationError => "comment sanitization error", + Self::PageFetchError => "page fetch error", + }).finish() } } impl AppState { fn get_db<'a>( - &'a self, - request: &HttpRequest, - ) -> Result, DatabaseAccessError> { - self.get_db_with_origin(get_request_origin(request)) - } - - fn get_db_with_origin<'a>( &'a self, origin: Option, - ) -> Result, DatabaseAccessError> { + ) -> Result, Error> { let origin = match origin { Some(origin) => origin, - None => return Err(DatabaseAccessError::BadOrigin), + None => return Err(Error::InvalidOrigin), }; match self.databases.get(&origin) { Some(database) => Ok(match database.lock() { Ok(database) => database, - Err(_) => return Err(DatabaseAccessError::AccessError), + Err(_) => return Err(Error::DatabaseAccessError), }), - None => return Err(DatabaseAccessError::AccessError), + None => return Err(Error::InvalidOrigin), } } } @@ -124,35 +123,40 @@ struct Arguments { email_required: bool, } -#[get("/{content_id}")] -async fn get_comments( +async fn _get_comments( data: web::Data, request: HttpRequest, content_id: web::Path, -) -> impl Responder { +) -> Result, Error> { let origin = get_request_origin(&request); - let comments = match web::block(move || { + match web::block(move || { Ok( - match match data.get_db_with_origin(origin) { + match match data.get_db(origin) { Ok(database) => database, Err(err) => return Err(err), } .get_comments(&content_id) { Ok(comments) => comments, - Err(_) => return Err(DatabaseAccessError::DatabaseError), + Err(_) => return Err(Error::DatabaseInternalError), }, ) - }) - .await - { - Ok(comments) => match comments { - Ok(comments) => comments, - Err(err) => return err.to_http_response(), - }, - Err(_) => return DatabaseAccessError::AccessError.to_http_response(), - }; - HttpResponse::Ok().json(comments) + }).await { + Ok(result) => result, + Err(_) => Err(Error::DatabaseAccessError), + } +} + +#[get("/{content_id}")] +async fn get_comments( + data: web::Data, + request: HttpRequest, + content_id: web::Path, +) -> HttpResponse { + match _get_comments(data, request, content_id).await { + Ok(comments) => HttpResponse::Ok().json(comments), + Err(err) => err.to_http_response(), + } } #[derive(Deserialize)] @@ -161,12 +165,11 @@ struct PostCommentsRequest { comment: Comment, } -#[post("/")] -async fn post_comment( +async fn _post_comment( data: web::Data, request: HttpRequest, bytes: web::Bytes, -) -> impl Responder { +) -> Result<(), Error> { match String::from_utf8(bytes.to_vec()) { Ok(text) => { let PostCommentsRequest { url, comment } = @@ -181,32 +184,26 @@ async fn post_comment( Ok(()) }; if let Err(_) = sanitize_req() { - return HttpResponse::InternalServerError() - .reason("failed to sanitize request") - .finish(); + return Err(Error::SanitizationError); } req } Err(_) => { - return HttpResponse::BadRequest() - .reason("invalid request body") - .finish() + return Err(Error::InvalidBody); } }; if comment.validate().is_err() { - return HttpResponse::BadRequest() - .reason("invalid comment field(s)") - .finish(); + return Err(Error::InvalidFields); } if comment.author.is_none() && data.arguments.name_required { - return HttpResponse::BadRequest().reason("name required").finish(); + return Err(Error::NameRequired); } if comment.email.is_none() && data.arguments.email_required { - return HttpResponse::BadRequest().reason("email required").finish(); + return Err(Error::EmailRequired); } let origin = match get_request_origin(&request) { Some(origin) => origin, - None => return HttpResponse::BadRequest().reason("bad origin").finish(), + None => return Err(Error::InvalidOrigin), }; // Check to see if provided URL is in scope. // This is to prevent malicious requests that try to get server to fetch external websites. @@ -218,31 +215,25 @@ async fn post_comment( break 'outer; } } - return HttpResponse::BadRequest() - .reason("url out of scope") - .finish(); + return Err(Error::InvalidUrl); } match get_page_data(&url).await { Ok(page_data_option) => match page_data_option { Some(page_data) => { if page_data.content_id != comment.content_id { - return HttpResponse::BadRequest() - .reason("content ids don't match") - .finish(); + return Err(Error::InvalidContentId) } } - None => return HttpResponse::BadRequest().reason("url invalid").finish(), // e.g. 404 + None => return Err(Error::InvalidUrl), // e.g. 404 }, Err(_) => { - return HttpResponse::InternalServerError() - .reason("failed to get page data") - .finish() + return Err(Error::PageFetchError); } }; match web::block(move || { - let database = match data.get_db_with_origin(Some(origin)) { + let database = match data.get_db(Some(origin)) { Ok(database) => database, - Err(err) => return Err(CommentCreationError::DatabaseAccessError(err)), + Err(err) => return Err(err), }; if let Some(parent) = comment.parent { 'outer2: loop { @@ -256,35 +247,38 @@ async fn post_comment( break; } } - } + }, Err(_) => { - return Err(CommentCreationError::DatabaseAccessError( - DatabaseAccessError::DatabaseError, - )) + return Err(Error::DatabaseInternalError); } }; - return Err(CommentCreationError::BadParent); + return Err(Error::InvalidParent); } } if let Err(_) = database.create_comment(&comment) { - return Err(CommentCreationError::DatabaseAccessError( - DatabaseAccessError::DatabaseError, - )); + return Err(Error::DatabaseInternalError); } Ok(()) }) .await { - Ok(result) => match result { - Ok(_) => HttpResponse::Ok().into(), - Err(error) => error.to_http_response(), - }, - Err(_) => DatabaseAccessError::AccessError.to_http_response(), + Ok(result) => result, + Err(_) => Err(Error::DatabaseAccessError), } } - Err(_) => HttpResponse::BadRequest() - .reason("failed to parse request body") - .finish(), + Err(_) => Err(Error::InvalidBody), + } +} + +#[post("/")] +async fn post_comment( + data: web::Data, + request: HttpRequest, + bytes: web::Bytes, +) -> HttpResponse { + match _post_comment(data, request, bytes).await { + Ok(_) => HttpResponse::Ok().finish(), + Err(err) => err.to_http_response(), } }