Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FromData Failures cannot be caught by error catchers and are not debug-logged #1199

Closed
villem opened this issue Jan 11, 2020 · 9 comments
Closed
Labels
deficiency Something doesn't work as well as it could

Comments

@villem
Copy link

villem commented Jan 11, 2020

Hello,

Bug reports must include:

  1. The version of Rocket you're using. Ensure it's the latest, if possible.

0.4.2, rustc --version
rustc 1.42.0-nightly (0de96d37f 2019-12-19)

  1. The operating system (distribution and version) where the issue occurs.

OSX 10.5.2
3. A brief description of the bug that includes:
* The nature of the bug.

FromData and FromDataSimple ask (status, Error) for Failure. As far as I can see the Error is not populated any where.

* When the bug occurs.
* What you expected vs. what actually happened.

It would be good to get Error output to up to default catcher response Outputs.

  1. How you uncovered the bug. Short, reproducible tests are especially useful.

Simplest FromDataSimple Guard do not show Error any where, not even debug level logs.

  1. Ideas, if any, about what Rocket is doing incorrectly.

Rocket transforms rocket::request::Outcome to rocket::Outcome and just drops the addition E info all together.
It would be very convenient if the Error info could be populated from data parsers to above levels. Now I need to do FromDataSimple functionality in post route fn itself.

@jebrosen
Copy link
Collaborator

This is covered in the docs at https://api.rocket.rs/v0.4/rocket/data/trait.FromData.html#outcomes

Note that users can request types of Result<S, E> and Option<S> to catch Failures and retrieve the error value.

We do want to expand the functionality so that errors from just about anywhere could go to error catchers (#749 (comment)), but it needs some work in rustc.

@jebrosen jebrosen added the deficiency Something doesn't work as well as it could label Jan 11, 2020
@jebrosen jebrosen changed the title rocket::request::Outcome Failure(status, E), E is unused and not populated FromData Failures cannot be caught by error catchers and are not debug-logged Jan 11, 2020
@villem
Copy link
Author

villem commented Jan 12, 2020

This is covered in the docs at https://api.rocket.rs/v0.4/rocket/data/trait.FromData.html#outcomes

Note that users can request types of Result<S, E> and Option<S> to catch Failures and retrieve the error value.

Do you have an example how I would do this? Or isn't rustc providing needed functionally currently.

Anyways I could easily workaround this with bit of Custom responder. Rocket is really nice! Thank you!

@jebrosen
Copy link
Collaborator

jebrosen commented Jan 12, 2020

form_kitchen_sink has an example: https://github.com/SergioBenitez/Rocket/blob/v0.4/examples/form_kitchen_sink/src/main.rs#L31 - the error type here is FormError, which matches up with the Error type specified for Form (https://api.rocket.rs/v0.4/rocket/request/struct.Form.html#impl-FromData%3C%27f%3E)

@villem
Copy link
Author

villem commented Jan 12, 2020

This example is great and I can now use types in a really simple way. Thank you!

@Scipi
Copy link

Scipi commented Dec 30, 2020

Hi all,

Using rocket = "0.4.6"

I'm having a similar issue here with Request Guards. I have a couple custom guards that can fail in various ways (the database connection fails, missing headers/cookies, etc) and even though I encapsulate these failures in an Error type and pass them back to Rocket, they never get logged as reasons for Outcome::Failure.

I could always get around this in my own code by writing my own logging in the Request Guard (which can sometimes hurt readability) or by wrapping the from_request method like so:

...
impl FromRequest for MyRequestGuard {
    fn from_request(r: Request) -> Outcome<Self, Self::Error> {
        match _from_request(r) {
            o@Outcome::Success(_) => o,
            Outcome::Failure((s,e)) => {
                info!("MyRequestGuard failed with: {} - {}", s, e);
                Outcome::Failure((s, e))
            }
            o@Outcome::Forward(_) => o
        }
    }
}


fn _from_request(r: Request) -> Outcome<MyRequestGuard, MyError> {
   // ...
}

But this seems like unnecessary boilerplate for something Rocket is able to handle.

This issue is exasperated for Data Guards, too. In order to do logging it seems like I have to use Result<S, E> on every endpoint that uses them and then I have to remember to do the actual logging in the endpoint. That's fine for a couple endpoints, but when you have a couple dozen scattered around across the API it becomes very easy to miss one. It also doesn't make sense that a user would have to log an error that occurred within Rocket's handling of a request on Rocket's behalf.

@SergioBenitez
Copy link
Member

Errors and forwards from all guards are now being logged by Rocket.

@svenstaro
Copy link

I think logging is only part of the solution. I'd also like to be able to act on it in catchers which is probably the primary thing in this issue.

@SergioBenitez
Copy link
Member

SergioBenitez commented May 20, 2021

There is already an issue for handling typed errors in catchers. See #749.

@svenstaro
Copy link

Alright.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deficiency Something doesn't work as well as it could
Projects
None yet
Development

No branches or pull requests

5 participants