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

implement Endpoint trait for Box<dyn Endpoint> #710

Merged
merged 1 commit into from
Sep 28, 2020

Conversation

mendelt
Copy link
Contributor

@mendelt mendelt commented Sep 28, 2020

Right now adding endpoints dynamically is difficult because only types that implement the Endpoint trait directly can be added. When you don't know in advance which endpoint you want to add in a certain situation it can be useful to be able to add boxed endpoints. This implementation makes this possible.

I did not add any tests for this. It seems like a trivial piece of code where the test will probably be more fragile than the code itself.

@yoshuawuyts yoshuawuyts merged commit fd6ba06 into http-rs:main Sep 28, 2020
@hasali19
Copy link

hasali19 commented Oct 1, 2020

I just noticed this while going through the code for an unrelated reason. I'm still a bit new to rust but won't this cause an infinite recursion. For example:

fn endpoint() -> Box<dyn tide::Endpoint<()>> {
    Box::new(|_| async { Ok("") })
}

#[async_std::main]
async fn main() {
    let mut app = tide::Server::new();

    app.at("/").get(endpoint());

    let _: tide::http::Response = app
        .respond(tide::http::Request::new(
            tide::http::Method::Get,
            tide::http::Url::parse("http://example.com/").unwrap(),
        ))
        .await
        .unwrap();
}

This code causes a stack overflow.

@mendelt
Copy link
Contributor Author

mendelt commented Oct 1, 2020

Hmm yeah, good catch! I'll see if I can fix that!
thanks!

@mendelt
Copy link
Contributor Author

mendelt commented Oct 1, 2020

Ok, this was stupid of me. I forgot to add as_ref in the method where the boxed endpoint should call the endpoint inside the box. So it called itself. This should be fixed in this pr #711

Thanks again @hasali19 for noticing this.

@mendelt mendelt deleted the boxed_endpoint branch December 21, 2020 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants