Re: [pve-devel] [pbs-devel] [PATCH proxmox v2 01/12] notify: implement webhook targets

["Max Carrara" <m.carrara@proxmox.com>]

On Mon Jul 22, 2024 at 9:30 AM CEST, Lukas Wagner wrote:
>
>
> On  2024-07-17 17:35, Max Carrara wrote:
> >> +        let handlebars = setup_handlebars();
> >> +        let body_template = self.base_64_decode(self.config.body.as_deref().unwrap_or_default())?;
> >> +
> >> +        let body = handlebars
> >> +            .render_template(&body_template, &data)
> >> +            .map_err(|err| {
> >> +                // TODO: Cleanup error types, they have become a bit messy.
> >> +                // No user of the notify crate distinguish between the error types any way, so
> >> +                // we can refactor without any issues....
> >> +                Error::Generic(format!("failed to render webhook body: {err}"))
> > 
> > I'm curious, how would you clean up the error types in particular?
> > 
>
> Right now, error handling is a bit messy... Some endpoints primarily use
> the `NotifyFailed` variant, which wraps another error, while in some places
> where I need a leaf error type that does not wrap any error I use the
> `Generic` variant, which only stores a string.
> I could have used the `NotifyFailed` variant here, but that one would
> not have allowed me to add additional context ("failed to render webhook ..."),
> unless wrapping another `Generic` variant...
>
> I don't have yet made any detailed plans on how to clean that up though.

Hmm, I see - thanks for elaborating! I'm not really sure how to clean
them up either, but if I can think of something, I'll let you know.



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel