Panic instead of silently discarding fallbacks (#529)

This introduces two new possible panics when constructing routers:

- If merging two routers that each have a fallback. Previously that left
  side fallback would be silently discarded.
- If nesting a router that has a fallback. Previously it would be
  silently discarded.

Overall this should make things more explicit and users shouldn't have
to worry "why isn't my fallback" working.

Fixes #488

axum/CHANGELOG.md

@@ -28,10 +28,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **breaking:** The `Handler<B, T>` trait is now defined as `Handler<T, B =
   Body>`. That is the type parameters have been swapped and `B` defaults to
   `axum::body::Body` ([#527])
+- **breaking:** `Router::merge` will panic if both routers have fallbacks.
+  Previously the left side fallback would be silently discarded ([#529])
+- **breaking:** `Router::nest` will panic if the nested router has a fallback.
+  Previously it would be silently discarded ([#529])
 - Update WebSockets to use tokio-tungstenite 0.16 ([#525])
 
 [#525]: https://github.com/tokio-rs/axum/pull/525
 [#527]: https://github.com/tokio-rs/axum/pull/527
+[#529]: https://github.com/tokio-rs/axum/pull/529
 [#534]: https://github.com/tokio-rs/axum/pull/534
 
 # 0.3.3 (13. November, 2021)

axum/src/docs/routing/fallback.md

@@ -26,45 +26,3 @@ async fn fallback(uri: Uri) -> impl IntoResponse {
 Fallbacks only apply to routes that aren't matched by anything in the
 router. If a handler is matched by a request but returns 404 the
 fallback is not called.
-
-## When used with `Router::merge`
-
-If a router with a fallback is merged with another router that also has
-a fallback the fallback of the second router takes precedence:
-
-```rust
-use axum::{
-    Router,
-    routing::get,
-    handler::Handler,
-    response::IntoResponse,
-    http::{StatusCode, Uri},
-};
-
-let one = Router::new()
-    .route("/one", get(|| async {}))
-    .fallback(fallback_one.into_service());
-
-let two = Router::new()
-    .route("/two", get(|| async {}))
-    .fallback(fallback_two.into_service());
-
-let app = one.merge(two);
-
-async fn fallback_one() -> impl IntoResponse {}
-async fn fallback_two() -> impl IntoResponse {}
-
-// the fallback for `app` is `fallback_two`
-# async {
-# hyper::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap();
-# };
-```
-
-If only one of the routers have a fallback that will be used in the
-merged router.
-
-## When used with `Router::nest`
-
-If a router with a fallback is nested inside another router the fallback
-of the nested router will be discarded and not used. This is such that
-the outer router's fallback takes precedence.

axum/src/docs/routing/merge.md

@@ -36,3 +36,8 @@ let app = Router::new()
 # hyper::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap();
 # };
 ```
+
+## Panics
+
+- If two routers that each have a [fallback](Router::fallback) are merged. This
+  is because `Router` only allows a single fallback.

axum/src/docs/routing/nest.md

@@ -121,5 +121,7 @@ let app = Router::new()
 for more details.
 - If the route contains a wildcard (`*`).
 - If `path` is empty.
+- If the nested router has a [fallback](Router::fallback). This is because
+  `Router` only allows a single fallback.
 
 [`OriginalUri`]: crate::extract::OriginalUri

axum/src/routing/mod.rs

@@ -189,14 +189,17 @@ where
                 let Router {
                     mut routes,
                     node,
-                    // discard the fallback of the nested router
+                    fallback,
-                    fallback: _,
                     // nesting a router that has something nested at root
                     // doesn't mean something is nested at root in _this_ router
                     // thus we don't need to propagate that
                     nested_at_root: _,
                 } = router;
 
+                if let Fallback::Custom(_) = fallback {
+                    panic!("Cannot nest `Router`s that has a fallback");
+                }
+
                 for (id, nested_path) in node.route_id_to_path {
                     let route = routes.remove(&id).unwrap();
                     let full_path = if &*nested_path == "/" {
@@ -253,7 +256,9 @@ where
             (Fallback::Default(_), pick @ Fallback::Default(_)) => pick,
             (Fallback::Default(_), pick @ Fallback::Custom(_)) => pick,
             (pick @ Fallback::Custom(_), Fallback::Default(_)) => pick,
-            (Fallback::Custom(_), pick @ Fallback::Custom(_)) => pick,
+            (Fallback::Custom(_), Fallback::Custom(_)) => {
+                panic!("Cannot merge two `Router`s that both have a fallback")
+            }
         };
 
         self.nested_at_root = self.nested_at_root || nested_at_root;

axum/src/routing/tests/fallback.rs

@@ -49,25 +49,3 @@ async fn or() {
     assert_eq!(res.status(), StatusCode::OK);
     assert_eq!(res.text().await, "fallback");
 }
-
-#[tokio::test]
-async fn fallback_on_or() {
-    let one = Router::new()
-        .route("/one", get(|| async {}))
-        .fallback((|| async { "fallback one" }).into_service());
-
-    let two = Router::new()
-        .route("/two", get(|| async {}))
-        .fallback((|| async { "fallback two" }).into_service());
-
-    let app = one.merge(two);
-
-    let client = TestClient::new(app);
-
-    assert_eq!(client.get("/one").send().await.status(), StatusCode::OK);
-    assert_eq!(client.get("/two").send().await.status(), StatusCode::OK);
-
-    let res = client.get("/does-not-exist").send().await;
-    assert_eq!(res.status(), StatusCode::OK);
-    assert_eq!(res.text().await, "fallback two");
-}

axum/src/routing/tests/mod.rs

@@ -566,3 +566,21 @@ async fn different_methods_added_in_different_routes_deeply_nested() {
     let body = res.text().await;
     assert_eq!(body, "POST");
 }
+
+#[tokio::test]
+#[should_panic(expected = "Cannot merge two `Router`s that both have a fallback")]
+async fn merging_routers_with_fallbacks_panics() {
+    async fn fallback() {}
+    let one = Router::new().fallback(fallback.into_service());
+    let two = Router::new().fallback(fallback.into_service());
+    TestClient::new(one.merge(two));
+}
+
+#[tokio::test]
+#[should_panic(expected = "Cannot nest `Router`s that has a fallback")]
+async fn nesting_router_with_fallbacks_panics() {
+    async fn fallback() {}
+    let one = Router::new().fallback(fallback.into_service());
+    let app = Router::new().nest("/", one);
+    TestClient::new(app);
+}