refactor error handling

Cargo.toml

@@ -20,12 +20,6 @@ codecov = { repository = "fafhrd91/actix-web", branch = "master", service = "git
 name = "actix_web"
 path = "src/lib.rs"
 
-[features]
-default = []
-
-# Enable nightly features
-nightly = []
-
 [dependencies]
 log = "0.3"
 time = "0.1"

examples/multipart/src/main.rs

@@ -15,13 +15,11 @@ impl Actor for MyRoute {
 impl Route for MyRoute {
     type State = ();
 
-    fn request(req: HttpRequest, payload: Payload, ctx: &mut HttpContext<Self>) -> Reply<Self> {
+    fn request(req: &mut HttpRequest, payload: Payload,
+               ctx: &mut HttpContext<Self>) -> RouteResult<Self> {
         println!("{:?}", req);
 
-        let multipart = match req.multipart(payload) {
-            Ok(mp) => mp,
-            Err(e) => return e.into(),
-        };
+        let multipart = req.multipart(payload)?;
 
         // get Multipart stream
         WrapStream::<MyRoute>::actstream(multipart)

examples/websocket-chat/src/main.rs

@@ -48,11 +48,11 @@ impl Actor for WsChatSession {
 impl Route for WsChatSession {
     type State = WsChatSessionState;
 
-    fn request(req: HttpRequest, payload: Payload, ctx: &mut HttpContext<Self>) -> Reply<Self>
+    fn request(req: &mut HttpRequest,
+               payload: Payload, ctx: &mut HttpContext<Self>) -> RouteResult<Self>
     {
         // websocket handshakre, it may fail if request is not websocket request
-        match ws::handshake(&req) {
-            Ok(resp) => {
+        let resp = ws::handshake(&req)?;
         ctx.start(resp);
         ctx.add_stream(ws::WsStream::new(payload));
         Reply::async(
@@ -62,11 +62,6 @@ impl Route for WsChatSession {
                 room: "Main".to_owned(),
                 name: None})
     }
-            Err(err) => {
-                Reply::reply(err)
-            }
-        }
-    }
 }
 
 /// Handle messages from chat server, we simply send it to peer websocket

examples/websocket.rs

@@ -17,19 +17,13 @@ impl Route for MyWebSocket {
     type State = ();
 
     fn request(req: &mut HttpRequest,
-               payload: Payload, ctx: &mut HttpContext<Self>) -> Reply<Self>
+               payload: Payload, ctx: &mut HttpContext<Self>) -> RouteResult<Self>
     {
-        match ws::handshake(&req) {
-            Ok(resp) => {
+        let resp = ws::handshake(&req)?;
         ctx.start(resp);
         ctx.add_stream(ws::WsStream::new(payload));
         Reply::async(MyWebSocket)
     }
-            Err(err) => {
-                Reply::reply(err)
-            }
-        }
-    }
 }
 
 impl StreamHandler<ws::Message> for MyWebSocket {

src/application.rs

@@ -170,7 +170,7 @@ impl<S> ApplicationBuilder<S> where S: 'static {
     ///
     ///     fn request(req: &mut HttpRequest,
     ///                payload: Payload,
-    ///                ctx: &mut HttpContext<Self>) -> Reply<Self> {
+    ///                ctx: &mut HttpContext<Self>) -> RouteResult<Self> {
     ///         Reply::reply(httpcodes::HTTPOk)
     ///     }
     /// }

src/error.rs

@@ -110,8 +110,7 @@ impl From<httparse::Error> for ParseError {
 /// Return `BadRequest` for `ParseError`
 impl From<ParseError> for HttpResponse {
     fn from(err: ParseError) -> Self {
-        HttpResponse::new(StatusCode::BAD_REQUEST,
-                          Body::Binary(err.description().into()))
+        HttpResponse::from_error(StatusCode::BAD_REQUEST, err)
     }
 }
 
@@ -119,24 +118,21 @@ impl From<ParseError> for HttpResponse {
 /// Response generation can return `HttpError`, so it is internal error
 impl From<HttpError> for HttpResponse {
     fn from(err: HttpError) -> Self {
-        HttpResponse::new(StatusCode::INTERNAL_SERVER_ERROR,
-                          Body::Binary(err.description().into()))
+        HttpResponse::from_error(StatusCode::INTERNAL_SERVER_ERROR, err)
     }
 }
 
 /// Return `BadRequest` for `cookie::ParseError`
 impl From<cookie::ParseError> for HttpResponse {
     fn from(err: cookie::ParseError) -> Self {
-        HttpResponse::new(StatusCode::BAD_REQUEST,
-                          Body::Binary(err.description().into()))
+        HttpResponse::from_error(StatusCode::BAD_REQUEST, err)
     }
 }
 
 /// Return `BadRequest` for `MultipartError`
 impl From<MultipartError> for HttpResponse {
     fn from(err: MultipartError) -> Self {
-        HttpResponse::new(StatusCode::BAD_REQUEST,
-                          Body::Binary(err.description().into()))
+        HttpResponse::from_error(StatusCode::BAD_REQUEST, err)
     }
 }
 

src/httpresponse.rs

@@ -1,10 +1,11 @@
 //! Pieces pertaining to the HTTP message protocol.
 use std::{io, mem, str};
+use std::error::Error as Error;
 use std::convert::Into;
 
 use cookie::CookieJar;
 use bytes::Bytes;
-use http::{StatusCode, Version, HeaderMap, HttpTryFrom, Error};
+use http::{StatusCode, Version, HeaderMap, HttpTryFrom, Error as HttpError};
 use http::header::{self, HeaderName, HeaderValue};
 
 use Cookie;
@@ -57,6 +58,7 @@ pub struct HttpResponse {
     body: Body,
     chunked: bool,
     connection_type: Option<ConnectionType>,
+    error: Option<Box<Error>>,
 }
 
 impl HttpResponse {
@@ -81,9 +83,34 @@ impl HttpResponse {
             chunked: false,
             // compression: None,
             connection_type: None,
+            error: None,
         }
     }
 
+    /// Constructs a response from error
+    #[inline]
+    pub fn from_error<E: Error + 'static>(status: StatusCode, error: E) -> HttpResponse {
+        let body = Body::Binary(error.description().into());
+
+        HttpResponse {
+            version: None,
+            headers: Default::default(),
+            status: status,
+            reason: None,
+            body: body,
+            chunked: false,
+            // compression: None,
+            connection_type: None,
+            error: Some(Box::new(error)),
+        }
+    }
+
+    /// The `error` which is responsible for this response
+    #[inline]
+    pub fn error(&self) -> Option<&Box<Error>> {
+        self.error.as_ref()
+    }
+
     /// Get the HTTP version of this response.
     #[inline]
     pub fn version(&self) -> Option<Version> {
@@ -226,7 +253,7 @@ impl Parts {
 #[derive(Debug)]
 pub struct HttpResponseBuilder {
     parts: Option<Parts>,
-    err: Option<Error>,
+    err: Option<HttpError>,
 }
 
 impl HttpResponseBuilder {
@@ -348,7 +375,7 @@ impl HttpResponseBuilder {
     }
 
     /// Set a body
-    pub fn body<B: Into<Body>>(&mut self, body: B) -> Result<HttpResponse, Error> {
+    pub fn body<B: Into<Body>>(&mut self, body: B) -> Result<HttpResponse, HttpError> {
         let mut parts = self.parts.take().expect("cannot reuse response builder");
         if let Some(e) = self.err.take() {
             return Err(e)
@@ -366,11 +393,12 @@ impl HttpResponseBuilder {
             body: body.into(),
             chunked: parts.chunked,
             connection_type: parts.connection_type,
+            error: None,
         })
     }
 }
 
-fn parts<'a>(parts: &'a mut Option<Parts>, err: &Option<Error>) -> Option<&'a mut Parts>
+fn parts<'a>(parts: &'a mut Option<Parts>, err: &Option<HttpError>) -> Option<&'a mut Parts>
 {
     if err.is_some() {
         return None

src/lib.rs

@@ -1,9 +1,5 @@
 //! Http framework for [Actix](https://github.com/fafhrd91/actix)
 
-#![cfg_attr(feature="nightly", feature(
-    try_trait, // std::ops::Try #42327
-))]
-
 #[macro_use]
 extern crate log;
 extern crate time;
@@ -53,7 +49,7 @@ pub use application::{Application, ApplicationBuilder, Middleware};
 pub use httprequest::{HttpRequest, UrlEncoded};
 pub use httpresponse::{Body, HttpResponse, HttpResponseBuilder};
 pub use payload::{Payload, PayloadItem, PayloadError};
-pub use route::{Route, RouteFactory, RouteHandler};
+pub use route::{Route, RouteFactory, RouteHandler, RouteResult};
 pub use resource::{Reply, Resource};
 pub use recognizer::{Params, RouteRecognizer};
 pub use logger::Logger;

src/resource.rs

@@ -8,7 +8,7 @@ use http::Method;
 use futures::Stream;
 
 use task::Task;
-use route::{Route, RouteHandler, Frame, FnHandler, StreamHandler};
+use route::{Route, RouteHandler, RouteResult, Frame, FnHandler, StreamHandler};
 use payload::Payload;
 use context::HttpContext;
 use httprequest::HttpRequest;
@@ -141,13 +141,13 @@ pub struct Reply<A: Actor + Route> (ReplyItem<A>);
 impl<A> Reply<A> where A: Actor + Route
 {
     /// Create async response
-    pub fn async(act: A) -> Self {
-        Reply(ReplyItem::Actor(act))
+    pub fn async(act: A) -> RouteResult<A> {
+        Ok(Reply(ReplyItem::Actor(act)))
     }
 
     /// Send response
-    pub fn reply<R: Into<HttpResponse>>(response: R) -> Self {
-        Reply(ReplyItem::Message(response.into()))
+    pub fn reply<R: Into<HttpResponse>>(response: R) -> RouteResult<A> {
+        Ok(Reply(ReplyItem::Message(response.into())))
     }
 
     pub fn into(self, mut ctx: HttpContext<A>) -> Task where A: Actor<Context=HttpContext<A>>
@@ -168,27 +168,6 @@ impl<A, T> From<T> for Reply<A>
     where T: Into<HttpResponse>, A: Actor + Route
 {
     fn from(item: T) -> Self {
-        Reply::reply(item)
-    }
-}
-
-#[cfg(feature="nightly")]
-use std::ops::Try;
-
-#[cfg(feature="nightly")]
-impl<A> Try for Reply<A> where A: Actor + Route {
-    type Ok = HttpResponse;
-    type Error = HttpResponse;
-
-    fn into_result(self) -> Result<Self::Ok, Self::Error> {
-        panic!("Reply -> Result conversion is not supported")
-    }
-
-    fn from_error(v: Self::Error) -> Self {
-        Reply::reply(v)
-    }
-
-    fn from_ok(v: Self::Ok) -> Self {
-        Reply::reply(v)
+        Reply(ReplyItem::Message(item.into()))
     }
 }

src/route.rs

@@ -33,6 +33,9 @@ pub trait RouteHandler<S>: 'static {
     fn set_prefix(&mut self, prefix: String) {}
 }
 
+/// Request handling result.
+pub type RouteResult<T> = Result<Reply<T>, HttpResponse>;
+
 /// Actors with ability to handle http requests.
 #[allow(unused_variables)]
 pub trait Route: Actor {
@@ -74,7 +77,7 @@ pub trait Route: Actor {
     /// In that case `HttpContext::start` and `HttpContext::write` has to be used
     /// for writing response.
     fn request(req: &mut HttpRequest,
-               payload: Payload, ctx: &mut Self::Context) -> Reply<Self>;
+               payload: Payload, ctx: &mut Self::Context) -> RouteResult<Self>;
 
     /// This method creates `RouteFactory` for this actor.
     fn factory() -> RouteFactory<Self, Self::State> {
@@ -99,7 +102,10 @@ impl<A, S> RouteHandler<S> for RouteFactory<A, S>
                 return Task::reply(resp)
             }
         }
-        A::request(req, payload, &mut ctx).into(ctx)
+        match A::request(req, payload, &mut ctx) {
+            Ok(reply) => reply.into(ctx),
+            Err(err) => Task::reply(err),
+        }
     }
 }
 

src/ws.rs

@@ -22,7 +22,8 @@
 //! impl Route for WsRoute {
 //!     type State = ();
 //!
-//!     fn request(req: &mut HttpRequest, payload: Payload, ctx: &mut HttpContext<Self>) -> Reply<Self>
+//!     fn request(req: &mut HttpRequest,
+//!                payload: Payload, ctx: &mut HttpContext<Self>) -> RouteResult<Self>
 //!     {
 //!         // WebSocket handshake
 //!         match ws::handshake(&req) {