From 8af3ff10e2347da70c35eb45046f8a04843f7256 Mon Sep 17 00:00:00 2001 From: boastful-squirrel Date: Sun, 21 Apr 2019 16:27:34 +0200 Subject: Rework error system + avoid panics in main() --- src/file_upload.rs | 57 +++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 18 deletions(-) (limited to 'src/file_upload.rs') diff --git a/src/file_upload.rs b/src/file_upload.rs index 534083c..960f831 100644 --- a/src/file_upload.rs +++ b/src/file_upload.rs @@ -1,5 +1,3 @@ -use crate::errors::FileUploadErrorKind; -use crate::renderer::file_upload_error; use actix_web::{ dev, http::header, multipart, FromRequest, FutureResponse, HttpMessage, HttpRequest, HttpResponse, Query, @@ -12,6 +10,9 @@ use std::{ path::{Component, PathBuf}, }; +use crate::errors::ContextualErrorKind; +use crate::renderer::file_upload_error; + /// Query parameters #[derive(Debug, Deserialize)] struct QueryParameters { @@ -23,24 +24,32 @@ fn save_file( field: multipart::Field, file_path: PathBuf, overwrite_files: bool, -) -> Box> { +) -> Box> { if !overwrite_files && file_path.exists() { - return Box::new(future::err(FileUploadErrorKind::FileExist)); + return Box::new(future::err(ContextualErrorKind::CustomError( + "File already exists, and the overwrite_files option has not been set".to_string(), + ))); } + let mut file = match std::fs::File::create(file_path) { Ok(file) => file, Err(e) => { - return Box::new(future::err(FileUploadErrorKind::IOError(e))); + return Box::new(future::err(ContextualErrorKind::IOError( + "Failed to create file".to_string(), + e, + ))); } }; Box::new( field - .map_err(FileUploadErrorKind::MultipartError) + .map_err(ContextualErrorKind::MultipartError) .fold(0i64, move |acc, bytes| { let rt = file .write_all(bytes.as_ref()) .map(|_| acc + bytes.len() as i64) - .map_err(FileUploadErrorKind::IOError); + .map_err(|e| { + ContextualErrorKind::IOError("Failed to write to file".to_string(), e) + }); future::result(rt) }), ) @@ -51,44 +60,56 @@ fn handle_multipart( item: multipart::MultipartItem, mut file_path: PathBuf, overwrite_files: bool, -) -> Box> { +) -> Box> { match item { multipart::MultipartItem::Field(field) => { let filename = field .headers() .get(header::CONTENT_DISPOSITION) - .ok_or(FileUploadErrorKind::ParseError) + .ok_or(ContextualErrorKind::ParseError) .and_then(|cd| { header::ContentDisposition::from_raw(cd) - .map_err(|_| FileUploadErrorKind::ParseError) + .map_err(|_| ContextualErrorKind::ParseError) }) .and_then(|content_disposition| { content_disposition .get_filename() - .ok_or(FileUploadErrorKind::ParseError) + .ok_or(ContextualErrorKind::ParseError) .map(String::from) }); - let err = |e: FileUploadErrorKind| Box::new(future::err(e).into_stream()); + let err = |e: ContextualErrorKind| Box::new(future::err(e).into_stream()); match filename { Ok(f) => { match fs::metadata(&file_path) { Ok(metadata) => { - if !metadata.is_dir() || metadata.permissions().readonly() { - return err(FileUploadErrorKind::InsufficientPermissions); + if !metadata.is_dir() { + return err(ContextualErrorKind::InvalidPathError(format!( + "cannot upload file to {}, since it's not a directory", + &file_path.display() + ))); + } else if metadata.permissions().readonly() { + return err(ContextualErrorKind::InsufficientPermissionsError( + file_path.display().to_string(), + )); } } Err(_) => { - return err(FileUploadErrorKind::InsufficientPermissions); + return err(ContextualErrorKind::InsufficientPermissionsError( + file_path.display().to_string(), + )); } } file_path = file_path.join(f); Box::new(save_file(field, file_path, overwrite_files).into_stream()) } - Err(e) => err(e), + Err(e) => err(e( + "HTTP header".to_string(), + "Failed to retrieve the name of the file to upload".to_string(), + )), } } multipart::MultipartItem::Nested(mp) => Box::new( - mp.map_err(FileUploadErrorKind::MultipartError) + mp.map_err(ContextualErrorKind::MultipartError) .map(move |item| handle_multipart(item, file_path.clone(), overwrite_files)) .flatten(), ), @@ -134,7 +155,7 @@ pub fn upload_file(req: &HttpRequest) -> FutureResponse< let overwrite_files = req.state().overwrite_files; Box::new( req.multipart() - .map_err(FileUploadErrorKind::MultipartError) + .map_err(ContextualErrorKind::MultipartError) .map(move |item| handle_multipart(item, target_dir.clone(), overwrite_files)) .flatten() .collect() -- cgit v1.2.3 From 23567f3b73ef309ba9afda51e084751b64942c53 Mon Sep 17 00:00:00 2001 From: boastful-squirrel Date: Sun, 21 Apr 2019 19:21:59 +0200 Subject: Print upload/archive errors also in terminal --- src/file_upload.rs | 1 + 1 file changed, 1 insertion(+) (limited to 'src/file_upload.rs') diff --git a/src/file_upload.rs b/src/file_upload.rs index 960f831..54c56dd 100644 --- a/src/file_upload.rs +++ b/src/file_upload.rs @@ -175,6 +175,7 @@ fn create_error_response( description: &str, return_path: &str, ) -> FutureResult { + log::error!("{}", description); future::ok( HttpResponse::BadRequest() .content_type("text/html; charset=utf-8") -- cgit v1.2.3 From 6dad3eb1bf0cb3b36cdb3b312cca7caa91de2f57 Mon Sep 17 00:00:00 2001 From: boastful-squirrel Date: Mon, 22 Apr 2019 00:39:38 +0200 Subject: Properly log error + added render_error method --- src/file_upload.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'src/file_upload.rs') diff --git a/src/file_upload.rs b/src/file_upload.rs index 54c56dd..3b84721 100644 --- a/src/file_upload.rs +++ b/src/file_upload.rs @@ -10,8 +10,8 @@ use std::{ path::{Component, PathBuf}, }; -use crate::errors::ContextualErrorKind; -use crate::renderer::file_upload_error; +use crate::errors::{self, ContextualErrorKind}; +use crate::renderer; /// Query parameters #[derive(Debug, Deserialize)] @@ -31,11 +31,11 @@ fn save_file( ))); } - let mut file = match std::fs::File::create(file_path) { + let mut file = match std::fs::File::create(&file_path) { Ok(file) => file, Err(e) => { return Box::new(future::err(ContextualErrorKind::IOError( - "Failed to create file".to_string(), + format!("Failed to create file in {}", file_path.display()), e, ))); } @@ -175,10 +175,10 @@ fn create_error_response( description: &str, return_path: &str, ) -> FutureResult { - log::error!("{}", description); + errors::log_error_chain(description.to_string()); future::ok( HttpResponse::BadRequest() .content_type("text/html; charset=utf-8") - .body(file_upload_error(description, return_path).into_string()), + .body(renderer::render_error(description, return_path).into_string()), ) } -- cgit v1.2.3