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

Lukas Wagner l.wagner at proxmox.com
Mon Jul 22 09:30:27 CEST 2024



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.

-- 
- Lukas




More information about the pbs-devel mailing list