Type of rocket::request::Outcome is pub type Outcome = outcome::Outcome<S, (Status, E), ()> which seems weird

[DanielJoyce]

What bothers me is it requires the Status code for the error to be decided up front, instead of at the end, when usually it can be derived from the error. This also means one FromRequest might decide on (403, ActualError), and another might decide on (500, ActualError ).

Usually I prefer encoding or deriving the status from a Error, or it's responder impl, or the global catcher.

Here, up front, the status must be decided immediately.

Hmm,

Looking here in the code

#[crate::async_trait]
impl<'r, T: FromRequest<'r>> FromRequest<'r> for Result<T, T::Error> {
    type Error = std::convert::Infallible;  // Infallible here

    async fn from_request(request: &'r Request<'_>) -> Outcome<Self, Self::Error> {
        match T::from_request(request).await {
            Success(val) => Success(Ok(val)),
            Failure((_, e)) => Success(Err(e)), // Oh, we're gonna pass the error out here...?
            Forward(_) => Forward(()),
        }
    }
}

It seems the error state of Result is mapped to the success state of Outcome, which means the responder impl for Err, or the global handler would be used. So what is Outcome::Error for?

Reading the docs:

Failure
A failure Outcome<S, E, F>, Failure(E), is returned when a function fails with some error and no processing can or should continue as a result. The meaning of a failure depends on the context.

In Rocket, a Failure generally means that a request is taken out of normal processing. The request is then given to the catcher corresponding to some status code. Users can catch failures by requesting a type of Result<S, E> or Option<S> in request handlers. For example, if a user’s handler looks like:

#[post("/", data = "<my_val>")]
fn hello(my_val: Result<S, E>) { /* ... */ }
The FromData implementation for the type S returns an Outcome with a Success(S) and Failure(E). If from_data returns a Failure, the Failure value will be unwrapped and the value will be used as the Err value of my_val while a Success will be unwrapped and used the Ok value.

This seems like duplicated behavior between the default error Catcher and having errors implement Responder.

Is Outcome::Failure (status, error) intended as a shortcut?

[DanielJoyce]

Basically, I've been cleaning up code in our Rocket app, and I've had problems deciding on which way is best. I'm off the camp that if errors need a variable status code, it should be encoded in the error, mapped to a enum field, etc, by the responder.

IE, instead of returning (401, SecurityError), or (403, SecurityError), SecurityError would have a enum field, Reason::Unauthorized/Notauntheticated, and the responder for the error would then be able to set 401/403 from that. There is no chance of accidentally retuirning (402, SecurityError) or (404, SecurityError)

[DanielJoyce]

Looking at the Catchers, it appears the error is thrown away? So I assume it is logged, but the responder is not called?

[jebrosen]

I think this comment can answer some of the questions: #1145 (comment)

Specifically:

Seem like you have to choose between Status and Error when using a request guard

In a sense yes: The Error type is visible only when using Result<RequestGuard, RequestGuard::Error> and the Status is visible only by it determining which catcher is used.

There is no requirement that request guard Errors implement Responder (in fact, many don't), and it's additionally quite difficult in Rust to alter program flow at runtime based on whether or not a value implements a trait. Essentially, we have to either require all possible errors from request guards to implement Responder, build Rocket's APIs with the assumption that errors don't implement it, or come up with a new design. (There is further discussion of this issue, and ways we could do better, in #749.)

[DanielJoyce]

I find OutCome plays very poorly with Result. Any reason Result wasn't used with something like Result(ValueOrForward, Error)

ValueOrForward would then be ValueOrForward::Value and ValueOrForward::Forward

This would also make ? work again with early error return as opposed to needing try_outcome!

[DanielJoyce]

Its potentially conceptually not as pure, but forward is not a failure, and it plays nice with a lot more of the existing rust infrastructure.

[SergioBenitez]

I think all of your qualms would be solved with try_trait_v2. Any suggestion to nest Results produces an arbitrary preference between Forward and Failure which is undesired and exactly what is avoided with Outcome.

I find OutCome plays very poorly with Result.

Does this just mean you find it difficult to convert between the two? We can add methods to Result via an extension trait to ease this, for instance. Imagine how it would feel to interoperate between Option and Result if it weren't for the myriad conversion methods provided by the standard library.

Edit: I should add that I've considered adjusting Outcome to be:

enum Outcome<T, E, F> {
    Result(Result<T, E>),
    Forward(F),
}

Then there's the obvious Into<Outcome> for Result<T, E> and adapter methods become a bit more obvious, though not significantly so.

[SergioBenitez]

I'm closing this out as there doesn't appear to be anything actionable here.