Typed catchers

[the10thWiz]

Addresses #749, adds support for typed catchers via the transient crate.

When a FromRequest, FromParam, FromQuery, or FromData implementation returns an Error or Forward, if the returned error implements Any<Co<'r>>, it is boxed and travels with the Outcome.

A catcher can have a third parameter, now (&E, Status, &Request), where E is some type that implements Any<Co<'r>>. If the error contained in the Outcome is of the same type, it is downcast to
the specific type, and passed to the catcher.

A working example can be seen in the error-handling example - simply visit /hello/name/a to see a ParseIntError returned by the FromParam of i8.

TODO

• Better syntax for passing the error type to catchers. This is likely going to look like a new FromError trait, and a error = "<error>" parameter for the upcast error.
  • Requires new compile tests
• Catchers should carry an Option<TypeId>, and use it to prevent false collisions.
  • Update matcher code to also check TypeId
• Special TypedError trait
• Responder should return an Outcome<..., Self::Error, ...>
• Improve example error message
• Any<Co<'r>> should be Any<Inv<'r>>. Transient actually already protects us from casting Any<Inv<'r>> to a different Any<Inv<'l>>, by telling the compiler that Inv<'a> is invariant wrt 'a.
• Documentation needs to be written - this is a pretty big change, and somewhat difficult to use. There are a number of small gotchas - from types that don't implement Transient, to simply using the wrong error type in the catcher. I'm not sure what the right way to present this is.
• We should consider logging or carrying the error type's name, to help users identify what the type is, and why their catcher doesn't catch it. (Also log if the type implements Transient, since many types don't).
  • Tests (likely using a tracing subscriber) should validate this (I'm not sure how, since type_name isn't a stable name - nevermind, it is, since the test is compiled with the same version).
• Export a TypedError derive macro.
  • I'm current involved with @JRRudy1 in working on an improved derive macro, which would be safe to use.
  • Also derive a Transient implementation at the same time.
• rocket::form::Errors (and it's parts) should implement Transient. (Now non-issue)
• FromError trait
• Fairings should be able to reject requests (and include a TypedError)
• Clean up TODOs
• Contrib
  • dyn_templates: The responder and fairing impls need to be updated.
  • (sync_)db_pools: The error types should implement TypedError.
  • ws: The error types should implement TypedError, and responder/fairing impls need to be updated.

[SergioBenitez]

Thank you so, so much for this. As you know, this is a feature that's been
requested for quite some time now, so it's exciting to see some real progress
here.

While reviewing this PR, I found myself consistently asking:

• Does this overcome the fundamental technical hurdles that have plagued this
  getting this feature into Rocket?
• Does this meet the usability requirements we expect?
• Is this feature-complete enough to avoid missing expectation due to missing
  features?

I believe the answer to the first question is yes, which is truly exciting!
I temper this statement only because I also believe we're missing some key
usability-impacting features, and there's a concern that the approach won't
extend to these features, but at a first glance it seems probable.

Feature-Completeness

This PR current allows errors from input guards (FromParam, FromSegments,
FromForm, and FromData) to be type-erased and later downcast into a concrete
type in an error catcher. The critical missing piece is allowing the same for
responders. In particular, we want the following to compile and work as one
might expect:

#[derive(Debug, Transient /* Error */)]
struct MyType<'r>(&'r str);

#[get("/<foo>")]
fn hello(foo: &str) -> Result<&'static str, MyType<'_>> {
    Err(MyType(foo))
}

#[catch(default)]
fn error(foo: &MyType) -> &str {
    foo.0
}

This means that Responder needs to change to accommodate an error type.

type Outcome<'o> = Outcome<Response<'o>, E, Status>;

pub trait Responder<'r, 'o: 'r> {
    type Error: Debug + 'o;

    fn respond_to(self, request: &'r Request<'_>) -> Outcome<'o, Self::Error>;
}

Where:

• Outcome::Success(Response) is akin to today's Ok(Response)
• Outcome::Forward(Status) is akin to today's Err(Status) and can be read
  as "forwarding to the catcher for Status"
• Outcome::Error(Self::Error) is new. For many types, Self:Error will be
  set to Infallible, implying that the responder only succeeds or forwards.

One thing to consider is that we probably want to associate a status with the
error, and we also want to allow the error to respond directly if nothing
catches it.

Here we have two obvious approaches:

• Require Responder::Error: Responder, apply the static dispatch
  specialization trick to check for T: Transient as this PR does. Check that
  we don't run into an infinite error-loop (i.e, a responder that returns
  itself as an error which then errors with itself, and so on).

• Create a new trait, Error, and apply the static dispatch specialization
  trick to check for T: Error. The trait might look something like:

  trait Error<'r>: Transient + .. {
      fn respond_to(&self, request: &'r Request<'_>) -> Result<Response<'r>, Status> {
          Err(self.status())
      }
  
      /// Maybe? Ignore for now.
      fn status(&self) -> Status { Status::InternalServerError }
  
      /// Log on catcher failure: expected type Foo, have error Bar.
      fn type_name(&self) -> &'static str { .. }
  
      fn upcast(self) -> Box<dyn Transient<'r>> { .. }
  
      fn source(&self) -> Option<Box<dyn Error<'r>>> { .. }
  }

It's not obvious to me at this point which approach might be better, or if
there's another approach that's missing here. The main difference is that the
former requires the error type to implement Responder, which may not be what
we want, and furthermore that the Responder implementation not vary between
being used as a regular response and as an error. As such, I lean towards the
latter approach.

Usability

My main concerns on the usability front are two-fold:

• the extensions to the #[catch] macro. In particular, I'd like to find a
  way to remove the need for additional attribute arguments of status and
  perhaps even error.
• the difficulty in implementing custom downcastable errors

#[catch]

In other attributes, like #[route], we need attribute parameters because
either they provide information about what to parse (path, segments, and query
params) or they operate in a fundamentally different manner at the type-system
level: (data and request). There's also a security and category-theoretical
argument to be made about creating different class types for parseable
parameters.

But with status and error, none of these argument need necessarily apply:
all of the information derives from references or copy-able types without and
particular security considerations. So the only reason* (there's one reason we
might want an error argument, actually) for the arguments to exist is for our
sake in implementing codegen.

More concretely, the #[catch] attribute separates types into three categories:

• T: FromRequest
• T: Transient via error =
• Status via status =

I argue that in reality, these should all be one:

• T: FromError

  trait FromError<'r> {
      fn from_error(request: &'r Request<'_>, error: Option<ErasedError>);
  }

Then we can recover all of the obvious implementations:

impl<'r> FromError<'r> for Status {
    fn from_error(request: &'r Request<'_>, error: Option<ErasedError>) {
        error.as_ref().map_or(Status::InternalServerError, |e| e.status())
    }
}

Implementing Custom Errors

Consider the following complete program:

use rocket::*;
use rocket::request::{self, Request, FromRequest};
use rocket::outcome::Outcome;
use rocket::http::Status;

#[derive(Debug, transient::Transient)]
struct MyError<'r>(&'r str);

#[derive(Debug)]
struct MyGuard;

#[rocket::async_trait]
impl<'r> FromRequest<'r> for MyGuard {
    type Error = MyError<'r>;

    async fn from_request(req: &'r Request<'_>) -> request::Outcome<Self, Self::Error> {
        Outcome::Error((Status::InternalServerError, MyError(req.uri().path().as_str())))
    }
}

#[get("/")]
fn index(guard: MyGuard) -> &'static str {
    "Hello, world!"
}

#[catch(default, error = "<foo>")]
fn error<'a>(foo: &MyError<'a>) -> &'a str {
    foo.0
}

#[launch]
fn rocket() -> _ {
    rocket::build()
        .mount("/", routes![index])
        .register("/", catchers![error])
}

There are a few issues here:

• I needed to depend on transient directly or else the derive wouldn't work
  since it references transient::. This shouldn't need to be the case.
• The error catcher fails at runtime because, for whatever reason, the error
  type MyError is not meeting the specialization requirements. I would
  expect that if my type doesn't meet the requirements to be an error, then
  I wouldn't be allowed to use it as typed error at all, and my program would
  fail to compile.
• Covariance needs to be declared: can we figure it out automatically, i.e,
  via by calling rustc, or for references syntactically (&T)?

This also begs the question: what exactly is required to be implemented of error
types? Transient does not appear to be sufficient. This further supports the
idea of a Rocket-specific Error trait that has all of the requirements
built-in. And if Error is a Rocket trait, then we can re-export the derive
to allow users to do the following, completely relegating transient to an
implementation detail.

#[derive(rocket::Error)]
struct MyError<'r> { .. }

---

As an aside, the following gives an error message that we should look to improve:

#[derive(Debug, Responder, Transient)]
struct MyType<'r>(&'r str);

#[catch(default, error = "<foo>")]
fn error<'a>(uri: &Origin, foo: &'a MyType) -> &'a str {
    foo.0
}

[SergioBenitez]

This feels brittle. What we're doing here is partitioning catchers into typed and untyped variants. If that's the case, then we should do so directly. May I suggest:

fn rank(base: Path<'_>, typed: bool) -> isize {
    let length = base.segments().filter(|s| !s.is_empty()).count();
    let order = (length * 2) + (typed as usize);
    -(order as isize)
}

[the10thWiz]

This logic has been fully removed. The preference for typed catchers is now implemented in lifecycle.rs, since it also needs to check the error's .source() method, it now always checks for a matching type before a catcher with any type.

[SergioBenitez]

Then this becomes rank(uri, false).

[SergioBenitez]

This doesn't feel right. It doesn't look like matching was changed, so it's almost certainly incorrect for collision checking to be changed. When we look at the rules from #2790 (comment), and in particular:

$$\forall r_1 r_2, q. \text{match}(q, r_1) \land match(q, r_2) \implies collide(r_1, r_2)$$

...we see that this breaks the rule since two catchers can match but not collide, i.e., when two types have the same base and code but differently typed errors, we get a match for both no collision which leads to a routing ambiguity.