Re: [pve-devel] [pbs-devel] [PATCH proxmox 1/5] http-error: add new http-error crate

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Wed, Jul 26, 2023 at 02:50:02PM +0200, Lukas Wagner wrote:
> Break out proxmox-router's HttpError into it's own crate so that it can
> be used without pulling in proxmox-router.
> 
> This commit also implements `Serialize` for `HttpError` so that it can
> be returned from perlmod bindings, allowing Perl code to access the
> status code as well as the message.
> 
> Also add some smoke-tests to make sure that the `http_bail` and
> `http_err` macros actually produce valid code.
> 
> Suggested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  Cargo.toml                    |  2 +
>  proxmox-http-error/Cargo.toml | 16 +++++++
>  proxmox-http-error/src/lib.rs | 81 +++++++++++++++++++++++++++++++++++
>  3 files changed, 99 insertions(+)
>  create mode 100644 proxmox-http-error/Cargo.toml
>  create mode 100644 proxmox-http-error/src/lib.rs
> 
> diff --git a/Cargo.toml b/Cargo.toml
> index 54be5f6..cb82253 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -7,6 +7,7 @@ members = [
>      "proxmox-borrow",
>      "proxmox-compression",
>      "proxmox-http",
> +    "proxmox-http-error",
>      "proxmox-human-byte",
>      "proxmox-io",
>      "proxmox-lang",
> @@ -88,6 +89,7 @@ proxmox-api-macro = { version = "1.0.4", path = "proxmox-api-macro" }
>  proxmox-async = { version = "0.4.1", path = "proxmox-async" }
>  proxmox-compression = { version = "0.2.0", path = "proxmox-compression" }
>  proxmox-http = { version = "0.9.0", path = "proxmox-http" }
> +proxmox-http-error = { version = "0.1.0", path = "proxmox-http-error" }
>  proxmox-human-byte = { version = "0.1.0", path = "proxmox-human-byte" }
>  proxmox-io = { version = "1.0.0", path = "proxmox-io" }
>  proxmox-lang = { version = "1.1", path = "proxmox-lang" }
> diff --git a/proxmox-http-error/Cargo.toml b/proxmox-http-error/Cargo.toml
> new file mode 100644
> index 0000000..17be2e0
> --- /dev/null
> +++ b/proxmox-http-error/Cargo.toml
> @@ -0,0 +1,16 @@
> +[package]
> +name = "proxmox-http-error"
> +version = "0.1.0"
> +
> +edition.workspace = true
> +authors.workspace = true
> +license.workspace = true
> +repository.workspace = true
> +description = "Proxmox HTTP Error"
> +
> +exclude.workspace = true
> +
> +[dependencies]
> +anyhow.workspace = true
> +http.workspace = true
> +serde = { workspace = true, features = ["derive"]}

^ No need for the [derive] here if you don't use it ;-)

> diff --git a/proxmox-http-error/src/lib.rs b/proxmox-http-error/src/lib.rs
> new file mode 100644
> index 0000000..de45cb9
> --- /dev/null
> +++ b/proxmox-http-error/src/lib.rs
> @@ -0,0 +1,81 @@
> +use serde::{ser::SerializeStruct, Serialize, Serializer};
> +use std::fmt;
> +
> +#[doc(hidden)]
> +pub use http::StatusCode;
> +
> +/// HTTP error including `StatusCode` and message.
> +#[derive(Debug)]
> +pub struct HttpError {
> +    pub code: StatusCode,
> +    pub message: String,
> +}
> +
> +impl std::error::Error for HttpError {}
> +
> +impl HttpError {
> +    pub fn new(code: StatusCode, message: String) -> Self {
> +        HttpError { code, message }
> +    }
> +}
> +
> +impl fmt::Display for HttpError {
> +    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +        write!(f, "{}", self.message)
> +    }
> +}
> +
> +impl Serialize for HttpError {
> +    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
> +    where
> +        S: Serializer,
> +    {
> +        let mut state = serializer.serialize_struct("HttpError", 2)?;
> +        state.serialize_field("code", &self.code.as_u16())?;
> +        state.serialize_field("message", &self.message)?;
> +        state.end()
> +    }
> +}
> +
> +/// Macro to create a HttpError inside a anyhow::Error
> +#[macro_export]
> +macro_rules! http_err {
> +    ($status:ident, $($fmt:tt)+) => {{
> +        ::anyhow::Error::from($crate::HttpError::new(
> +            $crate::StatusCode::$status,
> +            format!($($fmt)+)
> +        ))
> +    }};
> +}
> +
> +/// Bail with an error generated with the `http_err!` macro.
> +#[macro_export]
> +macro_rules! http_bail {
> +    ($status:ident, $($fmt:tt)+) => {{
> +        return Err($crate::http_err!($status, $($fmt)+));
> +    }};
> +}
> +
> +#[cfg(test)]
> +mod tests {
> +    use super::*;
> +
> +    #[test]
> +    fn test_http_err() {
> +        // Make sure the macro generates valid code.
> +        http_err!(IM_A_TEAPOT, "Cannot brew coffee");
> +    }
> +
> +    #[test]
> +    fn test_http_bail() {
> +        fn t() -> Result<(), anyhow::Error> {
> +            // Make sure the macro generates valid code.
> +            http_bail!(
> +                UNAVAILABLE_FOR_LEGAL_REASONS,
> +                "Nothing to see here, move along"
> +            );
> +        }
> +
> +        assert!(t().is_err());
> +    }
> +}
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
>