Add a method-not-allowed fallback on the Router level

[ThibSrb]

• I have looked for existing issues (including closed) about this

Feature Request

Motivation

It's a common mistake to expect Router::fallback to be triggered on method-not-allowed.
Furthermore, as shown by #1005, it is pretty easy to misinterpret the current behavior for a bug.
While returning HTTP 405 is normal in this case, it would be helpful to have a simple baked-in solution to handle it, in a way similar to fallback.

Proposal

Add a Router::method_not_allowed_fallback method that allows defining a fallback for this specific situation

Usage

use std::net::SocketAddr;

use axum::{routing::get, Router, response::IntoResponse};
use hyper::Server;

async fn hello_world() -> impl IntoResponse {
    "Hello, world!\n"
}

async fn default_fallback() -> impl IntoResponse {
    "Default fallback\n"
}

async fn handle_405() -> impl IntoResponse {
    "Method not allowed fallback"
}

#[tokio::main]
async fn main() -> Result<(), hyper::Error> {
    let router = Router::new()
        .route("/", get(hello_world))
        .fallback(default_fallback)
        .method_not_allowed_fallback(handle_405);

    let addr = SocketAddr::from(([127, 0, 0, 1], 3000));

    Server::bind(&addr).serve(router.into_make_service()).await
}

Alternatives

An included middleware such as MethodNotAllowedLayer(fallback: Handler).

Usage

use std::net::SocketAddr;

use axum::{response::IntoResponse, routing::get, Router, middleware::MethodNotAllowedLayer};
use hyper::Server;

async fn hello_world() -> impl IntoResponse {
    "Hello, world!\n"
}

async fn default_fallback() -> impl IntoResponse {
    "Default fallback\n"
}

async fn handle_405() -> impl IntoResponse {
    "Method not allowed fallback"
}

#[tokio::main]
async fn main() -> Result<(), hyper::Error> {
    let router = Router::new()
        .route("/", get(hello_world))
        .layer(MethodNotAllowedLayer(handle_405))
        .fallback(default_fallback);

    let addr = SocketAddr::from(([127, 0, 0, 1], 3000));

    Server::bind(&addr).serve(router.into_make_service()).await
}

Anyway, I think it would be great to mention this topic in the Router::fallback documentation.

[davidpdrsn]

I wonder if we should just apply the normal Router::fallback to both 404 and 405 (keeping the Allow header of course). Do you think that would work for most cases? Is it often that you want different fallbacks for 404 and 405?

[jplatte]

Wdym by keeping the allow header? You mean setting it if the user-provided fallback handler / service hasn't set it? What about the status code, currently it's also the fallback handler / service's task to set it to 404 / 405 if appropriate.

[davidpdrsn]

Hm that's a good point. Didn't consider that 🤔 Not sure how to deal with that. It feels like it should still be possible to set the Allow header even if using something like Router::method_not_allowed_fallback. Otherwise you'd have to remember to set and update that yourself.

[GenericNerd]

I would like to pitch in on this, this issue is something that I'm currently coming across and it's rather annoying to not have a good alternative that will cover the method not found attribute.

For a little bit of context, our API implementation is trying to return JSON for most of the common client errors (the fallback handler currently covers the 404 case).

Having either a method_not_allowed_fallback or something similar would be incredible!

[manortec]

I am using axum as an api backend, I hope this issue could be solved.

[Lachstec]

Hey there, I would volunteer to work on this, if it is still relevant.

[jplatte]

It is. One recommendation regarding the implementation: it might be good to only set the Allow header if the fallback service didn't set it. But we can further discuss that on the PR if there's any questions around that, not important to get that bit right immediately.

[Lachstec]

@jplatte What do you think would be the most elegant way on implementing it? Im familiar with axum, but this is my first contribution and im not that well versed in the internals. My first intuition would be to go with MethodRouters fallback, but I'm sure that there is a better way 😅.

[jplatte]

I haven't looked at this code in a while, but I'd try to make Router call some private method on every MethodRouter that gets added via .route. This method, let's call it default_fallback, is the same as fallback except it does nothing if an explicit fallback has already been set on the MethodRouter by the user.

[emwalker]

I was also looking for a way to return a JSON response to method-not-allowed rejections, so I'll be interested in whatever PR comes out of this.

[Lachstec]

Sorry for keeping you waiting, wasn't available for the past two weeks. Ive pushed an attempt of mine to implement it, the example from OP works as expected now, although the CI pipeline is currently failing. Feedback would be appreciated as I am currently not confident in opening a real PR for this 😅

[jplatte]

That PR is as real as PRs get 😉
Feel free to ping me again if I haven't responded on the PR by Friday.

[Lachstec]

Hi there @jplatte, sorry for letting this fall behind for so long. CI is passing now, would be nice if you could give some feedback.

[jplatte]

No worries. I should have probably left some useful comments earlier already, before you came back to fix CI.