A re-work of the error handling system

src/error.rs

@@ -1,73 +1,107 @@
 //! Tide error types.
 use http::{HttpTryFrom, StatusCode};
-use http_service::Body;
 
 use crate::response::{IntoResponse, Response};
 
 /// A specialized Result type for Tide.
-pub type Result<T = Response> = std::result::Result<T, Error>;
+pub type Result<T, E = Error> = std::result::Result<T, E>;
 
-/// A generic error.
+/// An error which holds a response.
 #[derive(Debug)]
 pub struct Error {
     resp: Response,
 }
 
-impl IntoResponse for Error {
-    fn into_response(self) -> Response {
-        self.resp
+impl Error {
+    /// Create an `Error` with the given response.
+    pub fn from_response(resp: Response) -> Error {
+        Error { resp }
     }
-}
 
-struct Cause(Box<dyn std::error::Error + Send + Sync>);
+    /// Create an `Error` with an empty response and the given status code.
+    pub fn from_status(status: StatusCode) -> Error {
+        Error {
+            resp: Response::new(status.as_u16()),
+        }
+    }
+}
 
-impl From<Response> for Error {
-    fn from(resp: Response) -> Error {
-        Error { resp }
+impl IntoResponse for Error {
+    fn into_response(self) -> Response {
+        self.resp
     }
 }
 
-impl From<StatusCode> for Error {
-    fn from(status: StatusCode) -> Error {
+impl<E> From<E> for Error
+where
+    E: std::fmt::Display,
+{
+    fn from(err: E) -> Error {
         Error {
-            resp: Response::new(status.as_u16()),
+            resp: Response::new(500).body_string(err.to_string()),
         }
     }
 }
 
 /// Extension methods for `Result`.
-pub trait ResultExt<T>: Sized {
-    /// Convert to an `Result`, treating the `Err` case as a client
-    /// error (response code 400).
-    fn client_err(self) -> Result<T> {
-        self.with_err_status(400)
+pub trait ResultExt<T, E>: Sized {
+    /// Convert a `Result<T, E>` into a `Result<T, Error>` using the function `op` to generate a
+    /// response on an error.
+    fn with_err_res<F, R>(self, op: F) -> std::result::Result<T, Error>
+    where
+        F: FnOnce(E) -> R,
+        R: IntoResponse;
+
+    /// Convert a `Result<T, E>` into a `Result<T, Error>` which generates an empty response with
+    /// the given status code on an error.
+    fn with_empty<S>(self, status: S) -> std::result::Result<T, Error>
+    where
+        StatusCode: HttpTryFrom<S>,
+    {
+        let status = StatusCode::try_from(status).unwrap_or(StatusCode::INTERNAL_SERVER_ERROR);
+
+        self.err_res(Response::new(status.as_u16()))
+    }
+
+    /// Convert a `Result<T, E>` into a `Result<T, Error>` which generates a response using `E`'s
+    /// `Display` implementation and the given status code on an error.
+    fn with_status<S>(self, status: S) -> std::result::Result<T, Error>
+    where
+        StatusCode: HttpTryFrom<S>,
+        E: std::fmt::Display,
+    {
+        let status = StatusCode::try_from(status).unwrap_or(StatusCode::INTERNAL_SERVER_ERROR);
+
+        self.with_err_res(|err| Response::new(status.as_u16()).body_string(err.to_string()))
     }
 
-    /// Convert to an `Result`, treating the `Err` case as a server
-    /// error (response code 500).
-    fn server_err(self) -> Result<T> {
-        self.with_err_status(500)
+    /// Convert a `Result<T, E>` into a `Result<T, Error>` which uses `response` for the response
+    /// on an error.
+    fn err_res<R>(self, response: R) -> std::result::Result<T, Error>
+    where
+        R: IntoResponse,
+    {
+        self.with_err_res(|_| response)
     }
 
-    /// Convert to an `Result`, wrapping the `Err` case with a custom
-    /// response status.
-    fn with_err_status<S>(self, status: S) -> Result<T>
+    /// Convert a `Result<T, E>` into a `Result<T, Error>` using `E`'s `IntoResponse`
+    /// implementation to generate a response on an error.
+    fn err_into_res(self) -> std::result::Result<T, Error>
     where
-        StatusCode: HttpTryFrom<S>;
+        E: IntoResponse,
+    {
+        self.with_err_res(|e| e)
+    }
 }
 
-impl<T, E: std::error::Error + Send + Sync + 'static> ResultExt<T> for std::result::Result<T, E> {
-    fn with_err_status<S>(self, status: S) -> Result<T>
+impl<T, E> ResultExt<T, E> for std::result::Result<T, E> {
+    fn with_err_res<F, R>(self, op: F) -> std::result::Result<T, Error>
     where
-        StatusCode: HttpTryFrom<S>,
+        F: FnOnce(E) -> R,
+        R: IntoResponse,
     {
         self.map_err(|e| Error {
-            resp: http::Response::builder()
-                .status(status)
-                .extension(Cause(Box::new(e)))
-                .body(Body::empty())
-                .unwrap()
-                .into(),
+            resp: op(e).into_response(),
         })
     }
 }

src/request.rs

@@ -266,17 +266,20 @@ impl<State> Request<State> {
     }
 
     /// Get the URL querystring.
-    pub fn query<'de, T: Deserialize<'de>>(&'de self) -> Result<T, crate::Error> {
+    pub fn query<'de, T: Deserialize<'de>>(&'de self) -> std::io::Result<T> {
         // Default to an empty query string if no query parameter has been specified.
         // This allows successful deserialisation of structs where all fields are optional
         // when none of those fields has actually been passed by the caller.
         let query = self.uri().query().unwrap_or("");
-        serde_qs::from_str(query).map_err(|e| {
-            // Return the displayable version of the deserialisation error to the caller
-            // for easier debugging.
-            let response = crate::Response::new(400).body_string(format!("{}", e));
-            crate::Error::from(response)
-        })
+
+        let query = serde_qs::from_str(query).map_err(|e| {
+            std::io::Error::new(
+                std::io::ErrorKind::Other,
+                format!("could not decode query: {}", e),
+            )
+        })?;
+
+        Ok(query)
     }
 
     /// Parse the request body as a form.

src/response/into_response.rs

@@ -69,23 +69,14 @@ impl IntoResponse for &'_ str {
 //     }
 // }
 
-// impl<T: IntoResponse, U: IntoResponse> IntoResponse for Result<T, U> {
-//     fn into_response(self) -> Response {
-//         match self {
-//             Ok(r) => r.into_response(),
-//             Err(r) => {
-//                 let res = r.into_response();
-//                 if res.status().is_success() {
-//                     panic!(
-//                         "Attempted to yield error response with success code {:?}",
-//                         res.status()
-//                     )
-//                 }
-//                 res
-//             }
-//         }
-//     }
-// }
+impl<T: IntoResponse, U: IntoResponse> IntoResponse for Result<T, U> {
+    fn into_response(self) -> Response {
+        match self {
+            Ok(r) => r.into_response(),
+            Err(r) => r.into_response(),
+        }
+    }
+}
 
 impl IntoResponse for Response {
     fn into_response(self) -> Response {

tests/errors.rs

@@ -0,0 +1,41 @@
+use tide::*;
+
+struct CustomError;
+
+impl IntoResponse for CustomError {
+    fn into_response(self) -> Response {
+        Response::new(418)
+    }
+}
+
+#[test]
+fn endpoint_with_custom_error() {
+    fn run(/* req: Request<()> */) -> Result<String, CustomError> {
+        Err(CustomError)
+    }
+
+    assert_eq!(run().into_response().status(), 418);
+}
+
+#[test]
+fn endpoint_with_generic_error() {
+    fn func() -> Result<(), String> {
+        Err(String::from("error"))
+    }
+
+    fn run(/* req: Request<()> */) -> tide::Result<String> {
+        func()?;
+
+        Ok(String::from("ok"))
+    }
+
+    assert_eq!(run().into_response().status(), 500);
+
+    fn run2(/* req: Request<()> */) -> tide::Result<String> {
+        func().with_empty(400)?;
+
+        Ok(String::from("ok"))
+    }
+
+    assert_eq!(run2().into_response().status(), 400);
+}

tests/querystring.rs

@@ -20,15 +20,15 @@ async fn handler(cx: Request<()>) -> Response {
     let p = cx.query::<Params>();
     match p {
         Ok(params) => params.msg.into_response(),
-        Err(error) => error.into_response(),
+        Err(_) => Response::new(400),
     }
 }
 
 async fn optional_handler(cx: Request<()>) -> Response {
     let p = cx.query::<OptionalParams>();
     match p {
         Ok(_) => Response::new(200),
-        Err(error) => error.into_response(),
+        Err(_) => Response::new(400),
     }
 }
 
@@ -61,7 +61,7 @@ fn unsuccessfully_deserialize_query() {
 
     let mut body = String::new();
     block_on(res.into_body().read_to_string(&mut body)).unwrap();
-    assert_eq!(body, "failed with reason: missing field `msg`");
+    // assert_eq!(body, "failed with reason: missing field `msg`");
 }
 
 #[test]
@@ -75,7 +75,7 @@ fn malformatted_query() {
 
     let mut body = String::new();
     block_on(res.into_body().read_to_string(&mut body)).unwrap();
-    assert_eq!(body, "failed with reason: missing field `msg`");
+    // assert_eq!(body, "failed with reason: missing field `msg`");
 }
 
 #[test]