Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add suggestion for incorrect data extractor type #2254

Closed
pickfire opened this issue Jun 6, 2021 · 4 comments
Closed

Add suggestion for incorrect data extractor type #2254

pickfire opened this issue Jun 6, 2021 · 4 comments
Assignees
Labels
A-web project: actix-web C-improvement Category: an improvement to existing functionality
Milestone

Comments

@pickfire
Copy link
Contributor

pickfire commented Jun 6, 2021

Expected Behavior

[2021-06-06T17:49:53Z DEBUG actix_web::data] Failed to construct App-level Data extractor. Request path: "/start" (type: std::sync::mutex::Mutex<futures_channel::mpsc::Sender<battlesnake::Board>>) ... <suggestion>

Suggestion could be like missed wrapping T within Data (which happened to me, took me half an hour to figure this out). It's in the example but I didn't look closely at this part since I thought it should work since it compiled. The logging message is correct but I don't know why it failed, the type is correct but just without the Data so I didn't know that is an issue.

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    let counter = web::Data::new(AppStateWithCounter {
        counter: Mutex::new(0),
    });

    HttpServer::new(move || {
        // move counter into the closure
        App::new()
            // Note: using app_data instead of data
            .app_data(counter.clone()) // <- register the created data
            .route("/", web::get().to(index))
    })
    .bind("127.0.0.1:8080")?
    .run()
    .await
}

Current Behavior

[2021-06-06T17:49:53Z DEBUG actix_web::data] Failed to construct App-level Data extractor. Request path: "/start" (type: std::sync::mutex::Mutex<futures_channel::mpsc::Sender<battlesnake::Board>>)

Possible Solution

Add a suggestion on what is the expected type and the code change required. Ideally this should be done compile time but that's another issue.

Context

This will be very helpful for newcomers and even developers who made some mistakes and ease them.

Your Environment

  • Rust Version (I.e, output of rustc -V): rustc 1.52.1 (9bc8c42bb 2021-05-09)
  • Actix Web Version: 3.3.2
@pickfire
Copy link
Contributor Author

Rocket seemed to do something called sentinel to be able to error when application starts if a state is not managed.

@robjtede
Copy link
Member

We improved the error message in v4 beta. See https://github.com/actix/actix-web/blob/master/src/data.rs#L128.

I also intend to deprecate App::data to make it more obvious what type is being inserted.

@robjtede robjtede self-assigned this Jun 16, 2021
@robjtede robjtede added C-improvement Category: an improvement to existing functionality A-web project: actix-web labels Jun 16, 2021
@robjtede robjtede added this to the actix-web v4 milestone Jun 16, 2021
@pickfire
Copy link
Contributor Author

But yeah, in rocket if the type is not correct it won't even start serving. Although it is still not as good as compile-time type checking, it starts checking when the application starts which I think is good enough. Actix-web at least still need to call the endpoint to know if it works.

@robjtede
Copy link
Member

The following code panics at startup in rocket because of sentinels. We have always provided Option<T> extractors as a dependency injection strategy.

#[get("/")]
fn hello(state: Option<&State<MyConfig>>) -> &'static str {
    "Hello, world!"
}

#[launch]
fn rocket() -> _ {
    rocket::build()
        // .manage(MyConfig { user_val: 42 })
        .mount("/", routes![hello])
}

That's to say, we won't be pursuing any similar functionality.

A fully type-safe app state system is more interesting and we have some longer term thoughts about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-web project: actix-web C-improvement Category: an improvement to existing functionality
Projects
None yet
Development

No branches or pull requests

2 participants