Support fallbacks for nested routers

[davidpdrsn]

This branch includes #1086 so that should be reviewed and merged first.

---

This reworks how fallbacks are implemented to make them more consistent with
regular routes and fix #1053.

The big win is that nested fallbacks "just work":

let api = Router::new().fallback(api_fallback);
let app = Router::new().nest("/api", api).fallback(outer_fallback);

// GET /api/doesnt/exist goes to `api_fallback`
// GET /doesnt/exist goes to `outer_fallback`

I also made one breaking change for merging routers that only have "default
fallbacks" is done.

A default fallback is the one used when you haven't added a fallback with
Router::fallback. So if you do Router::new() that router has a default
fallback. Router::new().layer(...) adds a middleware to the default fallback.

When you merge routers that only have default fallbacks (regardless of
middleware) the combined router will have the fallback from the left hand side
router. I.e.:

let inner = Router::new();
let outer = Router::new();

// this will have the fallback from `outer`
let combined = outer.merge(inner);

// same applies if you have middleware

// `inner`'s default middleware now has a middleware
let inner = Router::new().layer(...);
let outer = Router::new();

// this will have the fallback from `outer`, i.e. the fallback with middleware
// `inner` has been dropped
let combined = outer.merge(inner);

I think this is the least surprising behavior since the routers you merge tends
to be "sub routers" so having their fallbacks overwrite the ones on the outer
router is surprising I think.

All this only applies to default fallbacks. Any custom fallbacks added with
Router::fallback are merged as normal and will panic if there are conflicts.

Additionally adding a fallback twice will now panic

Router::new().fallback(...).fallback(...)

This wouldn't panic previously and was inconsistent.

Fixes #1053

TODO

• docs
• changelog

[lilyball]

As mentioned in #1086 (comment), these docs need to be updated to describe how nested routers handle fallbacks, as well as the following example (which uses nest_service to allow the nested router to have a fallback).

[jplatte]

Partial review, will come back to this.

edit: Oh, I thought this was #1086 😅

[jplatte]

Suggested change

	When [fallbacks] are called differs between `nest` and `nested_service`. Routers
	nested with `nest` will delegate to the outer router's fallback if they don't
	have a matching route, whereas `nested_service` will not.
	When [fallbacks] are called differs between `nest` and `nest_service`. Routers
	nested with `nest` will delegate to the outer router's fallback if they don't
	have a matching route, whereas `nest_service` will not.

[jplatte]

Would it be correct to add this? It's the semantics I would intuitively expect.

Suggested change

	// the fallback is not called for request starting with `/assets` but will be
	// called for requests starting with `/api` if `nested_router` doesn't have
	// a matching route
	.fallback(fallback.into_service());
	// the fallback is not called for request starting with `/assets` but will be
	// called for requests starting with `/api` if `nested_router` doesn't have
	// a matching route, or its own fallback service
	.fallback(fallback.into_service());

[jplatte]

I don't know where the link definition for this is, but we should make sure to link to a specific version of tower-http, given the example below is going to stop working with the next breaking release (due to io::Error => Infallible).

[davidpdrsn]

My intention is so somewhat coordinate the next release of axum and tower-http but yeah will need to be updated.

[davidpdrsn]

Hehe yes #1086 need to be merged first as this builds that.

[davidpdrsn]

Maybe also worth trying to convert a layered default fallback into a custom fallback so it's not lost.

But have to think about the edge cases and what might be least surprising.

[davidpdrsn]

I'm starting to think that maybe we shouldn't even support nested routers calling the outer routers fallback if they don't have their own.

It's nice if you have one function that builds something like an API router to be able to nest that all at once. And it's a little inconvenient having to add the fallback both to the api and outer routers.

However it definitely is simpler to just not support it. Both implementation wise but also might be easier to understand. That way all services would be nested "opaquely" and require their own fallbacks. And we'd only have one nest function.

[jplatte]

If nest and nest_service are only separate to allow the fallback to apply for the nested Router (which I agree is not a clear win), I would also prefer not supporting that.

[davidpdrsn]

Yeah that was the only reason 😕 I've come around as well. Will change it when I'm back from vacation.

[lilyball]

Not supporting fallbacks from a nested router to the outer router would be a shame though. Right now I can use nested routers to split up my routes into a hierarchy, like nesting "/api" to the result of some api_routes() method. But this relies on fallback behavior, so I don't have to configure the exact same fallback on every nested router.

I do agree that there is some potential for confusion between "nested router with a configured fallback" and "nested router without a fallback", but that can be handled via documentation. Besides just documenting that nested routers without a fallback will defer to their parent router for fallback behavior, we could also emphasize the fact that a fallback acts as a low-priority catch-all route. It should be pretty obvious what the difference is between nesting a router that only has specific routes vs nesting one that has a catch-all route, and so that would help form an intuition about how nested fallbacks work.

All of this to say, while at the moment I'm not relying on nested router fallback behavior, it's how I was planning on structuring more complicated router setups in the future.

[davidpdrsn]

Not supporting fallbacks from a nested router to the outer router would be a shame though. Right now I can use nested routers to split up my routes into a hierarchy, like nesting "/api" to the result of some api_routes() method. But this relies on fallback behavior, so I don't have to configure the exact same fallback on every nested router.

I agree it is unfortunate and that exact use case I do wanna support, just not sure it's worth this additional complexity.

If we were to drop this change and require explicit fallbacks on nested routers, how many places would you have to set a fallback in your code specifically? If it is only for the api_router() then maybe it's not so bad?

I personally don't like building routers out of more than one level of nested routers but I'm sure some people split things up a lot.

[lilyball]

If we were to drop this change and require explicit fallbacks on nested routers, how many places would you have to set a fallback in your code specifically? If it is only for the api_router() then maybe it's not so bad?

I'd have to set it on every nested router I use that doesn't already have its own fallback. So that really depends on my route structure. My inclination right now is if I have a hierarchical route structure I might want to actually use a separate Router per level, so that way I can split responsibility up for each separate component. But I'd typically want to define my 404 behavior in one place instead of having to duplicate it everywhere.

If I have only one level of nesting then I can conceivably require the root to apply the same fallback to each router it nests, but the moment I have more than one level of nesting that breaks and I'd have to explicitly pass the global fallback to every single function so it can attach that to its own router. That's doable but rather annoying. And a hierarchical structure feels like a common thing to find in a complex API.

I agree it is unfortunate and that exact use case I do wanna support, just not sure it's worth this additional complexity.

How much additional complexity is for fallback behavior? My impression is that most of the complexity of nesting routers comes from merging the nested router's routes into the parent router. I'm assuming the only reason you don't just convert the fallback on the nested router into a catch-all route is because of route_layer? Is there any chance you could simply represent a fallback from a nested router as a catch-all using a new Endpoint type that route_layer knows to skip? Or is there something else I'm missing here?

[davidpdrsn]

I've decided not to support this its not worth the additional complexity it adds. See #1086 (comment).