From 8ee3119fb0c56357e9d829afe22879fca7cb1130 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Tue, 1 Jun 2021 17:17:10 +0200 Subject: [PATCH] More error handling of layered handlers --- src/handler.rs | 17 +++++++++++++++-- src/lib.rs | 44 ++++++++++++++++++++++++-------------------- src/tests.rs | 49 +++++++++++++++++++++++++++++++++++++------------ 3 files changed, 76 insertions(+), 34 deletions(-) diff --git a/src/handler.rs b/src/handler.rs index 10df0889..b9ddc3a7 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -1,5 +1,6 @@ -use crate::{body::Body, extract::FromRequest, response::IntoResponse}; +use crate::{body::Body, HandleError, extract::FromRequest, response::IntoResponse}; use async_trait::async_trait; +use bytes::Bytes; use futures_util::future; use http::{Request, Response}; use std::{ @@ -8,7 +9,7 @@ use std::{ marker::PhantomData, task::{Context, Poll}, }; -use tower::{Layer, Service, ServiceExt}; +use tower::{Layer, BoxError, Service, ServiceExt}; mod sealed { pub trait HiddentTrait {} @@ -141,6 +142,18 @@ impl Layered { _input: PhantomData, } } + + pub fn handle_error(self, f: F) -> Layered, T> + where + S: Service, Response = Response>, + F: FnOnce(S::Error) -> Res, + Res: IntoResponse, + B: http_body::Body + Send + Sync + 'static, + B::Error: Into + Send + Sync + 'static, + { + let svc = HandleError::new(self.svc, f); + Layered::new(svc) + } } pub struct HandlerSvc { diff --git a/src/lib.rs b/src/lib.rs index 47069f98..3a6a13f8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,6 +7,7 @@ use bytes::Bytes; use futures_util::ready; use http::{Request, Response}; use pin_project::pin_project; +use response::IntoResponse; use std::{ convert::Infallible, fmt, @@ -22,8 +23,8 @@ pub mod handler; pub mod response; pub mod routing; -pub use tower_http::add_extension::{AddExtension, AddExtensionLayer}; pub use async_trait::async_trait; +pub use tower_http::add_extension::{AddExtension, AddExtensionLayer}; #[cfg(test)] mod tests; @@ -114,20 +115,15 @@ impl ResultExt for Result { pub struct BoxStdError(#[source] pub(crate) tower::BoxError); pub trait ServiceExt: Service, Response = Response> { - fn handle_error(self, f: F) -> HandleError + fn handle_error(self, f: F) -> HandleError where Self: Sized, - F: FnOnce(Self::Error) -> Response, + F: FnOnce(Self::Error) -> Res, + Res: IntoResponse, B: http_body::Body + Send + Sync + 'static, B::Error: Into + Send + Sync + 'static, - NewBody: http_body::Body + Send + Sync + 'static, - NewBody::Error: Into + Send + Sync + 'static, { - HandleError { - inner: self, - f, - poll_ready_error: None, - } + HandleError::new(self, f) } } @@ -139,6 +135,16 @@ pub struct HandleError { poll_ready_error: Option, } +impl HandleError { + pub(crate) fn new(inner: S, f: F) -> Self { + Self { + inner, + f, + poll_ready_error: None, + } + } +} + impl fmt::Debug for HandleError where S: fmt::Debug, @@ -167,14 +173,13 @@ where } } -impl Service> for HandleError +impl Service> for HandleError where S: Service, Response = Response>, - F: FnOnce(S::Error) -> Response + Clone, + F: FnOnce(S::Error) -> Res + Clone, + Res: IntoResponse, B: http_body::Body + Send + Sync + 'static, B::Error: Into + Send + Sync + 'static, - NewBody: http_body::Body + Send + Sync + 'static, - NewBody::Error: Into + Send + Sync + 'static, { type Response = Response; type Error = Infallible; @@ -218,14 +223,13 @@ enum Kind { Error(Option), } -impl Future for HandleErrorFuture +impl Future for HandleErrorFuture where Fut: Future, E>>, - F: FnOnce(E) -> Response, + F: FnOnce(E) -> Res, + Res: IntoResponse, B: http_body::Body + Send + Sync + 'static, B::Error: Into + Send + Sync + 'static, - NewBody: http_body::Body + Send + Sync + 'static, - NewBody::Error: Into + Send + Sync + 'static, { type Output = Result, Infallible>; @@ -237,13 +241,13 @@ where Ok(res) => Ok(res.map(BoxBody::new)).into(), Err(err) => { let f = this.f.take().unwrap(); - let res = f(err); + let res = f(err).into_response(); Ok(res.map(BoxBody::new)).into() } }, KindProj::Error(err) => { let f = this.f.take().unwrap(); - let res = f(err.take().unwrap()); + let res = f(err.take().unwrap()).into_response(); Ok(res.map(BoxBody::new)).into() } } diff --git a/src/tests.rs b/src/tests.rs index 183f6881..b3d2ecf4 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -3,7 +3,10 @@ use http::{Request, Response, StatusCode}; use hyper::{Body, Server}; use serde::Deserialize; use serde_json::json; -use std::net::{SocketAddr, TcpListener}; +use std::{ + net::{SocketAddr, TcpListener}, + time::Duration, +}; use tower::{make::Shared, BoxError, Service}; #[tokio::test] @@ -77,9 +80,7 @@ async fn consume_body_to_json_requires_json_content_type() { let app = app() .at("/") - .post(|_: Request, input: extract::Json| async { - input.0.foo - }) + .post(|_: Request, input: extract::Json| async { input.0.foo }) .into_service(); let addr = run_in_background(app).await; @@ -274,7 +275,7 @@ async fn boxing() { #[tokio::test] async fn service_handlers() { - use crate::{body::BoxBody, ServiceExt as _}; + use crate::ServiceExt as _; use std::convert::Infallible; use tower::service_fn; use tower_http::services::ServeFile; @@ -290,13 +291,7 @@ async fn service_handlers() { .at("/static/Cargo.toml") .get_service( ServeFile::new("Cargo.toml").handle_error(|error: std::io::Error| { - // `ServeFile` internally maps some errors to `404` so we don't have - // to handle those here - let body = BoxBody::from(error.to_string()); - Response::builder() - .status(StatusCode::INTERNAL_SERVER_ERROR) - .body(body) - .unwrap() + (StatusCode::INTERNAL_SERVER_ERROR, error.to_string()) }), ) // calling boxed isn't necessary here but done so @@ -356,6 +351,36 @@ async fn middleware_on_single_route() { assert_eq!(body, "Hello, World!"); } +#[tokio::test] +async fn handling_errors_from_layered_single_routes() { + use tower::timeout::TimeoutLayer; + + async fn handle(_req: Request) -> &'static str { + tokio::time::sleep(Duration::from_secs(10)).await; + "" + } + + let app = app() + .at("/") + .get( + handle + .layer(TimeoutLayer::new(Duration::from_millis(100))) + .handle_error(|_error: BoxError| StatusCode::INTERNAL_SERVER_ERROR), + ) + .into_service(); + + let addr = run_in_background(app).await; + + let res = reqwest::get(format!("http://{}", addr)).await.unwrap(); + assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR); +} + +// TODO(david): .layer() on RouteBuilder +// TODO(david): composing two apps +// TODO(david): composing two apps with one at a "sub path" +// TODO(david): composing two boxed apps +// TODO(david): composing two apps that have had layers applied + /// Run a `tower::Service` in the background and get a URI for it. pub async fn run_in_background(svc: S) -> SocketAddr where