Skip to content

Commit

Permalink
wip: testbench, locations, display hack, either
Browse files Browse the repository at this point in the history
display hack: when guard errors occur, log the error message with
`Display` if an impl exists, falling back to `Debug` otherwise

item locations: log the file and line for catchers and routes

either: impls `FromParam` for `Either`. changes the implementation of
`FromParam` for all `T: FromStr` to return the actual error when the
conversion fails instead of the previous `&str` for the value that
failed to parse. This results in better log error messages on guard
failure. To regain the ability to recover the value that failed to
parse, `FromParam` was implemented for `Either<A, B>`; `Either<T, &str>`
can be used as the previous `Result<T, &str>` could, with the benefit
that it now works for all `T`. `FromUriParam` and `UriDisplay` were also
implemented for `Either<A, B>`.

testbench: fix the testbench for new logging and test that display hack
works as intended for both `T: Display` and `T: !Display + Debug`.
  • Loading branch information
SergioBenitez committed May 20, 2024
1 parent 07cb899 commit 6f1c2a7
Show file tree
Hide file tree
Showing 35 changed files with 949 additions and 535 deletions.
2 changes: 1 addition & 1 deletion contrib/db_pools/lib/src/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ pub trait Pool: Sized + Send + Sync + 'static {
mod deadpool_postgres {
use deadpool::{managed::{Manager, Pool, PoolError, Object, BuildError}, Runtime};
use super::{Duration, Error, Config, Figment};
use rocket::Either;
use rocket::either::Either;

pub trait DeadManager: Manager + Sized + Send + Sync + 'static {
fn new(config: &Config) -> Result<Self, Self::Error>;
Expand Down
3 changes: 2 additions & 1 deletion core/codegen/src/attribute/catch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,10 @@ pub fn _catch(
}

#_catcher::StaticInfo {
name: stringify!(#user_catcher_fn_name),
name: ::core::stringify!(#user_catcher_fn_name),
code: #status_code,
handler: monomorphized_function,
location: (::core::file!(), ::core::line!(), ::core::column!()),
}
}

Expand Down
37 changes: 27 additions & 10 deletions core/codegen/src/attribute/route/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,18 +120,25 @@ fn query_decls(route: &Route) -> Option<TokenStream> {
fn request_guard_decl(guard: &Guard) -> TokenStream {
let (ident, ty) = (guard.fn_ident.rocketized(), &guard.ty);
define_spanned_export!(ty.span() =>
__req, __data, _request, FromRequest, Outcome
__req, __data, _request, display_hack, FromRequest, Outcome
);

quote_spanned! { ty.span() =>
let #ident: #ty = match <#ty as #FromRequest>::from_request(#__req).await {
#Outcome::Success(__v) => __v,
#Outcome::Forward(__e) => {
::rocket::info!(type_name = stringify!(#ty), "guard forwarding");
::rocket::info!(name: "forward", parameter = stringify!(#ident),
type_name = stringify!(#ty), status = __e.code,
"request guard forwarding");

return #Outcome::Forward((#__data, __e));
},
#[allow(unreachable_code)]
#Outcome::Error((__c, __e)) => {
::rocket::info!(type_name = stringify!(#ty), "guard failed: {__e:?}");
::rocket::info!(name: "failure", parameter = stringify!(#ident),
type_name = stringify!(#ty), reason = %#display_hack!(__e),
"request guard failed");

return #Outcome::Error(__c);
}
};
Expand All @@ -142,14 +149,14 @@ fn param_guard_decl(guard: &Guard) -> TokenStream {
let (i, name, ty) = (guard.index, &guard.name, &guard.ty);
define_spanned_export!(ty.span() =>
__req, __data, _None, _Some, _Ok, _Err,
Outcome, FromSegments, FromParam, Status
Outcome, FromSegments, FromParam, Status, display_hack
);

// Returned when a dynamic parameter fails to parse.
let parse_error = quote!({
::rocket::info!(name: "forward",
reason = %__error, parameter = #name, "type" = stringify!(#ty),
"parameter forwarding");
::rocket::info!(name: "forward", parameter = #name,
type_name = stringify!(#ty), reason = %#display_hack!(__error),
"path guard forwarding");

#Outcome::Forward((#__data, #Status::UnprocessableEntity))
});
Expand All @@ -161,6 +168,7 @@ fn param_guard_decl(guard: &Guard) -> TokenStream {
match #__req.routed_segment(#i) {
#_Some(__s) => match <#ty as #FromParam>::from_param(__s) {
#_Ok(__v) => __v,
#[allow(unreachable_code)]
#_Err(__error) => return #parse_error,
},
#_None => {
Expand All @@ -176,6 +184,7 @@ fn param_guard_decl(guard: &Guard) -> TokenStream {
true => quote_spanned! { ty.span() =>
match <#ty as #FromSegments>::from_segments(#__req.routed_segments(#i..)) {
#_Ok(__v) => __v,
#[allow(unreachable_code)]
#_Err(__error) => return #parse_error,
}
},
Expand All @@ -187,17 +196,24 @@ fn param_guard_decl(guard: &Guard) -> TokenStream {

fn data_guard_decl(guard: &Guard) -> TokenStream {
let (ident, ty) = (guard.fn_ident.rocketized(), &guard.ty);
define_spanned_export!(ty.span() => __req, __data, FromData, Outcome);
define_spanned_export!(ty.span() => __req, __data, display_hack, FromData, Outcome);

quote_spanned! { ty.span() =>
let #ident: #ty = match <#ty as #FromData>::from_data(#__req, #__data).await {
#Outcome::Success(__d) => __d,
#Outcome::Forward((__d, __e)) => {
::rocket::info!(type_name = stringify!(#ty), "data guard forwarding");
::rocket::info!(name: "forward", parameter = stringify!(#ident),
type_name = stringify!(#ty), status = __e.code,
"data guard forwarding");

return #Outcome::Forward((__d, __e));
}
#[allow(unreachable_code)]
#Outcome::Error((__c, __e)) => {
::rocket::info!(type_name = stringify!(#ty), "data guard failed: {__e:?}");
::rocket::info!(name: "failure", parameter = stringify!(#ident),
type_name = stringify!(#ty), reason = %#display_hack!(__e),
"data guard failed");

return #Outcome::Error(__c);
}
};
Expand Down Expand Up @@ -383,6 +399,7 @@ fn codegen_route(route: Route) -> Result<TokenStream> {
format: #format,
rank: #rank,
sentinels: #sentinels,
location: (::core::file!(), ::core::line!(), ::core::column!()),
}
}

Expand Down
1 change: 1 addition & 0 deletions core/codegen/src/exports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ define_exported_paths! {
_Vec => ::std::vec::Vec,
_Cow => ::std::borrow::Cow,
_ExitCode => ::std::process::ExitCode,
display_hack => ::rocket::error::display_hack,
BorrowMut => ::std::borrow::BorrowMut,
Outcome => ::rocket::outcome::Outcome,
FromForm => ::rocket::form::FromForm,
Expand Down
2 changes: 1 addition & 1 deletion core/codegen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1328,7 +1328,7 @@ pub fn catchers(input: TokenStream) -> TokenStream {
/// assert_eq!(bob2.to_string(), "/person/Bob%20Smith");
///
/// #[get("/person/<age>")]
/// fn ok(age: Result<u8, &str>) { }
/// fn ok(age: Result<u8, std::num::ParseIntError>) { }
///
/// let kid1 = uri!(ok(age = 10));
/// let kid2 = uri!(ok(12));
Expand Down
8 changes: 4 additions & 4 deletions core/codegen/tests/ui-fail-stable/catch_type_errors.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ error[E0277]: the trait bound `usize: Responder<'_, '_>` is not satisfied
<Box<str> as Responder<'r, 'static>>
<Box<[u8]> as Responder<'r, 'static>>
<Box<T> as Responder<'r, 'o>>
<rocket::Either<T, E> as Responder<'r, 'o>>
<rocket::either::Either<T, E> as Responder<'r, 'o>>
<Cow<'o, R> as Responder<'r, 'o>>
<rocket::tokio::fs::File as Responder<'r, 'static>>
<EventStream<S> as Responder<'r, 'r>>
Expand All @@ -29,7 +29,7 @@ error[E0277]: the trait bound `bool: Responder<'_, '_>` is not satisfied
<Box<str> as Responder<'r, 'static>>
<Box<[u8]> as Responder<'r, 'static>>
<Box<T> as Responder<'r, 'o>>
<rocket::Either<T, E> as Responder<'r, 'o>>
<rocket::either::Either<T, E> as Responder<'r, 'o>>
<Cow<'o, R> as Responder<'r, 'o>>
<rocket::tokio::fs::File as Responder<'r, 'static>>
<EventStream<S> as Responder<'r, 'r>>
Expand Down Expand Up @@ -62,7 +62,7 @@ error[E0277]: the trait bound `usize: Responder<'_, '_>` is not satisfied
<Box<str> as Responder<'r, 'static>>
<Box<[u8]> as Responder<'r, 'static>>
<Box<T> as Responder<'r, 'o>>
<rocket::Either<T, E> as Responder<'r, 'o>>
<rocket::either::Either<T, E> as Responder<'r, 'o>>
<Cow<'o, R> as Responder<'r, 'o>>
<rocket::tokio::fs::File as Responder<'r, 'static>>
<EventStream<S> as Responder<'r, 'r>>
Expand All @@ -81,7 +81,7 @@ error[E0277]: the trait bound `usize: Responder<'_, '_>` is not satisfied
<Box<str> as Responder<'r, 'static>>
<Box<[u8]> as Responder<'r, 'static>>
<Box<T> as Responder<'r, 'o>>
<rocket::Either<T, E> as Responder<'r, 'o>>
<rocket::either::Either<T, E> as Responder<'r, 'o>>
<Cow<'o, R> as Responder<'r, 'o>>
<rocket::tokio::fs::File as Responder<'r, 'static>>
<EventStream<S> as Responder<'r, 'r>>
Expand Down
6 changes: 3 additions & 3 deletions core/codegen/tests/ui-fail-stable/responder-types.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ error[E0277]: the trait bound `u8: Responder<'_, '_>` is not satisfied
<Box<str> as Responder<'r, 'static>>
<Box<[u8]> as Responder<'r, 'static>>
<Box<T> as Responder<'r, 'o>>
<rocket::Either<T, E> as Responder<'r, 'o>>
<rocket::either::Either<T, E> as Responder<'r, 'o>>
and $N others

error[E0277]: the trait bound `Header<'_>: From<u8>` is not satisfied
Expand Down Expand Up @@ -52,7 +52,7 @@ error[E0277]: the trait bound `u8: Responder<'_, '_>` is not satisfied
<Box<str> as Responder<'r, 'static>>
<Box<[u8]> as Responder<'r, 'static>>
<Box<T> as Responder<'r, 'o>>
<rocket::Either<T, E> as Responder<'r, 'o>>
<rocket::either::Either<T, E> as Responder<'r, 'o>>
and $N others

error[E0277]: the trait bound `Header<'_>: From<u8>` is not satisfied
Expand Down Expand Up @@ -117,7 +117,7 @@ error[E0277]: the trait bound `usize: Responder<'_, '_>` is not satisfied
<Box<str> as Responder<'r, 'static>>
<Box<[u8]> as Responder<'r, 'static>>
<Box<T> as Responder<'r, 'o>>
<rocket::Either<T, E> as Responder<'r, 'o>>
<rocket::either::Either<T, E> as Responder<'r, 'o>>
and $N others
note: required by a bound in `route::handler::<impl Outcome<Response<'o>, Status, (rocket::Data<'o>, Status)>>::from`
--> $WORKSPACE/core/lib/src/route/handler.rs
Expand Down
23 changes: 20 additions & 3 deletions core/http/src/uri/fmt/from_uri_param.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::collections::{BTreeMap, HashMap};
use std::path::{Path, PathBuf};
use std::collections::{BTreeMap, HashMap};

use either::Either;

use crate::uri::fmt::UriDisplay;
use crate::uri::fmt::{self, Part};
Expand Down Expand Up @@ -61,7 +63,7 @@ use crate::uri::fmt::{self, Part};
///
/// * `String`, `i8`, `i16`, `i32`, `i64`, `i128`, `isize`, `u8`, `u16`,
/// `u32`, `u64`, `u128`, `usize`, `f32`, `f64`, `bool`, `IpAddr`,
/// `Ipv4Addr`, `Ipv6Addr`, `&str`, `Cow<str>`
/// `Ipv4Addr`, `Ipv6Addr`, `&str`, `Cow<str>`, `Either<A, B>`
///
/// The following types have _identity_ implementations _only in [`Path`]_:
///
Expand Down Expand Up @@ -375,7 +377,9 @@ impl<A, T: FromUriParam<fmt::Path, A>> FromUriParam<fmt::Path, A> for Option<T>
}

/// A no cost conversion allowing `T` to be used in place of an `Result<T, E>`.
impl<A, E, T: FromUriParam<fmt::Path, A>> FromUriParam<fmt::Path, A> for Result<T, E> {
impl<A, E, T> FromUriParam<fmt::Path, A> for Result<T, E>
where T: FromUriParam<fmt::Path, A>
{
type Target = T::Target;

#[inline(always)]
Expand All @@ -384,6 +388,19 @@ impl<A, E, T: FromUriParam<fmt::Path, A>> FromUriParam<fmt::Path, A> for Result<
}
}

impl<P: Part, A, B, T, U> FromUriParam<P, Either<A, B>> for Either<T, U>
where T: FromUriParam<P, A>, U: FromUriParam<P, B>
{
type Target = Either<T::Target, U::Target>;

fn from_uri_param(param: Either<A, B>) -> Self::Target {
match param {
Either::Left(a) => Either::Left(T::from_uri_param(a)),
Either::Right(b) => Either::Right(U::from_uri_param(b)),
}
}
}

impl<A, T: FromUriParam<fmt::Query, A>> FromUriParam<fmt::Query, Option<A>> for Option<T> {
type Target = Option<T::Target>;

Expand Down
12 changes: 12 additions & 0 deletions core/http/src/uri/fmt/uri_display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::collections::{BTreeMap, HashMap};
use std::{fmt, path};
use std::borrow::Cow;

use either::Either;
use time::{macros::format_description, format_description::FormatItem};

use crate::RawStr;
Expand Down Expand Up @@ -421,6 +422,17 @@ impl<P: Part, T: UriDisplay<P> + ?Sized> UriDisplay<P> for &T {
}
}

/// Defers to `T` or `U` in `Either<T, U>`.
impl<P: Part, T: UriDisplay<P>, U: UriDisplay<P>> UriDisplay<P> for Either<T, U> {
#[inline(always)]
fn fmt(&self, f: &mut Formatter<'_, P>) -> fmt::Result {
match self {
Either::Left(t) => UriDisplay::fmt(t, f),
Either::Right(u) => UriDisplay::fmt(u, f),
}
}
}

/// Defers to the `UriDisplay<P>` implementation for `T`.
impl<P: Part, T: UriDisplay<P> + ?Sized> UriDisplay<P> for &mut T {
#[inline(always)]
Expand Down
9 changes: 8 additions & 1 deletion core/lib/src/catcher/catcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ pub struct Catcher {
///
/// This is -(number of nonempty segments in base).
pub(crate) rank: isize,

/// The catcher's file, line, and column location.
pub(crate) location: Option<(&'static str, u32, u32)>,
}

// The rank is computed as -(number of nonempty segments in base) => catchers
Expand Down Expand Up @@ -185,7 +188,8 @@ impl Catcher {
base: uri::Origin::root().clone(),
handler: Box::new(handler),
rank: rank(uri::Origin::root().path()),
code
code,
location: None,
}
}

Expand Down Expand Up @@ -328,6 +332,8 @@ pub struct StaticInfo {
pub code: Option<u16>,
/// The catcher's handler, i.e, the annotated function.
pub handler: for<'r> fn(Status, &'r Request<'_>) -> BoxFuture<'r>,
/// The file, line, and column where the catcher was defined.
pub location: (&'static str, u32, u32),
}

#[doc(hidden)]
Expand All @@ -336,6 +342,7 @@ impl From<StaticInfo> for Catcher {
fn from(info: StaticInfo) -> Catcher {
let mut catcher = Catcher::new(info.code, info.handler);
catcher.name = Some(info.name.into());
catcher.location = Some(info.location);
catcher
}
}
Expand Down
56 changes: 56 additions & 0 deletions core/lib/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,59 @@ pub(crate) fn log_server_error(error: &(dyn StdError + 'static)) {
});
}
}

#[doc(hidden)]
#[macro_export]
macro_rules! display_hack {
($v:expr) => ({
#[allow(unused_imports)]
use $crate::error::display_hack_impl::{DisplayHack, DefaultDisplay as _};

#[allow(unreachable_code)]
DisplayHack($v).display()
})
}

#[doc(hidden)]
pub use display_hack as display_hack;

#[doc(hidden)]
pub mod display_hack_impl {
use super::*;
use crate::util::Formatter;

/// The *magic*.
///
/// This type implements a `display()` method for all types that are either
/// `fmt::Display` _or_ `fmt::Debug`, using the former when available. It
/// does so by using a "specialization" hack: it has a blanket
/// DefaultDisplay trait impl for all types that are `fmt::Debug` and a
/// "specialized" inherent impl for all types that are `fmt::Display`.
///
/// As long as `T: Display`, the "specialized" impl is what Rust will
/// resolve `DisplayHack(v).display()` to when `T: fmt::Display` as it is an
/// inherent impl. Otherwise, Rust will fall back to the blanket impl.
pub struct DisplayHack<T: ?Sized>(pub T);

pub trait DefaultDisplay {
fn display(&self) -> impl fmt::Display;
}

/// Blanket implementation for `T: Debug`. This is what Rust will resolve
/// `DisplayHack<T>::display` to when `T: Debug`.
impl<T: fmt::Debug + ?Sized> DefaultDisplay for DisplayHack<T> {
#[inline(always)]
fn display(&self) -> impl fmt::Display {
Formatter(|f| fmt::Debug::fmt(&self.0, f))
}
}

/// "Specialized" implementation for `T: Display`. This is what Rust will
/// resolve `DisplayHack<T>::display` to when `T: Display`.
impl<T: fmt::Display + fmt::Debug + ?Sized> DisplayHack<T> {
#[inline(always)]
pub fn display(&self) -> impl fmt::Display + '_ {
Formatter(|f| fmt::Display::fmt(&self.0, f))
}
}
}
3 changes: 1 addition & 2 deletions core/lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ pub use tokio;
pub use figment;
pub use time;
pub use tracing;
pub use either;

#[macro_use]
pub mod trace;
Expand Down Expand Up @@ -164,8 +165,6 @@ mod router;
mod phase;
mod erased;

#[doc(hidden)] pub use either::Either;

#[doc(inline)] pub use rocket_codegen::*;

#[doc(inline)] pub use crate::response::Response;
Expand Down
Loading

0 comments on commit 6f1c2a7

Please sign in to comment.