Route Tracing

[cataggar]

I want to be be able to test that routes resolve correctly, given the HTTP method and URI.

#[cfg(test)]
mod test {
    use super::*;
    use rocket::Router;

    #[test]
    fn test_route_name() -> Result<(), Error> {
        let mut router = Router::new();
        for route in api_routes() {
            router.add_route(route);
        }

        assert_eq!("Hello".to_owned(), route_name(&router, "GET", "/")?);
        // a lot more asserts
        Ok(())
    }
}

// Get the route name from the HTTP method + URI combination.
fn route_name(router: &Router, method: &str, uri: &str) -> Result<String, Error> {
    let client = rocket::local::blocking::Client::debug_with(Vec::new()).map_err(Error::CreateLocalClient)?;
    let request_method = Method::from_str(method).map_err(|_| Error::InvalidHttpMethod(method.to_owned()))?;
    let request_uri = rocket::http::uri::Origin::parse(uri).map_err(|_| Error::ParseUri(uri.to_owned()))?;
    let request = client.req(request_method, request_uri);
    let mut routes = router.route(&request);
    let route = routes.next().ok_or_else(|| Error::RouteNotFound {
        method: method.to_owned(),
        uri: uri.to_owned(),
    })?;
    let name = route.name.as_ref().ok_or_else(|| Error::UnnamedRoute {
        method: method.to_owned(),
        uri: uri.to_owned(),
    })?;
    Ok(name.to_owned().to_string())
}

#[non_exhaustive]
#[derive(Debug, thiserror::Error)]
pub enum Error {
    #[error("unable to create rocket client: {0}")]
    CreateLocalClient(rocket::Error),
    #[error("unable to parse uri: {0}")]
    ParseUri(String),
    #[error("invalid http method: {0}")]
    InvalidHttpMethod(String),
    #[error("route not found for {method} {uri}")]
    RouteNotFound { method: String, uri: String },
    #[error("unnamed route for {method} {uri}")]
    UnnamedRoute { method: String, uri: String },
}

Existing Functionality

Router

Suggested Changes

The above code works if Router is made public #1873 and the route name is settable #1872.

Alternatives Considered

Additional Context

[SergioBenitez]

I want to be be able to test that routes resolve correctly, given the HTTP method and URI.

Why do you want to test that a URI + method resolves to a specific route? I'm not sure your code does what you might want it to. Importantly, it completely ignores guards and format, so you're bound to test the wrong thing. For example, the following will return "foo" for /foo/1024 when the route that will actually match is bar:

#[get("/foo/<bar>")]
fn foo(bar: u8) { .. }

#[get("/foo/<bar>", rank = 2)]
fn bar(bar: usize) { .. }

Your approach is also quite brittle as it checks that the route's name is a given string, but you could have two routes with the same name with no way to disambiguate as the code stands, and changing the route's name would break your tests. What's more, assuming no rank-resolved conflicts, you're effectively just checking that you put the right attribute on the right function. Why not issue a request (in testing) and see if you get the response you expect? This has none of the downsides and is actually checking something meaningful.

[liaden]

I think the desire here would be to test things similar to the routing
tests in
rails.

It could be useful to test the guards, formats, fairings, and routing
precedence all together, but without the actual execution of the backend
given a dev can introduce a bug when weaving these together.

Another reason I would like routing tests is because routing logic is
often scattered throughout application code. Having the tests in one
location is a form of documentation.

One nice thing from the routing specs is testing that a given request is
not routable without being coupled to the response details.

Some reasons to not need routing tests:

• Type safety guarantees mitigate a class of bugs around
  authorization/authentication.
• The routes.rb in Rails is a lot more complicated than Rocket's routing
  logic (allows for mounting a Rails Engine from a gem, delegating
  a request to anything Rack compatible, adding constaints/formats/etc to
  potentially multiple endpoints, defining nested routes, and definining
  namespaces).
• Dev could setup a mocking layer inside their functions to allow this
  testing.

[the10thWiz]

Looking into this, it actually seems fairly trivial to add support for testing which route handled a given request. The actual mechanics are somewhat complicated, but the LocalResponse type actually already contains a reference to route object. To make this easy to use, we should provide a macro, and I imagine the test code should then look something like:

use rocket::local::asynchronous::Client;

#[rocket::async_test]
async fn test_routing() {
    let client = Client::untracked(super::rocket()).await.expect("Failed to launch rocket");
    let res = client.get(uri!("/")).dispatch().await;
    assert_routed_with!(res, super::index);
}

The macro would then be expanded to something like:

{
    let route = res.route().expect("Request was not routed");// Route run for local client
    let expected_route = super::index {}.into_info();// Route info from user
    assert_eq!(route.name, expected_route.name);
    assert_eq!(route.method, expected_route.method);
    // etc.
}

However, there are a few issues. First, we acutally can't tell certain routes apart, since we only know the name of the function, which method, and the uri defined in the route macro. To actually handle this, we likely need to add some support for the route to hold onto which route it was called from - I think it may be trivial to implement this using an Option<Box<dyn Any + 'static>> in StaticInfo and Route. This could likely also be marked as #[cfg(test)] to reduce overhead when not running tests, but I don't think it's too major to begin with. This would then take advantage of Rust's Any trait to guarntee that it's looking at the right route.

I'm not sure if this should be implemented, but this does also take care of a few issues that were expressed. First, this requires running the full routing machinary, including request guards, and the actual route itself. This also makes it easy to check the response from the request (since the full response was generated), or simply add it as an additional check to an existing test.

There are still some issues. First, if a request guard fails, and the request is forwarded to a catcher, this will still show the route as being executed, since the route was the last one attempted. This also only shows the last route attempted (which will be the one that responded, if one did), so this may not work for testing routes with authentication, if there are backup routes (i.e. a redirect to the login page).

Another interesting option for testing whether a given route gets called would be to create a partial Rocket instance, where rather than mounting everything, you just mount the route in question and check whether it gets routed at all. These tests should also be run against your 'full' Rocket instance, to verify that another route doesn't take precedence. For testing, it may be useful to provide a clear_routes method (with #[cfg(test)]) on Rocket, which would unmount every route, while leaving the fairings, state, etc intact. This makes creating the partial Rocket easier, and always exactly the same set of fairings and state configured. This neatly solves the two issues with the previous tactic - since you know the handler that got called (since it was the only one mounted), and you will get a default 404 if it doesn't get called. This type of test might look something like:

use rocket::local::asynchronous::Client;

#[rocket::async_test]
async fn test_routing() {
    let full_client = Client::untracked(super::rocket()).await.expect("Failed to launch rocket");
    let full_res = full_client.get(uri!("/")).dispatch().await;
    assert_eq!(full_res.status(), Status::Ok, "No route responded");

    let index_client = Client::untracked(super::rocket().clear_routes().mount("/", routes![super::index])).await.expect("Failed to launch rocket");
    let index_res = index_client.get(uri!("/")).dispatch().await;
    assert_eq!(index_res.status(), Status::Ok, "Index route did not respond");

    assert_eq!(index_res.into_string(), full_res.into_string(), "Index route did not return the same reponse with the full rocket instance");
}

For testing authenticated routes, a partial client could be used to see if the authenticated route matches an unauthenticated request. Unlike a full client, if the route doesn't match it will return a 404, making the test potentially a lot shorter. Right now, I would recommend testing by placing some kind of key in the authenticated page (i.e. maybe the user's email or some other information that the page should only return to an authenticated user), and checking for it's presence in the response. This also catches potential information leaks in the fallback methods, since they shouldn't return this sensitive information.

The only real issue I see with this is that it is non-trivial to check whether the route returned a 404 or the default responder returned a 404. This could be trivially solved by mounting a default route that responds with a different status by default, but it's not an obvious solution. A better alternative may be to create a TestCatchingRoute with this behavior, but it also provides some kind of method to check whether it was called.

[SergioBenitez]

I'll reopen this as I'm quite intrigued by the ideas proposed by @the10thWiz above. Generalizing from there, I think it would be phenomenal if we could ask LocalResponse, unambiguously, to tell us what generated the response, whether it be a user's route, something from a library, a catcher, or whatever. Even better would be to know the chain of calls to get to the response, including forwards and so on. That would subsume the request in this issue and likely make for even better logging. I would accept such a proposal and implementation.

P.S: As an aside, I've long envisioned (at the suggestion of a colleague) a UI tool for Rocket that visually shows you the chain from request to response through routes, catchers, fairings, etc. The suggestion I've made above would enable such a tool.

[tessus]

P.S: As an aside, I've long envisioned (at the suggestion of a colleague) a UI tool for Rocket that visually shows you the chain from request to response through routes, catchers, fairings, etc.

Just an idea... Have you thought about using OpenTelemetry to instrument your code? This would give you all the info you need.

The advantage is it's not limited to routes but you can add trace info to all parts of the code (if wanted).

[cataggar]

This may be fixed by #2167, where routes get a TypeId when #[cfg(test)].

[the10thWiz]

@cataggar #2167 is specifically intended to address this issue. It also implements the route tracing noted above.

[SergioBenitez]

Note that with #1202 (comment), our trace messages now include all of the information necessary to understand the complete flow of a request. It doesn't give you a nice AP to ask questions, but all of the information is there.