Revamp error handling model (#402)

* Revamp error handling model

* changelog improvements and typo fixes

* Fix a few more Infallible bounds

* minor docs fixes

src/tests/handle_error.rs

@@ -1,6 +1,6 @@
 use super::*;
 use std::future::{pending, ready};
-use tower::{timeout::TimeoutLayer, MakeService};
+use tower::{timeout::TimeoutLayer, ServiceBuilder};
 
 async fn unit() {}
 
@@ -29,23 +29,17 @@ impl<R> Service<R> for Svc {
     }
 }
 
-fn check_make_svc<M, R, T, E>(_make_svc: M)
-where
-    M: MakeService<(), R, Response = T, Error = E>,
-{
-}
-
-fn handle_error<E>(_: E) -> Result<StatusCode, Infallible> {
-    Ok(StatusCode::INTERNAL_SERVER_ERROR)
-}
-
 #[tokio::test]
 async fn handler() {
     let app = Router::new().route(
         "/",
-        get(forever
-            .layer(timeout())
-            .handle_error(|_: BoxError| Ok::<_, Infallible>(StatusCode::REQUEST_TIMEOUT))),
+        get(forever.layer(
+            ServiceBuilder::new()
+                .layer(HandleErrorLayer::new(|_: BoxError| {
+                    StatusCode::REQUEST_TIMEOUT
+                }))
+                .layer(timeout()),
+        )),
     );
 
     let client = TestClient::new(app);
@@ -58,9 +52,13 @@ async fn handler() {
 async fn handler_multiple_methods_first() {
     let app = Router::new().route(
         "/",
-        get(forever
-            .layer(timeout())
-            .handle_error(|_: BoxError| Ok::<_, Infallible>(StatusCode::REQUEST_TIMEOUT)))
+        get(forever.layer(
+            ServiceBuilder::new()
+                .layer(HandleErrorLayer::new(|_: BoxError| {
+                    StatusCode::REQUEST_TIMEOUT
+                }))
+                .layer(timeout()),
+        ))
         .post(unit),
     );
 
@@ -76,9 +74,13 @@ async fn handler_multiple_methods_middle() {
         "/",
         delete(unit)
             .get(
-                forever
-                    .layer(timeout())
-                    .handle_error(|_: BoxError| Ok::<_, Infallible>(StatusCode::REQUEST_TIMEOUT)),
+                forever.layer(
+                    ServiceBuilder::new()
+                        .layer(HandleErrorLayer::new(|_: BoxError| {
+                            StatusCode::REQUEST_TIMEOUT
+                        }))
+                        .layer(timeout()),
+                ),
             )
             .post(unit),
     );
@@ -94,9 +96,13 @@ async fn handler_multiple_methods_last() {
     let app = Router::new().route(
         "/",
         delete(unit).get(
-            forever
-                .layer(timeout())
-                .handle_error(|_: BoxError| Ok::<_, Infallible>(StatusCode::REQUEST_TIMEOUT)),
+            forever.layer(
+                ServiceBuilder::new()
+                    .layer(HandleErrorLayer::new(|_: BoxError| {
+                        StatusCode::REQUEST_TIMEOUT
+                    }))
+                    .layer(timeout()),
+            ),
         ),
     );
 
@@ -105,82 +111,3 @@ async fn handler_multiple_methods_last() {
     let res = client.get("/").send().await;
     assert_eq!(res.status(), StatusCode::REQUEST_TIMEOUT);
 }
-
-#[test]
-fn service_propagates_errors() {
-    let app = Router::new().route("/echo", service::post::<_, Body>(Svc));
-
-    check_make_svc::<_, _, _, hyper::Error>(app.into_make_service());
-}
-
-#[test]
-fn service_nested_propagates_errors() {
-    let app = Router::new().route(
-        "/echo",
-        Router::new().nest("/foo", service::post::<_, Body>(Svc)),
-    );
-
-    check_make_svc::<_, _, _, hyper::Error>(app.into_make_service());
-}
-
-#[test]
-fn service_handle_on_method() {
-    let app = Router::new().route(
-        "/echo",
-        service::get::<_, Body>(Svc).handle_error(handle_error::<hyper::Error>),
-    );
-
-    check_make_svc::<_, _, _, Infallible>(app.into_make_service());
-}
-
-#[test]
-fn service_handle_on_method_multiple() {
-    let app = Router::new().route(
-        "/echo",
-        service::get::<_, Body>(Svc)
-            .post(Svc)
-            .handle_error(handle_error::<hyper::Error>),
-    );
-
-    check_make_svc::<_, _, _, Infallible>(app.into_make_service());
-}
-
-#[test]
-fn service_handle_on_router() {
-    let app = Router::new()
-        .route("/echo", service::get::<_, Body>(Svc))
-        .handle_error(handle_error::<hyper::Error>);
-
-    check_make_svc::<_, _, _, Infallible>(app.into_make_service());
-}
-
-#[test]
-fn service_handle_on_router_still_impls_routing_dsl() {
-    let app = Router::new()
-        .route("/echo", service::get::<_, Body>(Svc))
-        .handle_error(handle_error::<hyper::Error>)
-        .route("/", get(unit));
-
-    check_make_svc::<_, _, _, Infallible>(app.into_make_service());
-}
-
-#[test]
-fn layered() {
-    let app = Router::new()
-        .route("/echo", get::<_, Body, _>(unit))
-        .layer(timeout())
-        .handle_error(handle_error::<BoxError>);
-
-    check_make_svc::<_, _, _, Infallible>(app.into_make_service());
-}
-
-#[tokio::test] // async because of `.boxed()`
-async fn layered_boxed() {
-    let app = Router::new()
-        .route("/echo", get::<_, Body, _>(unit))
-        .layer(timeout())
-        .boxed()
-        .handle_error(handle_error::<BoxError>);
-
-    check_make_svc::<_, _, _, Infallible>(app.into_make_service());
-}

src/tests/mod.rs

@@ -1,5 +1,6 @@
 #![allow(clippy::blacklisted_name)]
 
+use crate::error_handling::HandleErrorLayer;
 use crate::BoxError;
 use crate::{
     extract::{self, Path},
@@ -339,7 +340,7 @@ async fn middleware_on_single_route() {
 
 #[tokio::test]
 async fn service_in_bottom() {
-    async fn handler(_req: Request<Body>) -> Result<Response<Body>, hyper::Error> {
+    async fn handler(_req: Request<Body>) -> Result<Response<Body>, Infallible> {
         Ok(Response::new(hyper::Body::empty()))
     }
 
@@ -532,7 +533,9 @@ async fn middleware_applies_to_routes_above() {
     let app = Router::new()
         .route("/one", get(std::future::pending::<()>))
         .layer(TimeoutLayer::new(Duration::new(0, 0)))
-        .handle_error(|_: BoxError| Ok::<_, Infallible>(StatusCode::REQUEST_TIMEOUT))
+        .layer(HandleErrorLayer::new(|_: BoxError| {
+            StatusCode::REQUEST_TIMEOUT
+        }))
         .route("/two", get(|| async {}));
 
     let client = TestClient::new(app);

src/tests/nest.rs

@@ -1,5 +1,6 @@
 use super::*;
 use crate::body::box_body;
+use crate::error_handling::HandleErrorExt;
 use crate::routing::EmptyRouter;
 use std::collections::HashMap;
 
@@ -169,10 +170,10 @@ async fn nest_static_file_server() {
     let app = Router::new().nest(
         "/static",
         service::get(tower_http::services::ServeDir::new(".")).handle_error(|error| {
-            Ok::<_, Infallible>((
+            (
                 StatusCode::INTERNAL_SERVER_ERROR,
                 format!("Unhandled internal error: {}", error),
-            ))
+            )
         }),
     );
 
@@ -255,5 +256,5 @@ async fn multiple_top_level_nests() {
 #[tokio::test]
 #[should_panic(expected = "Invalid route: nested routes cannot contain wildcards (*)")]
 async fn nest_cannot_contain_wildcards() {
-    Router::<EmptyRouter>::new().nest("/one/*rest", Router::<EmptyRouter>::new());
+    Router::<EmptyRouter>::new().nest::<_, Body>("/one/*rest", Router::<EmptyRouter>::new());
 }

src/tests/or.rs

@@ -1,5 +1,5 @@
 use super::*;
-use crate::{extract::OriginalUri, response::IntoResponse, Json};
+use crate::{error_handling::HandleErrorLayer, extract::OriginalUri, response::IntoResponse, Json};
 use serde_json::{json, Value};
 use tower::{limit::ConcurrencyLimitLayer, timeout::TimeoutLayer};
 
@@ -136,7 +136,7 @@ async fn layer_and_handle_error() {
     let two = Router::new()
         .route("/timeout", get(futures::future::pending::<()>))
         .layer(TimeoutLayer::new(Duration::from_millis(10)))
-        .handle_error(|_| Ok(StatusCode::REQUEST_TIMEOUT));
+        .layer(HandleErrorLayer::new(|_| StatusCode::REQUEST_TIMEOUT));
     let app = one.or(two);
 
     let client = TestClient::new(app);