Missing extensions are runtime errors rather than compile errors

[davidpdrsn]

Attempting to extract Extension<Foo> without having .layer(Extension(foo)) will result in a runtime error, rather than a compile error. That has been a long standing problem since 0.1. We don't have a good solution and ergonomic solution to it yet but I think its something we should experiment with for 0.6.

[davidpdrsn]

I have been thinking about this and have a proposal now.

The reason its hard in the first place is that Router::route takes any T: Service. So even if we added some state generic to Router it wouldn't be able to verify that the routes uses that same state type, as it just sees a generic T: Service.

My proposal is to change Router so instead its API looks like this

#[tokio::main]
async fn main() {
    let mut app = Router::new(AppState {});

    // can also consider having `Router::with_state(...)` and
    // then `Router::new()` just being `with_state(())`

    // add a route for `GET /` that goes to `handler`
    app.get("/", handler);

    // add routes for both `GET /foo` and `POST /foo`
    app.get("/foo", handler).post(handler);

    // this wouldn't work
    // the thing returned by `get` doesn't allow adding routes with new paths
    // just new methods
    // this we can still debate but its not strictly relevant to this issue
    // app.get("/something", handler).post("/something-else", bar);

    axum::Server::bind(...)
        .serve(app.into_make_service())
        .await
        .unwrap();
}

// the specific state our app needs
#[derive(Clone)]
struct AppState {}

async fn handler() {}

Now Router::{get, post, etc} can take the handler itself as the argument without any indirection.

Next add a new S (for state) type param to handler and accept the state in call:

pub trait Handler<S, T, B = Body>: Clone + Send + Sized + 'static {
    type Future: Future<Output = Response> + Send + 'static;

    fn call(self, state: S, req: Request<B>) -> Self::Future;
}

The signature of Router::get would be

impl<B> Router<B> {
    pub fn get<H, S, T>(mut self, path: &str, handler: T) -> Self
    where
        H: Handler<S, T, B>
    {
        // ...
    }
}

Next the same state is present in RequestParts

impl<S, B> RequestParts<S, B> {
    // only immutable access to the state
    pub fn state(&self) -> &S {
        &self.state
    }
}

This changes FromRequest to

#[async_trait]
pub trait FromRequest<S, B>: Sized {
    type Rejection: IntoResponse;

    async fn from_request(req: &mut RequestParts<S, B>) -> Result<Self, Self::Rejection>;
}

Next we add a State extractor that just accesses the state from RequestParts:

pub struct State<S>(pub S);

#[async_trait]
impl<S, B> FromRequest<S, B> for State<S>
where
    S: Clone
{
    type Rejection = Infallible;

    async fn from_request(req: &mut RequestParts<S, B>) -> Result<Self, Self::Rejection> {
        Ok(Self(req.state().clone()))
    }
}

I think that should work. Router::get binds the state param and its carried all the way through to the extractor. So if you extracted the wrong state type your handler wouldn't implement Handler<S, ...>. It would implement Handler for some other S2 type, which wouldn't compile.

nest, nest_service, and merge

nest, nest_service, and merge would work pretty much like they do today but I think should support "upcasting" the state so you can do app.nest("/api", api_router.map_state(Into::into)).

Routing to services

We should still provide Router::*_service methods for routing directly to Services:

let app = Router::new(());

app.get_service("/", some_service).post(handler).patch(service);

While this is a pretty big change to the API I think I'm fine with it. I don't love that we need .get(handler) and .get_service(service). Feels a bit redundant but don't think there is a way around that. Crucially its really only Router that changes. FromRequest and Handler change a bit but IntoResponse remains unchanged.

Thoughts?

[davidpdrsn]

Could also consider doing

app.at("/foo").get(handler).post(handler);

It is perhaps more symmetric 🤷 I haven't thought much about this and have to play around with it. Note this is exactly what tide's router does.

[jplatte]

This would be a lot of churn, removing MethodRouter completely. Could it possibly still exist with this API (but maybe not implement Service because it also needs to have that state passed when being invoked)?

[davidpdrsn]

Hm yeah that could work if MethodRouter doesn't implement Service. I didn't consider that.

I was a little worried that rust wouldn't be able to infer the state type if you do .route("/", get_service(...)) since get_service works for any state but that isn't a problem it turns out. See https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b8dc4cd60721fd4fd62dd1c36110f43c

Thats way nicer and should result in less churn.

Do you have an opinion about the changes to Handler, FromRequest, and friends?