Router::with_state_arc might be misleading

[denschub]

Note that this probably is neither a Bug Report or a Feature Request, and just a me-problem. But I got confused by it and @davidpdrsn asked me to file a bug on Reddit. :)

Bug Report

Version

0.6.0-rc.1

Description

I have a small app that has a non-Clone'able state, so I've been using an Arc in an Extension for now. I just poked around with the new State in 0.6.0-rc.1, and found axum::routing::Router::with_state_arc.

To me, this function felt like an invitation to pass an Arc to something Unclonable into it:

struct UnclonableExample(usize);

#[tokio::main]
async fn main() {
    let state = UnclonableExample(42);
    let state_arc = Arc::new(state);
    let router = Router::with_state_arc(state_arc).route("/", get(handler));
    // ...
}

async fn handler(State(state): State<Arc<UnclonableExample>>) -> impl IntoResponse {
    Html("<h1>foo</h1>")
}

But that doesn't work. This specific example fails with

error[E0308]: mismatched types
   --> src/main.rs:16:63
    |
16  |     let router = Router::with_state_arc(state_arc).route("/", get(handler));
    |                                                    -----      ^^^^^^^^^^^^ expected struct `UnclonableExample`, found struct `Arc`
    |                                                    |
    |                                                    arguments to this function are incorrect
    |
    = note: expected struct `MethodRouter<UnclonableExample, _>`
               found struct `MethodRouter<Arc<UnclonableExample>, _>`
note: associated function defined here
   --> /Users/denschub/.cargo/registry/src/github.com-1ecc6299db9ec823/axum-0.6.0-rc.1/src/routing/mod.rs:154:12
    |
154 |     pub fn route(mut self, path: &str, method_router: MethodRouter<S, B>) -> Self {
    |            ^^^^^

For more information about this error, try `rustc --explain E0308`.

and changing the function signature to State<UnclonableExample> of course also doesn't work, with a rather cryptic compiler error:

error[E0277]: the trait bound `fn(State<UnclonableExample>) -> impl Future<Output = impl IntoResponse> {handler}: Handler<_, _, _>` is not satisfied
   --> src/main.rs:16:67
    |
16  |     let router = Router::with_state_arc(state_arc).route("/", get(handler));
    |                                                               --- ^^^^^^^ the trait `Handler<_, _, _>` is not implemented for `fn(State<UnclonableExample>) -> impl Future<Output = impl IntoResponse> {handler}`
    |                                                               |
    |                                                               required by a bound introduced by this call
    |
    = help: the trait `Handler<T, S, B>` is implemented for `Layered<L, H, T, S, B>`
note: required by a bound in `axum::routing::get`
   --> /Users/denschub/.cargo/registry/src/github.com-1ecc6299db9ec823/axum-0.6.0-rc.1/src/routing/method_routing.rs:400:1
    |
400 | top_level_handler_fn!(get, GET);
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `axum::routing::get`
    = note: this error originates in the macro `top_level_handler_fn` (in Nightly builds, run with -Z macro-backtrace for more info)

The working solution is

let state = UnclonableExample(42);
let state_arc = Arc::new(state);
let router = Router::with_state(state_arc).route("/", get(handler));

and extracting it with

async fn handler(State(state): State<Arc<UnclonableExample>>) -> impl IntoResponse

but I wonder if others might be temporarily confused by this as well, and if maybe the documentation can be expanded. Right now, it just says

Create a new Router with the given Arc'ed state.

and it's not really indicating that you're not supposed to use this in an attempt to pass your Arc'ed unclonable state around. But I'm also happy with this being closed without action. 😄

[davidpdrsn]

Yeah I see what the problem is 😕 Router::with_state_arc takes an Arc<S> thus it expects S to be the state. In your case that is UnclonableExample, but what you're trying to extract is Arc<UnclonableExample>.

A silly workaround is to extract State<Arc<Arc<UnclonableExample>>> like so

use axum::{extract::State, routing::get, Router};
use std::sync::Arc;

struct UnclonableExample(usize);

#[tokio::main]
async fn main() {
    let state = UnclonableExample(42);
    let state_arc = Arc::new(state);
    let router: Router<_> = Router::with_state_arc(state_arc).route("/", get(handler));
}

#[axum::debug_handler]
async fn handler(State(state): State<Arc<Arc<UnclonableExample>>>) {}

This isn't great and something we try and address.

[davidpdrsn]

With that said State requires Clone because of https://github.com/tokio-rs/axum/blob/main/axum-core/src/extract/from_ref.rs#L18.

This this issue is about removing the double Arc.

[davidpdrsn]

Honestly that seems to be the fix. Making your state Clone:

#[derive(Clone)] // <---
struct UnclonableExample(usize);

#[tokio::main]
async fn main() {
    let state = UnclonableExample(42);
    let state_arc = Arc::new(state);
    let router: Router<_> = Router::with_state_arc(state_arc).route("/", get(handler));
}

async fn handler(State(state): State<UnclonableExample>) {}

[denschub]

Honestly that seems to be the fix. Making your state Clone:

Sure, that's possible in this case, but not everything is Clone'able, or one might not always want do that (if it's lots of data for whatever reason). But I'm fine with just passing an Arc into with_state() in those cases, as I mentioned in the end of my original comment.

[davidpdrsn]

Sure, that's possible in this case, but not everything is Clone'able, or one might not always want do that (if it's lots of data for whatever reason). But I'm fine with just passing an Arc into with_state() in those cases, as I mentioned in the end of my original comment.

The Clone impl is necessary because you cannot return references from extractors. No way around that I'm afraid.

You can always wrap your state in an Arc and use Router::with_state. That leads to an double Arc internally in the Router but that probably isn't so bad all things considered:

let state_arc = Arc::new(state);
let router: Router<_> = Router::with_state(state_arc).route("/", get(handler));
async fn handler(State(state): State<Arc<UnclonableExample>>) {}

[denschub]

You can always wrap your state in an Arc and use Router::with_state. That leads to an double Arc internally in the Router but that probably isn't so bad all things considered

Yeah, that works!

And thinking more about it... realistically, one would probably put Arcs inside the State if that's needed and make the State Clone, instead of throwing the entire State into an Arc and passing that around.

I'll close this. Thanks for your time! ❤️

[davidpdrsn]

I'll keep this open because I think its worth documenting.

[chrisglass]

This remains confusing to me.

I'm moving my app to 0.6.0-rc.1 since I think I would benefit from using the State extractor, but this has been a little difficult to get right (my app works fine with Extensions in 0.5.0 though).

Here's a minimal example of my problem: I can't seem to figure out how to make the compiler happy with the following (should be easy to build stand-alone, depends on e.g. uuid and axum 0.6.0-rc.1 only I think):

use axum::Router;
use serde_derive::Deserialize;

use axum::{
    extract::{Path, State},
    response::{Html, IntoResponse, Response},
    routing::get,
    Form,
};
use std::net::SocketAddr;
use uuid::Uuid;

#[derive(Default)]
struct AppState {
    // More stuff here but we have just a String (clonable)
    important: String,
}

#[tokio::main]
async fn main() {
    // The State for our application
    let state: AppState = AppState::default();

    // build our application with a route
    let app = Router::with_state(state)
        .route("/", get(get_index))
        .route("/entity/:entity_id", get(get_entity).post(post_entity));

    let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
    axum::Server::bind(&addr)
        .serve(app.into_make_service())
        .await
        .unwrap();
}

#[derive(Deserialize)]
struct PlayForm {
    play: String,
}

async fn get_entity(Path(entity_id): Path<Uuid>, State(state): State<AppState>) -> Response {
    let entity_id: Uuid = entity_id; // useless obv, it's just to do "something" with entity_id
    let the_state: String = state.important; // Same, this is useless, just accessing the state
    Html("<html><body><h1>Whatever</h1></body></html>").into_response()
}

async fn post_entity(
    Path(entity_id): Path<Uuid>,
    Form(form): Form<PlayForm>,
    State(state): State<AppState>,
) -> Response {
    let entity_id: Uuid = entity_id;
    let the_state: String = state.important;
    let the_play_string: String = form.play;
    Html("<html><body><h1>Whatever</h1></body></html>").into_response()
}

async fn get_index() -> Response {
    Html("<html><body><h1>Whatever</h1></body></html>").into_response()
}

The resulting compiler seems to be the famous "handlers are of the wrong type" error, but I can't figure out what exactly is wrong. I tried wrapping my state in an Arc, Extracting an Arc, double Arcs as above... but I'm really stuck :)

What am I doing wrong?

[davidpdrsn]

@chrisglass try https://docs.rs/axum/latest/axum/attr.debug_handler.html but AppState must impl Clone, same as for extensions.

Edit: and Form must be the last extractor. See https://docs.rs/axum/0.6.0-rc.1/axum/extract/index.html#the-order-of-extractors

[jplatte]

To be clear, the whole state just needs to be clonable if you want to extract it all at once, right? If you only ever extract sub-states, you just need the right FromRef impls and not necessarily any Clone impls?

[chrisglass]

Ohhh I see, yes, that makes sense. My state didn't derive Clone.

There's still something fishy with my example relating to the form extractor (state is fixed though!), but that's probably for another issue (or maybe I'll just figure it out myself :) )

Edit: And Form must be the last extractor. OK! Thanks a lot for your help. I'll take a stab at sending some doc touch ups then, to help the next readers :)

[davidpdrsn]

To be clear, the whole state just needs to be clonable if you want to extract it all at once, right? If you only ever extract sub-states, you just need the right FromRef impls and not necessarily any Clone impls?

Yes