From 2e7ceb3f4a630f8b77277e630cf8bf4f639bc248 Mon Sep 17 00:00:00 2001 From: Sergey Mitskevich Date: Fri, 3 Aug 2018 20:28:35 +0300 Subject: [PATCH] Implemented .cause() for InternalError's Fail trait. This require Fail on InternalError's content. --- src/context.rs | 4 +- src/error.rs | 127 +++++++++++++++++++++++++--------- src/extractor.rs | 25 +++---- src/middleware/errhandlers.rs | 4 +- src/ws/context.rs | 2 +- tests/test_middleware.rs | 8 +-- 6 files changed, 117 insertions(+), 53 deletions(-) diff --git a/src/context.rs b/src/context.rs index 71a5af2d8..6222b9398 100644 --- a/src/context.rs +++ b/src/context.rs @@ -238,7 +238,9 @@ where if self.fut.alive() { match self.fut.poll() { Ok(Async::NotReady) | Ok(Async::Ready(())) => (), - Err(_) => return Err(ErrorInternalServerError("error")), + Err(_) => { + return Err(ErrorInternalServerError(::error::FailMsg("error"))) + } } } diff --git a/src/error.rs b/src/error.rs index 76c8e79ec..cc3df2868 100644 --- a/src/error.rs +++ b/src/error.rs @@ -633,6 +633,35 @@ impl ResponseError for StaticFileError { } } +/// Helper type for making `Fail` instance from `&'static str` without memory allocation. +/// +/// In following example handler is returning error message with *BAD REQUEST* status code. +/// +/// ```rust +/// # extern crate actix_web; +/// # use actix_web::*; +/// +/// fn index(req: HttpRequest) -> Result<()> { +/// Err(error::ErrorBadRequest(error::FailMsg("example of error message"))) +/// } +/// # fn main() {} +/// ``` +pub struct FailMsg(pub &'static str); + +impl Fail for FailMsg {} + +impl fmt::Debug for FailMsg { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Debug::fmt(&self.0, f) + } +} + +impl fmt::Display for FailMsg { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Display::fmt(&self.0, f) + } +} + /// Helper type that can wrap any error and generate custom response. /// /// In following example any `io::Error` will be converted into "BAD REQUEST" @@ -684,8 +713,12 @@ impl InternalError { impl Fail for InternalError where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { + fn cause(&self) -> Option<&Fail> { + self.cause.cause() + } + fn backtrace(&self) -> Option<&Backtrace> { Some(&self.backtrace) } @@ -711,7 +744,7 @@ where impl ResponseError for InternalError where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { fn error_response(&self) -> HttpResponse { match self.status { @@ -729,7 +762,7 @@ where impl Responder for InternalError where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { type Item = HttpResponse; type Error = Error; @@ -744,7 +777,7 @@ where #[allow(non_snake_case)] pub fn ErrorBadRequest(err: T) -> Error where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { InternalError::new(err, StatusCode::BAD_REQUEST).into() } @@ -754,7 +787,7 @@ where #[allow(non_snake_case)] pub fn ErrorUnauthorized(err: T) -> Error where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { InternalError::new(err, StatusCode::UNAUTHORIZED).into() } @@ -764,7 +797,7 @@ where #[allow(non_snake_case)] pub fn ErrorForbidden(err: T) -> Error where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { InternalError::new(err, StatusCode::FORBIDDEN).into() } @@ -774,7 +807,7 @@ where #[allow(non_snake_case)] pub fn ErrorNotFound(err: T) -> Error where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { InternalError::new(err, StatusCode::NOT_FOUND).into() } @@ -784,7 +817,7 @@ where #[allow(non_snake_case)] pub fn ErrorMethodNotAllowed(err: T) -> Error where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { InternalError::new(err, StatusCode::METHOD_NOT_ALLOWED).into() } @@ -794,7 +827,7 @@ where #[allow(non_snake_case)] pub fn ErrorRequestTimeout(err: T) -> Error where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { InternalError::new(err, StatusCode::REQUEST_TIMEOUT).into() } @@ -804,7 +837,7 @@ where #[allow(non_snake_case)] pub fn ErrorConflict(err: T) -> Error where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { InternalError::new(err, StatusCode::CONFLICT).into() } @@ -814,7 +847,7 @@ where #[allow(non_snake_case)] pub fn ErrorGone(err: T) -> Error where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { InternalError::new(err, StatusCode::GONE).into() } @@ -824,7 +857,7 @@ where #[allow(non_snake_case)] pub fn ErrorPreconditionFailed(err: T) -> Error where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { InternalError::new(err, StatusCode::PRECONDITION_FAILED).into() } @@ -834,7 +867,7 @@ where #[allow(non_snake_case)] pub fn ErrorExpectationFailed(err: T) -> Error where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { InternalError::new(err, StatusCode::EXPECTATION_FAILED).into() } @@ -844,7 +877,7 @@ where #[allow(non_snake_case)] pub fn ErrorInternalServerError(err: T) -> Error where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { InternalError::new(err, StatusCode::INTERNAL_SERVER_ERROR).into() } @@ -854,7 +887,7 @@ where #[allow(non_snake_case)] pub fn ErrorNotImplemented(err: T) -> Error where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { InternalError::new(err, StatusCode::NOT_IMPLEMENTED).into() } @@ -864,7 +897,7 @@ where #[allow(non_snake_case)] pub fn ErrorBadGateway(err: T) -> Error where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { InternalError::new(err, StatusCode::BAD_GATEWAY).into() } @@ -874,7 +907,7 @@ where #[allow(non_snake_case)] pub fn ErrorServiceUnavailable(err: T) -> Error where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { InternalError::new(err, StatusCode::SERVICE_UNAVAILABLE).into() } @@ -884,7 +917,7 @@ where #[allow(non_snake_case)] pub fn ErrorGatewayTimeout(err: T) -> Error where - T: Send + Sync + fmt::Debug + fmt::Display + 'static, + T: Fail, { InternalError::new(err, StatusCode::GATEWAY_TIMEOUT).into() } @@ -934,7 +967,7 @@ mod tests { #[test] fn test_backtrace() { - let e = ErrorBadRequest("err"); + let e = ErrorBadRequest(FailMsg("err")); let _ = e.backtrace(); } @@ -1034,6 +1067,25 @@ mod tests { assert_eq!(resp.status(), StatusCode::OK); } + #[test] + fn test_internal_error_cause() { + #[derive(Debug, Fail)] + #[fail(display = "demo error parent")] + struct DemoErrorParent; + + #[derive(Debug, Fail)] + #[fail(display = "demo error child")] + struct DemoErrorChild(#[cause] DemoErrorParent); + + let f = DemoErrorChild(DemoErrorParent); + let err = ErrorBadRequest(f); + assert_eq!("demo error child", format!("{}", err.as_fail())); + assert_eq!( + "demo error parent", + format!("{}", err.as_fail().cause().unwrap()) + ); + } + #[test] fn test_error_downcasting_direct() { #[derive(Debug, Fail)] @@ -1062,49 +1114,56 @@ mod tests { #[test] fn test_error_helpers() { - let r: HttpResponse = ErrorBadRequest("err").into(); + let r: HttpResponse = ErrorBadRequest(FailMsg("err")).into(); assert_eq!(r.status(), StatusCode::BAD_REQUEST); - let r: HttpResponse = ErrorUnauthorized("err").into(); + let r: HttpResponse = ErrorUnauthorized(FailMsg("err")).into(); assert_eq!(r.status(), StatusCode::UNAUTHORIZED); - let r: HttpResponse = ErrorForbidden("err").into(); + let r: HttpResponse = ErrorForbidden(FailMsg("err")).into(); assert_eq!(r.status(), StatusCode::FORBIDDEN); - let r: HttpResponse = ErrorNotFound("err").into(); + let r: HttpResponse = ErrorNotFound(FailMsg("err")).into(); assert_eq!(r.status(), StatusCode::NOT_FOUND); - let r: HttpResponse = ErrorMethodNotAllowed("err").into(); + let r: HttpResponse = ErrorMethodNotAllowed(FailMsg("err")).into(); assert_eq!(r.status(), StatusCode::METHOD_NOT_ALLOWED); - let r: HttpResponse = ErrorRequestTimeout("err").into(); + let r: HttpResponse = ErrorRequestTimeout(FailMsg("err")).into(); assert_eq!(r.status(), StatusCode::REQUEST_TIMEOUT); - let r: HttpResponse = ErrorConflict("err").into(); + let r: HttpResponse = ErrorConflict(FailMsg("err")).into(); assert_eq!(r.status(), StatusCode::CONFLICT); - let r: HttpResponse = ErrorGone("err").into(); + let r: HttpResponse = ErrorGone(FailMsg("err")).into(); assert_eq!(r.status(), StatusCode::GONE); - let r: HttpResponse = ErrorPreconditionFailed("err").into(); + let r: HttpResponse = ErrorPreconditionFailed(FailMsg("err")).into(); assert_eq!(r.status(), StatusCode::PRECONDITION_FAILED); - let r: HttpResponse = ErrorExpectationFailed("err").into(); + let r: HttpResponse = ErrorExpectationFailed(FailMsg("err")).into(); assert_eq!(r.status(), StatusCode::EXPECTATION_FAILED); - let r: HttpResponse = ErrorInternalServerError("err").into(); + let r: HttpResponse = ErrorInternalServerError(FailMsg("err")).into(); assert_eq!(r.status(), StatusCode::INTERNAL_SERVER_ERROR); - let r: HttpResponse = ErrorNotImplemented("err").into(); + let r: HttpResponse = ErrorNotImplemented(FailMsg("err")).into(); assert_eq!(r.status(), StatusCode::NOT_IMPLEMENTED); - let r: HttpResponse = ErrorBadGateway("err").into(); + let r: HttpResponse = ErrorBadGateway(FailMsg("err")).into(); assert_eq!(r.status(), StatusCode::BAD_GATEWAY); - let r: HttpResponse = ErrorServiceUnavailable("err").into(); + let r: HttpResponse = ErrorServiceUnavailable(FailMsg("err")).into(); assert_eq!(r.status(), StatusCode::SERVICE_UNAVAILABLE); - let r: HttpResponse = ErrorGatewayTimeout("err").into(); + let r: HttpResponse = ErrorGatewayTimeout(FailMsg("err")).into(); assert_eq!(r.status(), StatusCode::GATEWAY_TIMEOUT); } + + #[test] + fn test_error_helpers_from_std_err() { + let io_err = io::Error::new(io::ErrorKind::Other, "other"); + let err = ErrorNotImplemented(io_err); + assert_eq!("other", format!("{}", err)) + } } diff --git a/src/extractor.rs b/src/extractor.rs index 5c2c7f600..2b0a66ded 100644 --- a/src/extractor.rs +++ b/src/extractor.rs @@ -12,7 +12,7 @@ use serde::de::{self, DeserializeOwned}; use serde_urlencoded; use de::PathDeserializer; -use error::{Error, ErrorBadRequest, ErrorNotFound, UrlencodedError}; +use error::{Error, ErrorBadRequest, ErrorNotFound, FailMsg, UrlencodedError}; use handler::{AsyncResult, FromRequest}; use httpmessage::{HttpMessage, MessageBody, UrlEncoded}; use httprequest::HttpRequest; @@ -446,12 +446,13 @@ impl FromRequest for String { let enc: *const Encoding = encoding as *const Encoding; if enc == UTF_8 { Ok(str::from_utf8(body.as_ref()) - .map_err(|_| ErrorBadRequest("Can not decode body"))? - .to_owned()) + .map_err(|_| { + ErrorBadRequest(FailMsg("Can not decode body")) + })?.to_owned()) } else { - Ok(encoding - .decode(&body, DecoderTrap::Strict) - .map_err(|_| ErrorBadRequest("Can not decode body"))?) + Ok(encoding.decode(&body, DecoderTrap::Strict).map_err( + |_| ErrorBadRequest(FailMsg("Can not decode body")), + )?) } }), )) @@ -469,7 +470,7 @@ impl FromRequest for String { /// extern crate rand; /// #[macro_use] extern crate serde_derive; /// use actix_web::{http, App, Result, HttpRequest, Error, FromRequest}; -/// use actix_web::error::ErrorBadRequest; +/// use actix_web::error::{ErrorBadRequest, FailMsg}; /// /// #[derive(Debug, Deserialize)] /// struct Thing { name: String } @@ -483,7 +484,7 @@ impl FromRequest for String { /// if rand::random() { /// Ok(Thing { name: "thingy".into() }) /// } else { -/// Err(ErrorBadRequest("no luck")) +/// Err(ErrorBadRequest(FailMsg("no luck"))) /// } /// /// } @@ -531,7 +532,7 @@ where /// extern crate rand; /// #[macro_use] extern crate serde_derive; /// use actix_web::{http, App, Result, HttpRequest, Error, FromRequest}; -/// use actix_web::error::ErrorBadRequest; +/// use actix_web::error::{ErrorBadRequest, FailMsg}; /// /// #[derive(Debug, Deserialize)] /// struct Thing { name: String } @@ -545,7 +546,7 @@ where /// if rand::random() { /// Ok(Thing { name: "thingy".into() }) /// } else { -/// Err(ErrorBadRequest("no luck")) +/// Err(ErrorBadRequest(FailMsg("no luck"))) /// } /// /// } @@ -604,11 +605,11 @@ impl PayloadConfig { match req.mime_type() { Ok(Some(ref req_mt)) => { if mt != req_mt { - return Err(ErrorBadRequest("Unexpected Content-Type")); + return Err(ErrorBadRequest(FailMsg("Unexpected Content-Type"))); } } Ok(None) => { - return Err(ErrorBadRequest("Content-Type is expected")); + return Err(ErrorBadRequest(FailMsg("Content-Type is expected"))); } Err(err) => { return Err(err.into()); diff --git a/src/middleware/errhandlers.rs b/src/middleware/errhandlers.rs index 83c66aae1..04a182845 100644 --- a/src/middleware/errhandlers.rs +++ b/src/middleware/errhandlers.rs @@ -120,7 +120,9 @@ mod tests { impl Middleware for MiddlewareOne { fn start(&self, _: &HttpRequest) -> Result { - Err(ErrorInternalServerError("middleware error")) + Err(ErrorInternalServerError(::error::FailMsg( + "middleware error", + ))) } } diff --git a/src/ws/context.rs b/src/ws/context.rs index 4db83df5c..0401ee7eb 100644 --- a/src/ws/context.rs +++ b/src/ws/context.rs @@ -308,7 +308,7 @@ where fn poll(&mut self) -> Poll>, Error> { if self.fut.alive() && self.fut.poll().is_err() { - return Err(ErrorInternalServerError("error")); + return Err(ErrorInternalServerError(::error::FailMsg("error"))); } // frames diff --git a/tests/test_middleware.rs b/tests/test_middleware.rs index 4fa1c81da..4d9fe0b3b 100644 --- a/tests/test_middleware.rs +++ b/tests/test_middleware.rs @@ -8,7 +8,7 @@ use std::sync::Arc; use std::thread; use std::time::{Duration, Instant}; -use actix_web::error::{Error, ErrorInternalServerError}; +use actix_web::error::{Error, ErrorInternalServerError, FailMsg}; use actix_web::*; use futures::{future, Future}; use tokio_timer::Delay; @@ -332,7 +332,7 @@ fn test_scope_middleware_async_handler() { } fn index_test_middleware_async_error(_: &HttpRequest) -> FutureResponse { - future::result(Err(error::ErrorBadRequest("TEST"))).responder() + future::result(Err(error::ErrorBadRequest(FailMsg("TEST")))).responder() } #[test] @@ -794,7 +794,7 @@ struct MiddlewareWithErr; impl middleware::Middleware for MiddlewareWithErr { fn start(&self, _: &HttpRequest) -> Result { - Err(ErrorInternalServerError("middleware error")) + Err(ErrorInternalServerError(FailMsg("middleware error"))) } } @@ -803,7 +803,7 @@ struct MiddlewareAsyncWithErr; impl middleware::Middleware for MiddlewareAsyncWithErr { fn start(&self, _: &HttpRequest) -> Result { Ok(middleware::Started::Future(Box::new(future::err( - ErrorInternalServerError("middleware error"), + ErrorInternalServerError(FailMsg("middleware error")), )))) } }