Better hints for mismatching routes (i.e. due to format=) #1202
Comments
|
We log "Outcome: Forward" before trying the next route if a request guard returns |
jebrosen
added
accepted
jebrosen
changed the title
|
Another really easy class of error to detect would be to warn about unused query parameters passed to the url. That could be a sign that we've mistyped an Option parameter... |
This is an excellent idea! I'd absolutely love a PR in this direction. I'm happy to mentor as well.
This may prove to be overly verbose. I wonder if this can be a fairing? Perhaps Rocket would need to expose some extra functionality to make such a fairing possible? That'd be fantastic. Better yet, if the above could also be a fairing, that would be great! We could have a |
|
Hello @SergioBenitez, hello @jebrosen and hello rocket community <(^_^)> Please have a look at my prototype to solve this issue. It is built around a Matches enum which can either be a Success or a Fail. If a Route does match a Request, the result of route.matches(request) will be a Success and a Fail otherwise. A Fail describes itself by giving a Reason. A Reason is another Enum. A Reason describes itself by its kind. A Reason can give additional information based on its kind. Take for example Route "/topic/a". Matches::Fail(Reason::NumOfRouteSegsGreater(CmpSegLen)) contains a struct CmpSegLen instead of a tuple. This is an experiment to evaluate the advantages and disadvantages of more elaborate code in this context. It would probably be possible to not add any additional information and recalculate it later, if so desired. Though that approach comes also with its own disadvantages. I found these advantages and disadvantages for Matches:Fail(Reason::Kind(Info)) implementations:
I prefer the Info to be a struct. The ease of use and self explanatory style of code outweighs the disadvantages in my opinion. I tried to make the prototype expressive in terms of a proof of concept. For that reason a Route and a Request are very simple structs, just enough to show the gist of it. The Reason enum does not yet cover reasons for a query mismatch or a format mismatch. After reading the code I hope you can imagine roughly how those could be implemented too. I hope it meets your requirements for a concept and if it does not, please let me know. I will try to elaborate the prototype further to relieve your concerns if you desire.
Deriving the "closest match" from a set of Fails should be doable with a proper definition of "closest match" with the Info and Reason given for each Fail. We could even analyse strings to hint a fix in some cases. I would be glad to implement this for rocket. To get started I would need a little help setting up the development environment. In return I would write a small guide to set up the development environment for newbie rocket lib devs like myself. What do you say? May I take this issue with your guidance? A note on performance: A note on the used lifetime markers: Particular sources of rocket I examined before I wrote the prototype:
fn main() {
let router = Router {
routes: vec![
Route { path: vec![] },
Route {
path: vec!["guide"],
},
Route {
path: vec!["guide", "*"],
},
],
};
router.route(&Request {
path: vec!["guide"],
});
router.route(&Request { path: vec![] });
router.route(&Request {
path: vec!["not_found"],
});
router.route(&Request {
path: vec!["guide", "stuff", "a thing"],
});
}
enum Matches<'s> {
Success,
Fail(Reason<'s>),
}
#[derive(Debug)]
enum Reason<'s> {
NumOfRouteSegsGreater(CmpSegLen),
NumOfRouteSegsLesser(CmpSegLen),
SegmentDoesNotMatch(usize, &'s str, &'s str),
// fail reasons for query mismatch
// fail reasons for format mismatch
}
#[derive(Debug)]
struct CmpSegLen {
route_segs_len: usize,
req_segs_len: usize,
}
struct Router {
routes: Vec<Route>,
}
impl Router {
fn route(&self, request: &Request) {
println!("try to route {:?}", request.path);
for route in &self.routes {
match route.matches(request) {
Matches::Success => {
println!("Matched route {:?}", route.path);
println!("routing SUCCEEDED 🚀");
println!();
return;
}
Matches::Fail(reason) => println!(
"route {:?} did not match, because: {:?}",
route.path, reason
),
}
}
println!("routing FAILED 😱");
println!();
}
}
struct Route {
path: Vec<&'static str>,
}
struct Request {
path: Vec<&'static str>,
}
impl Route {
fn matches<'s>(&'s self, request: &'s Request) -> Matches<'s> {
let route_segs = &self.path;
let req_segs = &request.path;
use Matches::*;
if route_segs.len() > req_segs.len() {
return Fail(Reason::NumOfRouteSegsGreater(CmpSegLen {
route_segs_len: route_segs.len(),
req_segs_len: req_segs.len(),
}));
}
for (i, (route_seg, req_seg)) in route_segs.iter().zip(req_segs).enumerate() {
if *route_seg == "*" {
return Success;
} else if route_seg != req_seg {
return Fail(Reason::SegmentDoesNotMatch(i, route_seg, req_seg));
}
}
if route_segs.len() < req_segs.len() {
return Fail(Reason::NumOfRouteSegsLesser(CmpSegLen {
route_segs_len: route_segs.len(),
req_segs_len: req_segs.len(),
}));
}
// !(route_segs.len() < req_segs.len()) && !(route_segs.len() > req_segs.len())
// == (route_segs.len() == req_segs.len())
// !route_segs.iter().zip(req.segs).any(|(route_seg, req_seg)| route_seg != req_seg)
// == route_segs.iter().zip(req.segs).all(|(route_seg, req_seg)| route_seg == req_seg)
// I hope this is somewhat understandable. In code I would elaborate further why this is a Success.
Success
}
} |
|
Hello everyone, I tinkered with with Fairings and have found a way to implement this feature with a Fairing. My current approach would be to mimic the routing process of rocket and logging the process in data structures like the ones in the previous post. The logged data could be analyzed further if no match would be found. There are drawbacks to this approach:
#![feature(proc_macro_hygiene, decl_macro)]
use rocket::fairing::{Fairing, Info, Kind};
use rocket::{get, routes, Data, Request, Rocket, State};
use std::sync::RwLock;
fn main() {
rocket::ignite()
.mount("/", routes!(get_hello, get_index, get_guide))
.attach(RouteHinter {})
.launch();
}
#[get("/hello")]
fn get_hello() -> &'static str {
"Hello World!"
}
#[get("/")]
fn get_index() -> &'static str {
"Welcome Visitor!"
}
#[get("/guide/*")]
fn get_guide() -> &'static str {
"Welcome to the Guide Rustacian!"
}
struct RouteHinter {}
#[derive(Debug)]
struct RouteBox {
routes: RwLock<Vec<rocket::Route>>,
}
impl Fairing for RouteHinter {
fn info(&self) -> Info {
Info {
name: "Route Hinter",
kind: Kind::Attach | Kind::Launch | Kind::Request,
}
}
fn on_attach(&self, rocket: Rocket) -> Result<Rocket, Rocket> {
Ok(rocket.manage(RouteBox {
routes: RwLock::new(Vec::new()),
}))
}
fn on_launch(&self, rocket: &Rocket) {
let route_box = rocket
.state::<RouteBox>()
.expect("RouteBox initialized in on_attach");
let mut hint_routes = route_box.routes.write().unwrap();
for route in rocket.routes() {
hint_routes.push(route.clone());
}
}
fn on_request(&self, request: &mut Request, _: &Data) {
let route_box = request
.guard::<State<RouteBox>>()
.expect("RouteBox initialized in on_attach");
let routes = route_box.routes.read().unwrap();
println!(
"can check the following routes against request '{}':",
request.uri()
);
for route in routes.iter() {
println!("- {}", route.uri);
}
}
} |
|
Here is the fork with the progress so far: Here is an example you may run and experiment with: So far the code is unpolished and not documented enough. The print for the format parameter match is only partially implemented yet. Collaboration on this is much appreciated. I am new to contributing software to open source projects. If there is anything I can do better, please let me know. Especially if you prefer a better solution for collaboration than the fork I made, let me know. If you have any questions, please contact me here or over at our matrix channel. |
The feedback and compile time errors in Rocket are excellent - they've saved me lots of time already.
I did loose a chunk of time wondering why the url wasn't matching when I was trying to invoke it and it was because the ContentType was different.
It would be most excellent if rocket in development mode could say the closest matching url was to the console and why it failed to match.
It's already a pleasure to work with, but that would make rocket extremely easy to use.
The text was updated successfully, but these errors were encountered: