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

It is hard to use metered with a wrapped error #43

Open
espindola opened this issue Sep 19, 2023 · 3 comments
Open

It is hard to use metered with a wrapped error #43

espindola opened this issue Sep 19, 2023 · 3 comments

Comments

@espindola
Copy link

If you have

#[error_count(name = ErrorCount)]
enum Error {
    Foo,
    Bar,
}

It is hard to use it with a function like:

struct Wrapper<E> {
    e: E,
}

#[metered::metered(registry = BizMetrics)]
impl Biz {
    #[measure([WrappedMetric<ErrorCount>])]
    pub fn biz(&self, foo: bool) -> Result<(), Wrapper<Error>> {
        let e = if foo { Error::Foo } else { Error::Bar };
        Err(Wrapper { e })
    }
}

I did manage to write WrappedMetric, but had to require that T implements default. The full code is:

use metered::{
    clear::Clear,
    error_count,
    metric::{Advice, OnResult, OnResultMut},
    Enter, Metric,
};
use serde::Serialize;
use std::mem;

#[error_count(name = ErrorCount)]
enum Error {
    Foo,
    Bar,
}

struct Wrapper<E> {
    e: E,
}

#[derive(Serialize, Default, Debug)]
struct WrappedMetric<EC>(EC);

impl<EC: Clear> Clear for WrappedMetric<EC> {
    fn clear(&self) {
        self.0.clear();
    }
}

impl<EC: Enter> Enter for WrappedMetric<EC> {
    type E = EC::E;
    fn enter(&self) -> EC::E {
        self.0.enter()
    }
}

impl<T: Default, E, EC: Serialize + Default + Clear + Enter + OnResult<Result<T, E>>>
    Metric<Result<T, Wrapper<E>>> for WrappedMetric<EC>
{
}

impl<T: Default, E, EC: Enter + OnResult<Result<T, E>>> OnResultMut<Result<T, Wrapper<E>>>
    for WrappedMetric<EC>
{
    fn on_result(&self, enter: EC::E, result: &mut Result<T, Wrapper<E>>) -> Advice {
        // Many errors don't implement clone, so this only works if T implements default.                       
        let orig = mem::replace(result, Ok(Default::default()));
        let unwrapped = orig.map_err(|e| e.e);
        let ret = OnResult::on_result(&self.0, enter, &unwrapped);
        let recreated = unwrapped.map_err(|e| Wrapper { e });
        let _ = mem::replace(result, recreated);
        ret
    }
}

#[derive(Default, Debug)]
struct Biz {
    metrics: BizMetrics,
}

#[metered::metered(registry = BizMetrics)]
impl Biz {
    #[measure([WrappedMetric<ErrorCount>])]
    pub fn biz(&self, foo: bool) -> Result<(), Wrapper<Error>> {
        let e = if foo { Error::Foo } else { Error::Bar };
        Err(Wrapper { e })
    }
}

fn main() {
    let b = Biz {
        metrics: Default::default(),
    };
    b.biz(false).unwrap_err();
    b.biz(false).unwrap_err();
    b.biz(true).unwrap_err();
    let m = b.metrics.biz.wrapped_metric.0;
    println!("{:?} {:?}", m.foo, m.bar);
}
@espindola
Copy link
Author

I think a possible solution would be to change the signature to

 fn on_result(&self, enter: Self::E, _result: R) -> Advice { ... }

someone with a &Result<T,E> can get a Result<&T, &E> with as_ref and a Result<&T, &Wrapper<E>> can be converted to Result<T&, &E> with map_err.

@magnet
Copy link
Owner

magnet commented Dec 16, 2023

This would be a breaking change, but happy to consider a PR for this

@espindola
Copy link
Author

Cool. One simplification I found is that in order to get Metered, it is sufficient to implement OnResultMut, so we can leave the interface of OnResult unchanged. A complication is that we want to support any R, not just a Result<T, E>, so we cannot assume as_ref().

With the above what I did was change OnResultMut to take an R by value and return a new one. The changes are pretty small. I will open PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants