http-rs / http-types

Common types for HTTP operations
https://docs.rs/http-types
Apache License 2.0
200 stars 84 forks source link

bail! macro throws an error when the argument is a custom struct #365

Closed jasontheiler closed 1 year ago

jasontheiler commented 3 years ago

Hey everybody 👋

When I try to pass a custom struct (which implements Display and Debug) as an argument to the bail! macro, it throws the following error:

error[E0433]: failed to resolve: use of undeclared type `Error`
...

I think this is caused by this piece of the format_err! macro, because it seems to be different from all other macro evaluations:

($err:expr $(,)?) => ({
    let error = $err;
    Error::new_adhoc(error)
});

When I try to import tide::http::Error I get the following error:

error[E0624]: associated function `new_adhoc` is private
...

My best guess is that it is just a bug, since the macro's description also states that it can take a string, format string or custom type that implements Display and Debug.

Help would be really appreciated! 😅

Fishrock123 commented 3 years ago

@jasontheiler What is the output of your rustc -Vv?

jasontheiler commented 3 years ago

@Fishrock123

rustc 1.52.1 (9bc8c42bb 2021-05-09)
binary: rustc
commit-hash: 9bc8c42bb2f19e745a63f3445f1ac248fb015e53
commit-date: 2021-05-09
host: x86_64-unknown-linux-gnu
release: 1.52.1
LLVM version: 12.0.0
Fishrock123 commented 3 years ago

Could you provide complete examples of what does and doesn't work?

jasontheiler commented 3 years ago

I built a tide endpoint (since I use currently only use it there) to demonstrate that the bail_status! macro works with the CustomError struct, while the bail! macro throws the previously described error:

use std::fmt::{self, Display, Formatter};

use serde::Deserialize;
use tide::http::{bail, bail_status};
use tide::{Request, Response};

#[derive(Debug, Clone, Deserialize)]
struct User {
    name: String,
}

#[derive(Debug, Clone)]
struct CustomError {
    message: String,
}

impl Display for CustomError {
    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
        write!(f, "{}", self.message)
    }
}

async fn example_endpoint(mut req: Request<()>) -> tide::Result {
    // Works
    let _data = match req.body_json::<User>().await {
        Ok(data) => data,
        Err(_) => bail_status!(
            400,
            CustomError {
                message: "The request body could not be deserialized.".to_string()
            }
        ),
    };

    // Throws:
    // error[E0433]: failed to resolve: use of undeclared type `Error`
    //   --> src/controllers/users.rs:57:19
    //    |
    // 57 |           Err(_) => bail!(CustomError {
    //    |  ___________________^
    // 58 | |             message: "The request body could not be deserialized.".to_string()
    // 59 | |         }),
    //    | |__________^ not found in this scope
    //    |
    //    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
    let _data = match req.body_json::<User>().await {
        Ok(data) => data,
        Err(_) => bail!(CustomError {
            message: "The request body could not be deserialized.".to_string()
        }),
    };

    Ok(Response::new(200))
}

I hope this helps!

Fishrock123 commented 3 years ago

Fwiw bail! and similar macros are intended to be replaced by bail_status! (and similar _status) macros in the future.

jasontheiler commented 3 years ago

Really? I kinda liked the idea of having a status in my own Error struct, so that I can create different types of errors that are coupled to a specific status. E.g. an error for a failed deserialization of a request body that is coupled to a 400 Bad Request.

jasontheiler commented 3 years ago

I suppose this means that the issue is basically irrelevant and not worth discussing any longer, so I will close this issue here. Might I suggest to use #[deprecated] if you are already certain that something is going to be replaced in the near future?

Fishrock123 commented 3 years ago

I think there may be a bug here or a reasonable feature but I'm also busy and not sure what path forward @yoshuawuyts was planning here.