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

Allow specifying the error type for NewMiddleware and related traits #430

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions gotham/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ cookie = "0.13"
http = "0.2"
httpdate = "0.3"
failure = "0.1"
thiserror = "1.0"
itertools = "0.9.0"
tokio-rustls = { version = "0.12.1", optional = true }

Expand Down
36 changes: 31 additions & 5 deletions gotham/src/middleware/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,43 @@

use log::trace;

use std::io;
use std::convert::Infallible;
use std::error::Error;
use std::panic::RefUnwindSafe;
use std::pin::Pin;
use thiserror::Error;

use crate::handler::HandlerFuture;
use crate::middleware::{Middleware, NewMiddleware};
use crate::state::{request_id, State};

/// This error type is used by `NewMiddlewareChain` to wrap the two errors that can occur, one from
/// the middleware, and one from the chain, in one error type.
#[derive(Debug, Error)]
pub enum MiddlewareChainError<E, F>
where
E: Error + 'static,
F: Error + 'static,
{
/// Wrap an error returned by `NewMiddleware`.
#[error("{0}")]
MiddlewareError(#[source] E),
/// Wrap an error returned by `NewMiddlewareChain`.
#[error("{0}")]
ChainError(#[source] F),
}

/// A recursive type representing a pipeline, which is used to spawn a `MiddlewareChain`.
///
/// This type should never be implemented outside of Gotham, does not form part of the public API,
/// and is subject to change without notice.
#[doc(hidden)]
pub unsafe trait NewMiddlewareChain: RefUnwindSafe + Sized {
type Instance: MiddlewareChain;
type Err: Error + Send + 'static;

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

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

fn construct(&self) -> io::Result<Self::Instance> {
fn construct(&self) -> Result<Self::Instance, Self::Err> {
// This works as a recursive `map` over the "list" of `NewMiddleware`, and is used in
// creating the `Middleware` instances for serving a single request.
//
// The reversed order is preserved in the return value.
trace!(" adding middleware instance to pipeline");
let (ref nm, ref tail) = *self;
Ok((nm.new_middleware()?, tail.construct()?))
Ok((
nm.new_middleware()
.map_err(|err| MiddlewareChainError::MiddlewareError(err))?,
tail.construct()
.map_err(|err| MiddlewareChainError::ChainError(err))?,
))
}
}

unsafe impl NewMiddlewareChain for () {
type Instance = ();
type Err = Infallible;

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

use cookie::{Cookie, CookieJar};
Expand Down Expand Up @@ -50,9 +50,10 @@ impl Middleware for CookieParser {
/// `NewMiddleware` trait implementation.
impl NewMiddleware for CookieParser {
type Instance = Self;
type Err = Infallible;

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

use crate::handler::HandlerFuture;
Expand Down Expand Up @@ -40,9 +40,10 @@ impl RequestLogger {
/// which will clone the structure - should be cheaper for repeated calls.
impl NewMiddleware for RequestLogger {
type Instance = Self;
type Err = Infallible;

/// Returns a new middleware to be used to serve a request.
fn new_middleware(&self) -> io::Result<Self::Instance> {
fn new_middleware(&self) -> Result<Self, Infallible> {
Ok(*self)
}
}
Expand Down Expand Up @@ -130,9 +131,10 @@ impl SimpleLogger {
/// which will clone the structure - should be cheaper for repeated calls.
impl NewMiddleware for SimpleLogger {
type Instance = Self;
type Err = Infallible;

/// Returns a new middleware to be used to serve a request.
fn new_middleware(&self) -> io::Result<Self::Instance> {
fn new_middleware(&self) -> Result<Self::Instance, Infallible> {
Ok(*self)
}
}
Expand Down
11 changes: 7 additions & 4 deletions gotham/src/middleware/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! 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::error::Error;
use std::panic::RefUnwindSafe;
use std::pin::Pin;

Expand Down Expand Up @@ -347,7 +347,7 @@ pub trait Middleware {
/// ```rust
/// # extern crate gotham;
/// #
/// # use std::io;
/// # use std::convert::Infallible;
/// # use std::pin::Pin;
/// #
/// # use gotham::middleware::{NewMiddleware, Middleware};
Expand All @@ -361,8 +361,9 @@ pub trait Middleware {
///
/// impl NewMiddleware for MyMiddleware {
/// type Instance = Self;
/// type Err = Infallible;
///
/// fn new_middleware(&self) -> io::Result<Self::Instance> {
/// fn new_middleware(&self) -> Result<Self, Infallible> {
/// Ok(self.clone())
/// }
/// }
Expand All @@ -382,7 +383,9 @@ pub trait Middleware {
pub trait NewMiddleware: Sync + RefUnwindSafe {
/// The type of `Middleware` created by the `NewMiddleware`.
type Instance: Middleware;
/// The error that can occur when creating a new middleware.
type Err: Error + Send + 'static;

/// Create and return a new `Middleware` value.
fn new_middleware(&self) -> io::Result<Self::Instance>;
fn new_middleware(&self) -> Result<Self::Instance, Self::Err>;
}
5 changes: 3 additions & 2 deletions gotham/src/middleware/security/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ use crate::handler::HandlerFuture;
use crate::middleware::{Middleware, NewMiddleware};
use crate::state::State;
use futures::prelude::*;
use std::convert::Infallible;
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 @@ -58,9 +58,10 @@ impl Middleware for SecurityMiddleware {
/// `NewMiddleware` trait implementation.
impl NewMiddleware for SecurityMiddleware {
type Instance = Self;
type Err = Infallible;

/// Clones the current middleware to a new instance.
fn new_middleware(&self) -> io::Result<Self::Instance> {
fn new_middleware(&self) -> Result<Self, Infallible> {
Ok(self.clone())
}
}
6 changes: 4 additions & 2 deletions gotham/src/middleware/session/backend/memory.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::convert::Infallible;
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 @@ -71,8 +72,9 @@ impl Default for MemoryBackend {

impl NewBackend for MemoryBackend {
type Instance = MemoryBackend;
type Err = Infallible;

fn new_backend(&self) -> io::Result<Self::Instance> {
fn new_backend(&self) -> Result<Self::Instance, Infallible> {
Ok(self.clone())
}
}
Expand Down
6 changes: 4 additions & 2 deletions gotham/src/middleware/session/backend/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
pub(super) mod memory;

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

Expand All @@ -12,9 +12,11 @@ use crate::middleware::session::{SessionError, SessionIdentifier};
pub trait NewBackend: Sync + Clone + RefUnwindSafe {
/// The type of `Backend` created by the `NewBackend`.
type Instance: Backend + Send + 'static;
/// The type of error that might occur creating a new backend.
type Err: Error + Send + 'static;

/// Create and return a new `Backend` value.
fn new_backend(&self) -> io::Result<Self::Instance>;
fn new_backend(&self) -> Result<Self::Instance, Self::Err>;
}

/// Type alias for the trait objects returned by `Backend`.
Expand Down
17 changes: 8 additions & 9 deletions gotham/src/middleware/session/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//! Defines a session middleware with a pluggable backend.

use std::io;
use std::marker::PhantomData;
use std::ops::{Deref, DerefMut};
use std::panic::RefUnwindSafe;
Expand All @@ -16,6 +15,7 @@ use hyper::{Body, Response, StatusCode};
use log::{error, trace, warn};
use rand::RngCore;
use serde::{Deserialize, Serialize};
use thiserror::Error;

use super::cookie::CookieParser;
use super::{Middleware, NewMiddleware};
Expand All @@ -40,14 +40,17 @@ pub struct SessionIdentifier {
}

/// The kind of failure which occurred trying to perform a session operation.
#[derive(Debug)]
#[derive(Debug, Error)]
pub enum SessionError {
/// The backend failed, and the included message describes the problem.
#[error("The backend failed to return a session: {0}")]
Backend(String),
/// The session was unable to be deserialized.
#[error("The backend failed to deserialize the session")]
Deserialize,
/// Exhaustive match against this enum is unsupported.
#[doc(hidden)]
#[error("__NonExhaustive")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can simply specify #[non_exhaustive] on this enum if we bump MSRV to 1.40

__NonExhaustive,
}

Expand Down Expand Up @@ -487,8 +490,9 @@ where
T: Default + Serialize + for<'de> Deserialize<'de> + Send + 'static,
{
type Instance = SessionMiddleware<B::Instance, T>;
type Err = B::Err;

fn new_middleware(&self) -> io::Result<Self::Instance> {
fn new_middleware(&self) -> Result<Self::Instance, Self::Err> {
self.new_backend
.new_backend()
.map(|backend| SessionMiddleware {
Expand Down Expand Up @@ -1006,17 +1010,12 @@ where
}
Err(e) => {
error!(
"[{}] failed to retrieve session ({}) from backend: {:?}",
"[{}] failed to retrieve session ({}) from backend: {}",
state::request_id(&state),
identifier.value,
e
);

let e = io::Error::new(
io::ErrorKind::Other,
format!("backend failed to return session: {:?}", e),
);

future::err((state, e.into_handler_error()))
}
}
Expand Down
5 changes: 3 additions & 2 deletions gotham/src/middleware/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use crate::handler::HandlerFuture;
use crate::middleware::{Middleware, NewMiddleware};
use crate::state::{State, StateData};
use std::io;
use std::convert::Infallible;
use std::panic::RefUnwindSafe;
use std::pin::Pin;

Expand Down Expand Up @@ -61,9 +61,10 @@ where
T: Clone + RefUnwindSafe + StateData + Sync,
{
type Instance = Self;
type Err = Infallible;

/// Clones the current middleware to a new instance.
fn new_middleware(&self) -> io::Result<Self::Instance> {
fn new_middleware(&self) -> Result<Self, Infallible> {
Ok(self.clone())
}
}
6 changes: 3 additions & 3 deletions gotham/src/middleware/timer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ use crate::helpers::timing::Timer;
use crate::middleware::{Middleware, NewMiddleware};
use crate::state::State;
use futures::prelude::*;
use std::convert::Infallible;
use std::pin::Pin;

use std::io;

/// Middleware binding to attach request execution times inside headers.
///
/// This can be used to easily measure request time from outside the
Expand Down Expand Up @@ -44,9 +43,10 @@ impl Middleware for RequestTimer {
/// `NewMiddleware` trait implementation.
impl NewMiddleware for RequestTimer {
type Instance = Self;
type Err = Infallible;

/// Clones the current middleware to a new instance.
fn new_middleware(&self) -> io::Result<Self::Instance> {
fn new_middleware(&self) -> Result<Self, Infallible> {
Ok(self.clone())
}
}
Loading