Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Error Handling #438

Merged
merged 9 commits into from
Aug 27, 2020
2 changes: 1 addition & 1 deletion examples/diesel/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ fn router(repo: Repo) -> Router {

fn bad_request<E>(e: E) -> HandlerError
where
E: std::error::Error + Send + 'static,
E: IntoHandlerError,
{
e.into_handler_error().with_status(StatusCode::BAD_REQUEST)
}
Expand Down
2 changes: 1 addition & 1 deletion examples/handlers/stateful/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::pin::Pin;
use std::sync::{Arc, Mutex};
use std::time::SystemTime;

use gotham::error::Result;
use gotham::anyhow::Result;
use gotham::handler::{Handler, HandlerFuture, IntoResponse, NewHandler};
use gotham::router::builder::*;
use gotham::router::Router;
Expand Down
4 changes: 1 addition & 3 deletions gotham/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ serde = "1.0"
serde_derive = "1.0"
bincode = "1.0"
mime = "0.3"
# Using alpha version of mime_guess until mime crate stabilizes (releases 1.0).
# see https://github.com/hyperium/mime/issues/52
mime_guess = "2.0.1"
futures = "0.3.1"
tokio = { version = "0.2.6", features = ["full"] }
Expand All @@ -47,8 +45,8 @@ regex = "1.0"
cookie = "0.13"
http = "0.2"
httpdate = "0.3"
failure = "0.1"
itertools = "0.9.0"
anyhow = "1.0"
tokio-rustls = { version = "0.12.1", optional = true }

[dev-dependencies]
Expand Down
11 changes: 0 additions & 11 deletions gotham/src/error.rs

This file was deleted.

5 changes: 2 additions & 3 deletions gotham/src/handler/assets/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

mod accepted_encoding;

use crate::error::Result;
use bytes::{BufMut, Bytes, BytesMut};
use futures::prelude::*;
use futures::ready;
Expand Down Expand Up @@ -165,15 +164,15 @@ impl DirHandler {
impl NewHandler for FileHandler {
type Instance = Self;

fn new_handler(&self) -> Result<Self::Instance> {
fn new_handler(&self) -> anyhow::Result<Self::Instance> {
Ok(self.clone())
}
}

impl NewHandler for DirHandler {
type Instance = Self;

fn new_handler(&self) -> Result<Self::Instance> {
fn new_handler(&self) -> anyhow::Result<Self::Instance> {
Ok(self.clone())
}
}
Expand Down
24 changes: 20 additions & 4 deletions gotham/src/handler/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,20 @@ use crate::state::{request_id, State};
/// `Response`.
pub struct HandlerError {
status_code: StatusCode,
cause: Box<dyn Error + Send>,
cause: anyhow::Error,
}

/// Convert a generic `anyhow::Error` into a `HandlerError`, similar as you would a concrete error
/// type with `into_handler_error()`.
impl From<anyhow::Error> for HandlerError {
fn from(error: anyhow::Error) -> HandlerError {
trace!(" converting Error to HandlerError: {}", error);

HandlerError {
status_code: StatusCode::INTERNAL_SERVER_ERROR,
cause: error,
}
}
}

/// Allows conversion into a HandlerError from an implementing type.
Expand Down Expand Up @@ -50,14 +63,14 @@ pub trait IntoHandlerError {

impl<E> IntoHandlerError for E
where
E: Error + Send + 'static,
E: Display + Into<anyhow::Error>,
{
fn into_handler_error(self) -> HandlerError {
trace!(" converting Error to HandlerError: {}", self);

HandlerError {
status_code: StatusCode::INTERNAL_SERVER_ERROR,
cause: Box::new(self),
cause: self.into(),
}
}
}
Expand Down Expand Up @@ -140,7 +153,10 @@ impl IntoResponse for HandlerError {
self.status_code
.canonical_reason()
.unwrap_or("(unregistered)",),
self.source().map(Error::description).unwrap_or("(none)"),
self.source()
.map(ToString::to_string)
.as_deref()
.unwrap_or("(none)"),
);

create_empty_response(state, self.status_code)
Expand Down
32 changes: 22 additions & 10 deletions gotham/src/handler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
//! `Handler`, but the traits can also be implemented directly for greater control. See the
//! `Handler` trait for some examples of valid handlers.
use std::borrow::Cow;
use std::ops::Deref;
use std::panic::RefUnwindSafe;
use std::pin::Pin;
use std::sync::Arc;

use bytes::Bytes;
use futures::prelude::*;
Expand All @@ -16,7 +18,6 @@ use crate::helpers::http::response;
use crate::state::State;

mod error;
use crate::error::*;

/// Defines handlers for serving static assets.
pub mod assets;
Expand Down Expand Up @@ -131,7 +132,7 @@ pub type HandlerFuture =
/// #
/// # use gotham::handler::{Handler, HandlerFuture, NewHandler};
/// # use gotham::state::State;
/// # use gotham::error::*;
/// # use gotham::anyhow;
/// #
/// # fn main() {
/// #[derive(Copy, Clone)]
Expand All @@ -140,7 +141,7 @@ pub type HandlerFuture =
/// impl NewHandler for MyCustomHandler {
/// type Instance = Self;
///
/// fn new_handler(&self) -> Result<Self::Instance> {
/// fn new_handler(&self) -> anyhow::Result<Self::Instance> {
/// Ok(*self)
/// }
/// }
Expand Down Expand Up @@ -189,7 +190,7 @@ where
/// #
/// # use gotham::handler::{Handler, HandlerFuture, NewHandler};
/// # use gotham::state::State;
/// # use gotham::error::*;
/// # use gotham::anyhow;
/// #
/// # fn main() {
/// #[derive(Copy, Clone)]
Expand All @@ -198,7 +199,7 @@ where
/// impl NewHandler for MyCustomHandler {
/// type Instance = Self;
///
/// fn new_handler(&self) -> Result<Self::Instance> {
/// fn new_handler(&self) -> anyhow::Result<Self::Instance> {
/// Ok(*self)
/// }
/// }
Expand All @@ -225,7 +226,7 @@ where
/// #
/// # use gotham::handler::{Handler, HandlerFuture, NewHandler};
/// # use gotham::state::State;
/// # use gotham::error::*;
/// # use gotham::anyhow;
/// #
/// # fn main() {
/// #[derive(Copy, Clone)]
Expand All @@ -234,7 +235,7 @@ where
/// impl NewHandler for MyValueInstantiatingHandler {
/// type Instance = MyHandler;
///
/// fn new_handler(&self) -> Result<Self::Instance> {
/// fn new_handler(&self) -> anyhow::Result<Self::Instance> {
/// Ok(MyHandler)
/// }
/// }
Expand All @@ -257,21 +258,32 @@ pub trait NewHandler: Send + Sync + RefUnwindSafe {
type Instance: Handler + Send;

/// Create and return a new `Handler` value.
fn new_handler(&self) -> Result<Self::Instance>;
fn new_handler(&self) -> anyhow::Result<Self::Instance>;
}

impl<F, H> NewHandler for F
where
F: Fn() -> Result<H> + Send + Sync + RefUnwindSafe,
F: Fn() -> anyhow::Result<H> + Send + Sync + RefUnwindSafe,
H: Handler + Send,
{
type Instance = H;

fn new_handler(&self) -> Result<H> {
fn new_handler(&self) -> anyhow::Result<H> {
self()
}
}

impl<H> NewHandler for Arc<H>
where
H: NewHandler,
{
type Instance = H::Instance;

fn new_handler(&self) -> anyhow::Result<Self::Instance> {
self.deref().new_handler()
}
}

/// Represents a type which can be converted into the future type returned by a `Handler`.
///
/// This is used to allow functions with different return types to satisfy the `Handler` trait
Expand Down
4 changes: 3 additions & 1 deletion gotham/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
// TODO: Remove this when it's a hard error by default (error E0446).
// See Rust issue #34537 <https://github.com/rust-lang/rust/issues/34537>
#![deny(private_in_public)]
pub mod error;

pub mod extractor;
pub mod handler;
pub mod helpers;
Expand All @@ -45,6 +45,8 @@ pub mod plain;
#[cfg(feature = "rustls")]
pub mod tls;

/// Re-export anyhow
pub use anyhow;
/// Re-export hyper
pub use hyper;

Expand Down
7 changes: 3 additions & 4 deletions gotham/src/middleware/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

use log::trace;

use std::io;
use std::panic::RefUnwindSafe;
use std::pin::Pin;

Expand All @@ -19,7 +18,7 @@ pub unsafe trait NewMiddlewareChain: RefUnwindSafe + Sized {
type Instance: MiddlewareChain;

/// Create and return a new `MiddlewareChain` value.
fn construct(&self) -> io::Result<Self::Instance>;
fn construct(&self) -> anyhow::Result<Self::Instance>;
}

unsafe impl<T, U> NewMiddlewareChain for (T, U)
Expand All @@ -30,7 +29,7 @@ where
{
type Instance = (T::Instance, U::Instance);

fn construct(&self) -> io::Result<Self::Instance> {
fn construct(&self) -> anyhow::Result<Self::Instance> {
// This works as a recursive `map` over the "list" of `NewMiddleware`, and is used in
// creating the `Middleware` instances for serving a single request.
//
Expand All @@ -44,7 +43,7 @@ where
unsafe impl NewMiddlewareChain for () {
type Instance = ();

fn construct(&self) -> io::Result<Self::Instance> {
fn construct(&self) -> anyhow::Result<Self::Instance> {
// () marks the end of the list, so is returned as-is.
trace!(" completed middleware pipeline construction");
Ok(())
Expand Down
3 changes: 1 addition & 2 deletions gotham/src/middleware/cookie/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//! Defines a cookie parsing middleware to be attach cookies on requests.
use std::io;
use std::pin::Pin;

use cookie::{Cookie, CookieJar};
Expand Down Expand Up @@ -52,7 +51,7 @@ impl NewMiddleware for CookieParser {
type Instance = Self;

/// Clones the current middleware to a new instance.
fn new_middleware(&self) -> io::Result<Self::Instance> {
fn new_middleware(&self) -> anyhow::Result<Self::Instance> {
Ok(*self)
}
}
5 changes: 2 additions & 3 deletions gotham/src/middleware/logger/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use futures::prelude::*;
use hyper::{header::CONTENT_LENGTH, Method, Uri, Version};
use log::Level;
use log::{log, log_enabled};
use std::io;
use std::pin::Pin;

use crate::handler::HandlerFuture;
Expand Down Expand Up @@ -42,7 +41,7 @@ impl NewMiddleware for RequestLogger {
type Instance = Self;

/// Returns a new middleware to be used to serve a request.
fn new_middleware(&self) -> io::Result<Self::Instance> {
fn new_middleware(&self) -> anyhow::Result<Self::Instance> {
Ok(*self)
}
}
Expand Down Expand Up @@ -132,7 +131,7 @@ impl NewMiddleware for SimpleLogger {
type Instance = Self;

/// Returns a new middleware to be used to serve a request.
fn new_middleware(&self) -> io::Result<Self::Instance> {
fn new_middleware(&self) -> anyhow::Result<Self::Instance> {
Ok(*self)
}
}
Expand Down
6 changes: 2 additions & 4 deletions gotham/src/middleware/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Defines types for `Middleware`, a reusable unit of logic that can apply to a group of requests
//! by being added to the `Pipeline` in a `Router`.

use std::io;
use std::panic::RefUnwindSafe;
use std::pin::Pin;

Expand Down Expand Up @@ -347,7 +346,6 @@ pub trait Middleware {
/// ```rust
/// # extern crate gotham;
/// #
/// # use std::io;
/// # use std::pin::Pin;
/// #
/// # use gotham::middleware::{NewMiddleware, Middleware};
Expand All @@ -362,7 +360,7 @@ pub trait Middleware {
/// impl NewMiddleware for MyMiddleware {
/// type Instance = Self;
///
/// fn new_middleware(&self) -> io::Result<Self::Instance> {
/// fn new_middleware(&self) -> anyhow::Result<Self::Instance> {
/// Ok(self.clone())
/// }
/// }
Expand All @@ -384,5 +382,5 @@ pub trait NewMiddleware: Sync + RefUnwindSafe {
type Instance: Middleware;

/// Create and return a new `Middleware` value.
fn new_middleware(&self) -> io::Result<Self::Instance>;
fn new_middleware(&self) -> anyhow::Result<Self::Instance>;
}
3 changes: 1 addition & 2 deletions gotham/src/middleware/security/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use futures::prelude::*;
use std::pin::Pin;

use hyper::header::{HeaderValue, X_CONTENT_TYPE_OPTIONS, X_FRAME_OPTIONS, X_XSS_PROTECTION};
use std::io;

// constant strings to be used as header values
const XFO_VALUE: &str = "DENY";
Expand Down Expand Up @@ -60,7 +59,7 @@ impl NewMiddleware for SecurityMiddleware {
type Instance = Self;

/// Clones the current middleware to a new instance.
fn new_middleware(&self) -> io::Result<Self::Instance> {
fn new_middleware(&self) -> anyhow::Result<Self::Instance> {
Ok(self.clone())
}
}
4 changes: 2 additions & 2 deletions gotham/src/middleware/session/backend/memory.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::pin::Pin;
use std::sync::{Arc, Mutex, PoisonError, Weak};
use std::thread;
use std::time::{Duration, Instant};
use std::{io, thread};

use futures::prelude::*;
use linked_hash_map::LinkedHashMap;
Expand Down Expand Up @@ -72,7 +72,7 @@ impl Default for MemoryBackend {
impl NewBackend for MemoryBackend {
type Instance = MemoryBackend;

fn new_backend(&self) -> io::Result<Self::Instance> {
fn new_backend(&self) -> anyhow::Result<Self::Instance> {
Ok(self.clone())
}
}
Expand Down
Loading