From 26b7b814f4011f653851732439045cfe5ff183dd Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Fri, 15 Jul 2016 21:09:08 -0700 Subject: [PATCH] Progress on errors. Started Todo example. The error function now takes in a "RoutingError" structure. The idea is that the structure includes all of the information necessary for a user to processor the error as they wish. This interface is very incomplete and may change. At a minimum, the error structure should include: 1) The request that failed. 2) Why the request failed. 3) The chain of attempted route matches, if any. 4) Something else? --- examples/errors/src/main.rs | 8 +- examples/extended_validation/Cargo.toml | 8 ++ examples/extended_validation/src/files.rs | 13 +++ examples/extended_validation/src/main.rs | 81 +++++++++++++++++++ .../extended_validation/static/index.html | 8 ++ examples/todo/Cargo.toml | 8 ++ examples/todo/src/main.rs | 39 +++++++++ lib/src/catcher.rs | 29 ++++--- lib/src/error.rs | 28 +++++++ lib/src/lib.rs | 2 +- lib/src/request.rs | 12 ++- lib/src/rocket.rs | 16 ++-- macros/src/error_decorator.rs | 2 +- 13 files changed, 230 insertions(+), 24 deletions(-) create mode 100644 examples/extended_validation/Cargo.toml create mode 100644 examples/extended_validation/src/files.rs create mode 100644 examples/extended_validation/src/main.rs create mode 100644 examples/extended_validation/static/index.html create mode 100644 examples/todo/Cargo.toml create mode 100644 examples/todo/src/main.rs diff --git a/examples/errors/src/main.rs b/examples/errors/src/main.rs index 5c668e61..b143cfcf 100644 --- a/examples/errors/src/main.rs +++ b/examples/errors/src/main.rs @@ -2,7 +2,7 @@ #![plugin(rocket_macros)] extern crate rocket; -use rocket::Rocket; +use rocket::{Rocket, RoutingError}; #[route(GET, path = "/hello//")] fn hello(name: &str, age: i8) -> String { @@ -10,8 +10,10 @@ fn hello(name: &str, age: i8) -> String { } #[error(code = "404")] -fn not_found() -> &'static str { - "

Sorry pal.

Go to '/hello/<name>/<age>' instead.

" +fn not_found(error: RoutingError) -> String { + format!("

Sorry, but '{}' is not a valid path!

+

Try visiting /hello/<name>/<age> instead.

", + error.request.uri) } fn main() { diff --git a/examples/extended_validation/Cargo.toml b/examples/extended_validation/Cargo.toml new file mode 100644 index 00000000..1aafb436 --- /dev/null +++ b/examples/extended_validation/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "extended_validation" +version = "0.0.1" +authors = ["Sergio Benitez "] + +[dependencies] +rocket = { path = "../../lib" } +rocket_macros = { path = "../../macros" } diff --git a/examples/extended_validation/src/files.rs b/examples/extended_validation/src/files.rs new file mode 100644 index 00000000..63c0c3d9 --- /dev/null +++ b/examples/extended_validation/src/files.rs @@ -0,0 +1,13 @@ +use rocket; +use std::fs::File; +use std::io::Error as IOError; + +#[route(GET, path = "/")] +pub fn index() -> File { + File::open("static/index.html").unwrap() +} + +#[route(GET, path = "/")] +pub fn files(file: &str) -> Result { + File::open(format!("static/{}", file)) +} diff --git a/examples/extended_validation/src/main.rs b/examples/extended_validation/src/main.rs new file mode 100644 index 00000000..941cf897 --- /dev/null +++ b/examples/extended_validation/src/main.rs @@ -0,0 +1,81 @@ +#![feature(plugin, custom_derive)] +#![plugin(rocket_macros)] + +extern crate rocket; + +mod files; + +use rocket::Rocket; +use rocket::response::Redirect; +use rocket::form::FromFormValue; + +#[derive(Debug)] +struct StrongPassword<'r>(&'r str); + +#[derive(Debug)] +struct AdultAge(isize); + +#[derive(FromForm)] +struct UserLogin<'r> { + username: &'r str, + password: Result, &'r str>, + age: Result, +} + +impl<'v> FromFormValue<'v> for StrongPassword<'v> { + type Error = &'static str; + + fn parse(v: &'v str) -> Result { + if v.len() < 8 { + Err("Too short!") + } else { + Ok(StrongPassword(v)) + } + } +} + +impl<'v> FromFormValue<'v> for AdultAge { + type Error = &'static str; + + fn parse(v: &'v str) -> Result { + let age = match isize::parse(v) { + Ok(v) => v, + Err(_) => return Err("Age value is not a number.") + }; + + match age > 20 { + true => Ok(AdultAge(age)), + false => Err("Must be at least 21.") + } + } +} + +#[route(POST, path = "/login", form = "")] +fn login(user: UserLogin) -> Result { + if user.age.is_err() { + return Err(String::from(user.age.unwrap_err())); + } + + if user.password.is_err() { + return Err(String::from(user.password.unwrap_err())); + } + + match user.username { + "Sergio" => match user.password.unwrap().0 { + "password" => Ok(Redirect::other("/user/Sergio")), + _ => Err("Wrong password!".to_string()) + }, + _ => Err(format!("Unrecognized user, '{}'.", user.username)) + } +} + +#[route(GET, path = "/user/")] +fn user_page(username: &str) -> String { + format!("This is {}'s page.", username) +} + +fn main() { + let mut rocket = Rocket::new("localhost", 8000); + rocket.mount("/", routes![files::index, files::files, user_page, login]); + rocket.launch(); +} diff --git a/examples/extended_validation/static/index.html b/examples/extended_validation/static/index.html new file mode 100644 index 00000000..66bf2339 --- /dev/null +++ b/examples/extended_validation/static/index.html @@ -0,0 +1,8 @@ +

Login

+ +
+ Username: + Password: + Age: + +
diff --git a/examples/todo/Cargo.toml b/examples/todo/Cargo.toml new file mode 100644 index 00000000..3a20beb7 --- /dev/null +++ b/examples/todo/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "todo" +version = "0.0.1" +authors = ["Sergio Benitez "] + +[dependencies] +rocket = { path = "../../lib" } +rocket_macros = { path = "../../macros" } diff --git a/examples/todo/src/main.rs b/examples/todo/src/main.rs new file mode 100644 index 00000000..c073480c --- /dev/null +++ b/examples/todo/src/main.rs @@ -0,0 +1,39 @@ +#![feature(plugin, custom_derive)] +#![plugin(rocket_macros)] + +extern crate rocket; + +use rocket::Rocket; +use rocket::response::Redirect; + +#[derive(FromForm)] +struct Todo<'r> { + description: &'r str, +} + +#[route(POST, path = "/todo", form = "")] +fn new_todo(todo: Todo) -> Result { + // if todos.add(todo).is_ok() { + // Ok(Redirect::to("/")) + // } else { + // Err("Could not add todo to list.") + // } + + Ok(Redirect::to("/")) +} + +#[route(GET, path = "/todos")] +fn list_todos() -> &'static str { + "List all of the todos here!" +} + +#[route(GET, path = "/")] +fn index() -> Redirect { + Redirect::to("/todos") +} + +fn main() { + let mut rocket = Rocket::new("localhost", 8000); + rocket.mount("/", routes![index, list_todos, new_todo]); + rocket.launch(); +} diff --git a/lib/src/catcher.rs b/lib/src/catcher.rs index 791a6dd4..b02ee829 100644 --- a/lib/src/catcher.rs +++ b/lib/src/catcher.rs @@ -1,4 +1,8 @@ use handler::Handler; +use error::Error; +use response::Response; +use request::Request; +use error::RoutingError; use codegen::StaticCatchInfo; use std::fmt; @@ -7,15 +11,23 @@ use term_painter::Color::*; pub struct Catcher { pub code: u16, - pub handler: Handler, + handler: Handler, is_default: bool } +// TODO: Should `Catcher` be an interface? Should there be an `ErrorHandler` +// that takes in a `RoutingError` and returns a `Response`? What's the right +// interface here? + impl Catcher { pub fn new(code: u16, handler: Handler) -> Catcher { Catcher::new_with_default(code, handler, false) } + pub fn handle<'a>(&'a self, error: RoutingError<'a>) -> Response { + (self.handler)(error.request) + } + fn new_with_default(code: u16, handler: Handler, default: bool) -> Catcher { Catcher { code: code, @@ -47,13 +59,6 @@ pub mod defaults { use super::Catcher; use std::collections::HashMap; - pub fn get() -> HashMap { - let mut map = HashMap::new(); - map.insert(404, Catcher::new_with_default(404, not_found, true)); - map.insert(500, Catcher::new_with_default(500, internal_error, true)); - map - } - pub fn not_found(_request: Request) -> Response { Response::with_status(StatusCode::NotFound, "\ \ @@ -82,5 +87,11 @@ pub mod defaults { Rocket\ ") } -} + pub fn get() -> HashMap { + let mut map = HashMap::new(); + map.insert(404, Catcher::new_with_default(404, not_found, true)); + map.insert(500, Catcher::new_with_default(500, internal_error, true)); + map + } +} diff --git a/lib/src/error.rs b/lib/src/error.rs index 46af91a2..c860f396 100644 --- a/lib/src/error.rs +++ b/lib/src/error.rs @@ -1,3 +1,5 @@ +use request::Request; + #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] pub enum Error { BadMethod, @@ -5,3 +7,29 @@ pub enum Error { NoRoute, NoKey } + +pub struct RoutingError<'r> { + pub error: Error, + pub request: Request<'r>, + pub chain: Option<&'r [&'r str]> +} + +impl<'a> RoutingError<'a> { + pub fn unchained(request: Request<'a>) + -> RoutingError<'a> { + RoutingError { + error: Error::NoRoute, + request: request, + chain: None, + } + } + + pub fn new(error: Error, request: Request<'a>, chain: &'a [&'a str]) + -> RoutingError<'a> { + RoutingError { + error: error, + request: request, + chain: Some(chain) + } + } +} diff --git a/lib/src/lib.rs b/lib/src/lib.rs index 64e61a73..9052e77c 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -26,7 +26,7 @@ pub use codegen::{StaticRouteInfo, StaticCatchInfo}; pub use request::Request; pub use method::Method; pub use response::{Response, Responder}; -pub use error::Error; +pub use error::{Error, RoutingError}; pub use param::FromParam; pub use router::{Router, Route}; pub use catcher::Catcher; diff --git a/lib/src/request.rs b/lib/src/request.rs index 8b9d8143..0915fecc 100644 --- a/lib/src/request.rs +++ b/lib/src/request.rs @@ -1,18 +1,22 @@ use error::Error; use param::FromParam; +use method::Method; pub use hyper::server::Request as HyperRequest; #[derive(Clone)] pub struct Request<'a> { - params: Vec<&'a str>, + params: Option>, + pub method: Method, pub uri: &'a str, pub data: &'a [u8] } impl<'a> Request<'a> { - pub fn new(params: Vec<&'a str>, uri: &'a str, data: &'a [u8]) -> Request<'a> { + pub fn new(method: Method, uri: &'a str, params: Option>, + data: &'a [u8]) -> Request<'a> { Request { + method: method, params: params, uri: uri, data: data @@ -24,10 +28,10 @@ impl<'a> Request<'a> { } pub fn get_param>(&'a self, n: usize) -> Result { - if n >= self.params.len() { + if self.params.is_none() || n >= self.params.as_ref().unwrap().len() { Err(Error::NoKey) } else { - T::from_param(self.params[n]) + T::from_param(self.params.as_ref().unwrap()[n]) } } } diff --git a/lib/src/rocket.rs b/lib/src/rocket.rs index 32628082..f2be3623 100644 --- a/lib/src/rocket.rs +++ b/lib/src/rocket.rs @@ -80,10 +80,11 @@ impl Rocket { // A closure which we call when we know there is no route. let handle_not_found = |response: FreshHyperResponse| { - let request = Request::new(vec![], uri, &buf); - let handler_404 = self.catchers.get(&404).unwrap().handler; println!("{}", Red.paint("\t<= Dispatch failed. Returning 404.")); - handler_404(request).respond(response); + + let request = Request::new(method, uri, None, &buf); + let catcher = self.catchers.get(&404).unwrap(); + catcher.handle(RoutingError::unchained(request)).respond(response); }; // No route found. Handle the not_found error and return. @@ -93,17 +94,20 @@ impl Rocket { } // Okay, we've got a route. Unwrap it, generate a request, and try to - // dispatch. TODO: keep trying lower ranked routes before dispatching a - // not found error. + // dispatch. println!("\t=> {}", Magenta.paint("Dispatching request.")); let route = route.unwrap(); let params = route.get_params(uri); - let request = Request::new(params, uri, &buf); + let request = Request::new(method, uri, Some(params), &buf); let outcome = (route.handler)(request).respond(res); + // TODO: keep trying lower ranked routes before dispatching a not found + // error. println!("\t=> {} {}", White.paint("Outcome:"), outcome); outcome.map_forward(|res| { println!("{}", Red.paint("\t=> No further matching routes.")); + // TODO: Have some way to know why this was failed forward. Use that + // instead of always using an unchained error. handle_not_found(res); }); } diff --git a/macros/src/error_decorator.rs b/macros/src/error_decorator.rs index ea82d186..b1de4e3e 100644 --- a/macros/src/error_decorator.rs +++ b/macros/src/error_decorator.rs @@ -62,7 +62,7 @@ pub fn error_decorator(ecx: &mut ExtCtxt, sp: Span, meta_item: &MetaItem, fn $catch_fn_name<'rocket>(_req: rocket::Request<'rocket>) -> rocket::Response<'rocket> { // TODO: Figure out what type signature of catcher should be. - let result = $fn_name(); + let result = $fn_name(RoutingError::unchained(_req)); rocket::Response::with_raw_status($catch_code, result) } ).unwrap();