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

Breaking Change: replace Reject trait with Into<Rejection> #527

Closed
wants to merge 7 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions examples/rejections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ use std::num::NonZeroU16;

use serde_derive::Serialize;
use warp::http::StatusCode;
use warp::{reject, Filter, Rejection, Reply};
use warp::{
reject::{self, Debug},
Filter, Rejection, Reply,
};

/// Rejections represent cases where a filter should not continue processing
/// the request, but a different filter *could* process it.
Expand Down Expand Up @@ -39,8 +42,6 @@ fn div_by() -> impl Filter<Extract = (NonZeroU16,), Error = Rejection> + Copy {
#[derive(Debug)]
struct DivideByZero;

impl reject::Reject for DivideByZero {}

// JSON replies

/// A successful math operation.
Expand Down Expand Up @@ -76,7 +77,7 @@ async fn handle_rejection(err: Rejection) -> Result<impl Reply, Infallible> {
message = "METHOD_NOT_ALLOWED";
} else {
// We should have expected this... Just log and say its a 500
eprintln!("unhandled rejection: {:?}", err);
eprintln!("unhandled rejection: {:?}", err.debug());
code = StatusCode::INTERNAL_SERVER_ERROR;
message = "UNHANDLED_REJECTION";
}
Expand Down
1 change: 0 additions & 1 deletion examples/sse_chat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ enum Message {

#[derive(Debug)]
struct NotUtf8;
impl warp::reject::Reject for NotUtf8 {}

/// Our state of currently connected users.
///
Expand Down
3 changes: 2 additions & 1 deletion src/filter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use self::recover::Recover;
use self::unify::Unify;
use self::untuple_one::UntupleOne;
pub(crate) use self::wrap::{Wrap, WrapSealed};
use crate::reject::Debug;

// A crate-private base trait, allowing the actual `filter` method to change
// signatures without it being a breaking change.
Expand All @@ -45,7 +46,7 @@ pub trait FilterBase {
where
Self: Sized,
F: Fn(Self::Error) -> E + Clone,
E: ::std::fmt::Debug + Send,
E: Debug + Send,
{
MapErr {
filter: self,
Expand Down
4 changes: 2 additions & 2 deletions src/filter/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use futures::future::TryFuture;
use hyper::service::Service;
use pin_project::pin_project;

use crate::reject::IsReject;
use crate::reject::{Debug, IsReject};
use crate::reply::{Reply, Response};
use crate::route::{self, Route};
use crate::{Filter, Request};
Expand Down Expand Up @@ -129,7 +129,7 @@ where
Poll::Ready(Ok(ok)) => Poll::Ready(Ok(ok.into_response())),
Poll::Pending => Poll::Pending,
Poll::Ready(Err(err)) => {
log::debug!("rejected: {:?}", err);
log::debug!("rejected: {:?}", err.debug());
Poll::Ready(Ok(err.into_response()))
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/filters/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,7 @@ unit_error! {
#[cfg(test)]
mod tests {
use super::sanitize_path;
use crate::reject::Debug;
use bytes::BytesMut;

#[test]
Expand All @@ -476,7 +477,9 @@ mod tests {
}

assert_eq!(
sanitize_path(base, "/foo.html").unwrap(),
sanitize_path(base, "/foo.html")
.map_err(|r| panic!("{:?}", r.debug()))
.unwrap(),
p("/var/www/foo.html")
);

Expand Down
3 changes: 3 additions & 0 deletions src/filters/sse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ tuple_fmt!((A, B, C, D, E, F, G, H) => (0, 1, 2, 3, 4, 5, 6, 7));
/// Typically this identifier represented as number or string.
///
/// ```
/// use warp::reject::Debug;
/// let app = warp::sse::last_event_id::<u32>();
///
/// // The identifier is present
Expand All @@ -276,6 +277,7 @@ tuple_fmt!((A, B, C, D, E, F, G, H) => (0, 1, 2, 3, 4, 5, 6, 7));
/// .header("Last-Event-ID", "12")
/// .filter(&app)
/// .await
/// .map_err(|r| panic!("{:?}", r.debug()))
/// .unwrap(),
/// Some(12)
/// );
Expand All @@ -285,6 +287,7 @@ tuple_fmt!((A, B, C, D, E, F, G, H) => (0, 1, 2, 3, 4, 5, 6, 7));
/// warp::test::request()
/// .filter(&app)
/// .await
/// .map_err(|r| panic!("{:?}", r.debug()))
/// .unwrap(),
/// None
/// );
Expand Down
86 changes: 46 additions & 40 deletions src/reject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
//! ```

use std::any::Any;
use std::any::TypeId;
use std::convert::Infallible;
use std::error::Error as StdError;
use std::fmt;
Expand Down Expand Up @@ -112,38 +113,25 @@ pub(crate) fn unsupported_media_type() -> Rejection {
/// or else this will be returned as a `500 Internal Server Error`.
///
/// [`recover`]: ../trait.Filter.html#method.recover
pub fn custom<T: Reject>(err: T) -> Rejection {
Rejection::custom(Box::new(err))
pub fn custom(err: impl Into<Rejection>) -> Rejection {
err.into()
}

/// Protect against re-rejecting a rejection.
///
/// ```compile_fail
/// fn with(r: warp::Rejection) {
/// let _wat = warp::reject::custom(r);
/// }
/// ```
fn __reject_custom_compilefail() {}

/// A marker trait to ensure proper types are used for custom rejections.
///
/// # Example
///
/// ```
/// use warp::{Filter, reject::Reject};
/// use warp::Filter;
/// use std::fmt;
///
/// #[derive(Debug)]
/// struct RateLimited;
///
/// impl Reject for RateLimited {}
///
/// let route = warp::any().and_then(|| async {
/// Err::<(), _>(warp::reject::custom(RateLimited))
/// });
/// ```
// Require `Sized` for now to prevent passing a `Box<dyn Reject>`, since we
// would be double-boxing it, and the downcasting wouldn't work as expected.
pub trait Reject: fmt::Debug + Sized + Send + Sync + 'static {}

trait Cause: fmt::Debug + Send + Sync + 'static {
fn as_any(&self) -> &dyn Any;
Expand Down Expand Up @@ -171,6 +159,7 @@ pub(crate) fn known<T: Into<Known>>(err: T) -> Rejection {
/// Rejection of a request by a [`Filter`](crate::Filter).
///
/// See the [`reject`](module@crate::reject) documentation for more.
#[allow(missing_debug_implementations)]
pub struct Rejection {
reason: Reason,
}
Expand Down Expand Up @@ -267,12 +256,6 @@ impl Rejection {
}
}

fn custom(other: Box<dyn Cause>) -> Self {
Rejection {
reason: Reason::Other(Box::new(Rejections::Custom(other))),
}
}

/// Searches this `Rejection` for a specific cause.
///
/// A `Rejection` will accumulate causes over a `Filter` chain. This method
Expand All @@ -281,11 +264,11 @@ impl Rejection {
/// # Example
///
/// ```
/// use std::fmt;
///
/// #[derive(Debug)]
/// struct Nope;
///
/// impl warp::reject::Reject for Nope {}
///
/// let reject = warp::reject::custom(Nope);
///
/// if let Some(nope) = reject.find::<Nope>() {
Expand Down Expand Up @@ -317,10 +300,24 @@ impl Rejection {
}
}

impl From<Infallible> for Rejection {
#[inline]
fn from(infallible: Infallible) -> Rejection {
match infallible {}
impl<T> From<T> for Rejection
where
T: fmt::Debug + Sized + Send + Sync + 'static,
{
fn from(inner: T) -> Self {
if TypeId::of::<T>() == TypeId::of::<Infallible>() {
unimplemented!()
} else {
Rejection {
reason: Reason::Other(Box::new(Rejections::Custom(Box::new(inner)))),
}
}
}
}

impl Debug for Infallible {
fn debug(&self) -> Box<dyn fmt::Debug> {
match *self {}
}
}

Expand Down Expand Up @@ -354,9 +351,23 @@ impl IsReject for Rejection {
}
}

impl fmt::Debug for Rejection {
///
pub trait Debug {
///
fn debug(&self) -> Box<dyn std::fmt::Debug + '_>;
}

impl Debug for Rejection {
fn debug(&self) -> Box<dyn fmt::Debug + '_> {
Box::new(RejectionDebugger(self))
}
}

struct RejectionDebugger<'a>(&'a Rejection);

impl fmt::Debug for RejectionDebugger<'_> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.debug_tuple("Rejection").field(&self.reason).finish()
f.debug_tuple("Rejection").field(&self.0.reason).finish()
}
}

Expand Down Expand Up @@ -564,14 +575,13 @@ impl ::std::fmt::Display for MissingCookie {
impl StdError for MissingCookie {}

mod sealed {
use super::{Reason, Rejection, Rejections};
use super::{Debug, Reason, Rejection, Rejections};
use http::StatusCode;
use std::convert::Infallible;
use std::fmt;

// This sealed trait exists to allow Filters to return either `Rejection`
// or `!`. There are no other types that make sense, and so it is sealed.
pub trait IsReject: fmt::Debug + Send + Sync {
pub trait IsReject: Debug + Send + Sync {
fn status(&self) -> StatusCode;
fn into_response(&self) -> crate::reply::Response;
}
Expand Down Expand Up @@ -672,9 +682,6 @@ mod tests {
#[derive(Debug, PartialEq)]
struct Right;

impl Reject for Left {}
impl Reject for Right {}

#[test]
fn rejection_status() {
assert_eq!(not_found().status(), StatusCode::NOT_FOUND);
Expand Down Expand Up @@ -784,12 +791,11 @@ mod tests {

#[derive(Debug)]
struct X(u32);
impl Reject for X {}

fn combine_n<F, R>(n: u32, new_reject: F) -> Rejection
where
F: Fn(u32) -> R,
R: Reject,
R: Into<Rejection>,
{
let mut rej = not_found();

Expand All @@ -804,7 +810,7 @@ mod tests {
fn test_debug() {
let rej = combine_n(3, X);

let s = format!("{:?}", rej);
let s = format!("{:?}", rej.debug());
assert_eq!(s, "Rejection([X(0), X(1), X(2)])");
}
}
5 changes: 4 additions & 1 deletion src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ use serde_json;
use tokio::sync::{mpsc, oneshot};

use crate::filter::Filter;
use crate::reject::Debug;
use crate::reject::IsReject;
use crate::reply::Reply;
use crate::route::{self, Route};
Expand Down Expand Up @@ -290,13 +291,15 @@ impl RequestBuilder {
/// # Example
///
/// ```no_run
/// use warp::reject::Debug;
/// async {
/// let param = warp::path::param::<u32>();
///
/// let ex = warp::test::request()
/// .path("/41")
/// .filter(&param)
/// .await
/// .map_err(|r| panic!("{:?}", r.debug()))
/// .unwrap();
///
/// assert_eq!(ex, 41);
Expand Down Expand Up @@ -372,7 +375,7 @@ impl RequestBuilder {
let res = match result {
Ok(rep) => rep.into_response(),
Err(rej) => {
log::debug!("rejected: {:?}", rej);
log::debug!("rejected: {:?}", rej.debug());
rej.into_response()
}
};
Expand Down