fix(error_handling): introduce IntoError trait to reduce coherence issues

Cargo.toml

@@ -136,3 +136,8 @@ path = "examples/https.rs"
 
 name = "no_macro_custom_data"
 path = "examples/no_macro_custom_data.rs"
+
+[[example]]
+
+name = "error_handling"
+path = "examples/error_handling.rs"

examples/custom_error_handler.rs

@@ -42,9 +42,6 @@ fn main() {
         Action::Continue(())
     }
 
-    // issue #20178
-    let custom_handler: fn(&mut NickelError<()>, &mut Request<()>) -> Action = custom_handler;
-
     server.handle_error(custom_handler);
 
     server.listen("127.0.0.1:6767").unwrap();

examples/error_handling.rs

@@ -0,0 +1,147 @@
+#[macro_use] extern crate nickel;
+extern crate rustc_serialize;
+
+use nickel::{Nickel, Request, Response, MiddlewareResult, HttpRouter, NickelError};
+use nickel::{Action, IntoError};
+use nickel::status::StatusCode;
+use rustc_serialize::json::{self, Json};
+use std::{error,io, fmt};
+use std::io::Write;
+
+/// The crate-local server data type.
+struct MyData;
+
+/// An example of a crate-local error type.
+#[derive(Debug)]
+pub enum AppError {
+  Io(io::Error),
+  Custom(String)
+}
+
+/// Type alias for convenient use of the crate-local error type.
+type AppResult<T> = Result<T, AppError>;
+
+/// If the type you're implementing the trait for is crate-local, then you can
+/// make your `IntoError` implementation generic over the ServerData type if you
+/// want to. This can be good if you want to create an error type in a library
+/// which can be used by servers with any data type.
+impl<D> IntoError<D> for AppError {
+  fn into<'a>(self, res: Response<'a, D>) -> NickelError<'a, D> {
+    let internal_msg = format!("{}", self);
+
+    let status_code = match self {
+      AppError::Custom(_) => StatusCode::BadRequest,
+      AppError::Io(_)     => StatusCode::InternalServerError
+    };
+
+    NickelError::new(res, internal_msg, status_code)
+  }
+}
+
+/// By using a local type as the ServerData type, you can implement `IntoError`
+/// for foreign types. This example uses the `json::ParserError` type which is
+/// used whenever `Json::from_str` fails.
+impl IntoError<MyData> for json::ParserError {
+  fn into<'a>(self, res: Response<'a, MyData>) -> NickelError<'a, MyData> {
+    NickelError::new(res, "Failed to create json", StatusCode::ImATeapot)
+  }
+}
+
+fn will_fail() -> AppResult<String> {
+  Err(AppError::Custom("Something went wrong!".to_string()))
+}
+
+fn will_work() -> AppResult<&'static str> {
+  Ok("Everything went to plan!")
+}
+
+fn success_handler<'a, D>(_: &mut Request<D>, res: Response<'a, D>) -> MiddlewareResult<'a, D> {
+  let msg = try_with!(res, will_work());
+
+  res.send(msg)
+}
+
+fn failure_handler<'a, D>(_: &mut Request<D>, res: Response<'a, D>) -> MiddlewareResult<'a, D> {
+  let msg = try_with!(res, will_fail());
+
+  res.send(msg)
+}
+
+/// Note that this handler explicitly defined its data type so that the `IntoError` impl
+/// is able to be used.
+fn json_handler<'a>(_: &mut Request<MyData>, res: Response<'a, MyData>)
+                    -> MiddlewareResult<'a, MyData> {
+  let json = try_with!(res, Json::from_str("Not json"));
+
+  res.send(json)
+}
+
+fn main() {
+  let mut server = Nickel::with_data(MyData);
+  server.get("/failure", failure_handler);
+  server.get("/success", success_handler);
+  server.get("/json_nomacro", json_handler);
+  server.get("/json", middleware! { |_, res|
+    let json = try_with!(res, Json::from_str("Not json"));
+
+    return res.send(json)
+  });
+
+  server.handle_error(|err: &mut NickelError<_>, req: &mut Request<_>| {
+    // Print the internal error message and path to the console
+    println!("[{}] ERROR: {}",
+            req.path_without_query().unwrap(),
+            err.message);
+
+    // If we still have a stream available then render a customised response
+    // for some StatusCodes.
+    if let Some(ref mut res) = err.stream {
+      match res.status() {
+        StatusCode::ImATeapot => {
+          // Discard the result as if it fails there's not much we can do.
+          let _ = res.write_all(b"<h2>I'm a Teapot!</h2>");
+        }
+        StatusCode::NotFound => {
+          let _ = res.write_all(b"<h1>404 - Not Found</h1>");
+        }
+        // Let the next ErrorHandler do something.
+        _ => return Action::Continue(())
+      }
+    }
+
+    // Send nothing else, just let the client interpret the StatusCode.
+    Action::Halt(())
+  });
+
+  server.listen("127.0.0.1:6767").unwrap();
+}
+
+
+//
+// Other trait implementations for AppError
+//
+
+impl fmt::Display for AppError {
+  fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    match *self {
+      AppError::Custom(ref msg) => write!(f, "Custom error: {}", msg),
+      AppError::Io(ref err)     => write!(f, "IO error: {}", err)
+    }
+  }
+}
+
+impl error::Error for AppError {
+  fn description(&self) -> &str {
+    match *self {
+      AppError::Custom(ref msg) => msg,
+      AppError::Io(ref err)     => err.description()
+    }
+  }
+
+  fn cause(&self) -> Option<&error::Error> {
+    match *self {
+      AppError::Custom(_)   => None,
+      AppError::Io(ref err) => Some(err)
+    }
+  }
+}

examples/macro_example.rs

@@ -121,10 +121,7 @@ fn main() {
         }
     ));
 
-    // issue #20178
-    let custom_handler: fn(&mut NickelError<()>, &mut Request<()>) -> Action = custom_404;
-
-    server.handle_error(custom_handler);
+    server.handle_error(custom_404);
 
     println!("Running server!");
     server.listen("127.0.0.1:6767").unwrap();

src/lib.rs

@@ -26,7 +26,7 @@ pub use body_parser::{BodyError, FormBody, JsonBody};
 pub use query_string::QueryString;
 pub use urlencoded::{Params, Query};
 pub use router::{Router, Route, RouteResult, HttpRouter};
-pub use nickel_error::NickelError;
+pub use nickel_error::{IntoError, NickelError};
 pub use mimes::MediaType;
 pub use responder::Responder;
 pub use server::ListeningServer;

src/macros/mod.rs

@@ -1,13 +1,16 @@
 #[macro_use] mod middleware;
 #[macro_use] mod router;
 
+/// Variant of the `try!` macro which takes ownership of a Response on error.
+///
+/// See the `IntoError` documentation for usage examples.
 #[macro_export]
 macro_rules! try_with {
     ($res:expr, $exp:expr) => {{
         match $exp {
             ::std::result::Result::Ok(val) => val,
             ::std::result::Result::Err(e) => {
-                return Err(From::from(($res, e)))
+                return Err($crate::IntoError::into(e, $res))
             }
         }
     }};

src/middleware.rs

@@ -32,7 +32,8 @@ pub trait ErrorHandler<D>: Send + 'static + Sync {
     fn handle_error(&self, &mut NickelError<D>, &mut Request<D>) -> Action;
 }
 
-impl<D: 'static> ErrorHandler<D> for fn(&mut NickelError<D>, &mut Request<D>) -> Action {
+impl<D: 'static, F> ErrorHandler<D> for F
+where F: Send + Sync + 'static + Fn(&mut NickelError<D>, &mut Request<D>) -> Action {
     fn handle_error(&self, err: &mut NickelError<D>, req: &mut Request<D>) -> Action {
         (*self)(err, req)
     }

src/nickel_error.rs

@@ -69,22 +69,136 @@ impl<'a, D> NickelError<'a, D> {
     }
 }
 
-impl<'a, T, D> From<(Response<'a, D>, (StatusCode, T))> for NickelError<'a, D>
-        where T: Into<Box<Error + 'static>> {
-    fn from((res, (errorcode, err)): (Response<'a, D>, (StatusCode, T))) -> NickelError<'a, D> {
-        let err = err.into();
-        NickelError::new(res, err.description().to_string(), errorcode)
+/// `IntoError` is the required bounds for the `try_with!` macro.
+///
+/// The main reason to have this trait rather than relying on `Into`
+/// is that we need to consume the `Response` itself. If we have generic
+/// impls with `(Response, T)` then there's coherence problems for users
+/// who wish to implement this for foreign items.
+///
+/// # Basic usage
+/// ```rust
+/// #[macro_use]
+/// extern crate nickel;
+/// extern crate rustc_serialize;
+/// use nickel::{Request, Response, MiddlewareResult, JsonBody};
+/// use nickel::status::StatusCode;
+///
+/// #[derive(RustcDecodable)]
+/// struct Person {
+///     name: String,
+/// }
+///
+/// // An example handler which uses the built-in implementations of `IntoError`
+/// // via the `try_with!` macro.
+/// # #[allow(unused_assignments)]
+/// fn handler<'mw, D>(req: &mut Request<D>, res: Response<'mw, D>) -> MiddlewareResult<'mw, D> {
+///     let mut person: Person;
+///
+///     // A String which will be passed to your error handlers.
+///     // The StatusCode for the response will be InternalServerError.
+///     person = try_with!(res, req.json_as().map_err(|e| e.to_string()));
+///
+///     // A StatusCode which will be used for the response status.
+///     // The error message passed to error handlers will be empty.
+///     person = try_with!(res, req.json_as().map_err(|_| StatusCode::BadRequest));
+///
+///     // A Tuple of (StatusCode, T) where T: Into<Box<std::error::Error>>.
+///     // The Error implementation will be used for the message sent to error handlers.
+///     person = try_with!(res, req.json_as().map_err(|e| (StatusCode::BadRequest, e)));
+///
+///     res.send(format!("Hello {}!", person.name))
+/// }
+///
+/// # fn main() {
+/// #     use nickel::{HttpRouter, Nickel};
+/// #     let mut app = Nickel::new();
+/// #     app.get("*", handler);
+/// # }
+/// ```
+///
+/// # Custom implementations
+///
+/// ## Implementing for foreign error types
+///
+/// With this trait, if a user uses their own `ServerData` type then they can
+/// implement this trait for foreign types, e.g. the `json::ParserError` type.
+///
+/// ```rust
+/// extern crate nickel;
+/// extern crate rustc_serialize;
+///
+/// use nickel::{NickelError, Response, IntoError};
+/// use nickel::status::StatusCode;
+/// use rustc_serialize::json;
+///
+/// /// The crate-local server data type.
+/// struct ServerData;
+///
+/// // Note the explicit use of `ServerData` instead of a generic parameter.
+/// impl IntoError<ServerData> for json::ParserError {
+///     fn into<'a>(self, res: Response<'a, ServerData>) -> NickelError<'a, ServerData> {
+///         NickelError::new(res, "Failed to create json", StatusCode::ImATeapot)
+///     }
+/// }
+/// # fn main() {}
+/// ```
+///
+/// ## Implementing for local error types
+///
+/// With local error types, you can implement this generically over the server data
+/// type if you want. This is useful for library code where you want the users to be
+/// able to choose their own server data types.
+///
+/// ```rust
+/// use std::io;
+/// use nickel::{NickelError, Response, IntoError};
+/// use nickel::status::StatusCode;
+///
+/// /// A crate-local error type.
+/// #[derive(Debug)]
+/// pub enum AppError {
+///     Io(io::Error),
+///     Custom(String)
+/// }
+///
+/// impl<D> IntoError<D> for AppError {
+///     fn into<'a>(self, res: Response<'a, D>) -> NickelError<'a, D> {
+///         let internal_msg = format!("{:?}", self);
+///
+///         let status_code = match self {
+///             AppError::Custom(_) => StatusCode::BadRequest,
+///             AppError::Io(_)     => StatusCode::InternalServerError
+///         };
+///
+///         NickelError::new(res, internal_msg, status_code)
+///     }
+/// }
+/// # fn main() {}
+/// ```
+///
+/// See the `error_handling` example for a more complete example.
+pub trait IntoError<D> : Sized {
+    fn into<'a>(self, res: Response<'a, D>) -> NickelError<'a, D>;
+}
+
+impl<D> IntoError<D> for StatusCode {
+    fn into<'a>(self, res: Response<'a, D>) -> NickelError<'a, D> {
+        NickelError::new(res, "", self)
     }
 }
 
-impl<'a, D> From<(Response<'a, D>, String)> for NickelError<'a, D> {
-    fn from((res, msg): (Response<'a, D>, String)) -> NickelError<'a, D> {
-        NickelError::new(res, msg, StatusCode::InternalServerError)
+impl<D> IntoError<D> for String {
+    fn into<'a>(self, res: Response<'a, D>) -> NickelError<'a, D> {
+        NickelError::new(res, self, StatusCode::InternalServerError)
     }
 }
 
-impl<'a, D> From<(Response<'a, D>, StatusCode)> for NickelError<'a, D> {
-    fn from((res, code): (Response<'a, D>, StatusCode)) -> NickelError<'a, D> {
-        NickelError::new(res, "", code)
+impl<D, T> IntoError<D> for (StatusCode, T)
+where T: Into<Box<Error + 'static>> {
+    fn into<'a>(self, res: Response<'a, D>) -> NickelError<'a, D> {
+        let (status_code, err) = self;
+        let err = err.into();
+        NickelError::new(res, err.description().to_string(), status_code)
     }
 }

src/responder.rs

@@ -9,7 +9,7 @@
 //! in any request.
 //!
 //! Please see the examples for usage.
-use {Response, NickelError, MiddlewareResult, Halt};
+use {Response, MiddlewareResult, Halt, IntoError};
 use hyper::status::{StatusCode, StatusClass};
 use hyper::header;
 use serialize::json;
@@ -41,7 +41,7 @@ impl<D> Responder<D> for json::Json {
 
 impl<T, E, D> Responder<D> for Result<T, E>
         where T: Responder<D>,
-              for<'e> NickelError<'e, D>: From<(Response<'e, D>, E)> {
+              E: IntoError<D> {
     fn respond<'a>(self, res: Response<'a, D>) -> MiddlewareResult<'a, D> {
         let data = try_with!(res, self);
         res.send(data)

tests/example_tests.rs

@@ -27,6 +27,7 @@ mod examples {
     mod enable_cors;
     mod form_data;
     mod integration_testing;
+    mod error_handling;
 
     #[cfg(feature = "ssl")]
     mod https;