loco-rs / loco

🚂 🦀 The one-person framework for Rust for side-projects and startups
https://loco.rs
Apache License 2.0
3.85k stars 163 forks source link

Add ability to provide fallback handler to AppRoutes #643

Closed swlody closed 1 month ago

swlody commented 1 month ago

Feature Request

Currently, controller::AppRoutes::to_router() constructs an axum::Router with no fallback function. This means that it is not possible to have e.g. a custom 404 handler. It is possible to get around this by implementing Hooks::after_routes() and calling Router::fallback() on the provided axum::Router. However, the after_routes() callback is initiated after AppRoutes::to_router() is called https://github.com/loco-rs/loco/blob/7b716a73bf602c9a0aec95b0c72ac42791cf6305/src/boot.rs#L268-L269. This means that any middleware that was added is not provided to the fallback handler.

Describe the solution you'd like

Add a add_fallback(fallback: Handler) method to controller::AppRoutes so that a fallback handler can be added to the axum::Router and have all the relevant middleware applied to it.

This is potentially difficult to achieve since axum::Router::fallback() accepts an axum::handler::Handler<T, S> so AppRoutes would also need to be templated on <T, S> to be able to store the fallback on it.

Describe alternatives you've considered

Alternate solution is to move the call to after_routes() into controller::AppRoutes::to_router() so that it is actually applied immediately after the routes are registered. https://github.com/loco-rs/loco/blob/7b716a73bf602c9a0aec95b0c72ac42791cf6305/src/controller/app_routes.rs#L184-L190

This is however a potentially breaking change. Another problem with this method is that it would separate the location where Hooks::after_routes() is called from the location where Initializer::after_routes() is called. Also, AppRoutes::to_router() is not an async function while Hooks::after_routes() is (why?).

kaplanelad commented 1 month ago

@swlod, you can use after_routes hook, which gets the axum router. this is the place you can add the fallback custom function

swlody commented 1 month ago

@swlod, you can use after_routes hook, which gets the axum router. this is the place you can add the fallback custom function

Maybe I'm missing something here. The problem that I am having using after_routes is that any fallback added in this function doesn't have any of the default middleware applied to it because it is called after the middleware is added to the router.

swlody commented 1 month ago

One more potential solution that seems to work ok:

Add a new function to Hooks: router(). The user can optionally "seed" the router by returning a router from this function. In Hooks:

fn router(ctx: &AppContext) -> Result<AxumRouter> {
    Self::routes(ctx).to_router(ctx.clone(), AxumRouter::new())
}

In boot.rs:

// Old:
let app = H::routes(&app_context).to_router(app_context.clone())?;

// New:
let app = H::router(&app_context)?;

In AppRoutes:

// Old
pub fn to_router(&self, ctx: AppContext) -> Result<AXRouter> {
    let mut app = AXRouter::new();
    ...
}

// New:
pub fn to_router(
    &self,
    ctx: AppContext,
    mut app: AXRouter<AppContext>,
) -> Result<AXRouter> {
    ...
}

User implementation:

fn router(ctx: &AppContext) -> Result<Router> {
    // Anything that we want to add to the router before the middleware is initialized
    let router = Router::new().fallback(/* some fallback_handler */);

    let router = Self::routes(ctx).to_router(ctx.clone(), router)?;

    // Any functionality from `after_hooks` can go here instead:
    let router = router.layer(/* some layer */);
    Ok(router)
}

This gives more control than the after_routes() hook, which could be removed.