tikv / fail-rs

Fail points for rust
Apache License 2.0
338 stars 40 forks source link

Add `try_fail_point!` #68

Open cgwalters opened 1 year ago

cgwalters commented 1 year ago

In my project I have a ton of code which uses anyhow::Result. I want a convenient way to force a function to return a stock error from a failpoint. Today this requires something like:

    fail::fail_point!("main", true, |msg| {
        let msg = msg.as_deref().unwrap_or("synthetic error");
        Err(anyhow::anyhow!("{msg}"))
    });

which is cumbersome to copy around.

Now, I conservatively made this a new macro. I am not sure how often the use case of a fail point for an infallible (i.e. non-Result) function occurs. It may make sense to require those to take a distinct inject_point! or something?

cgwalters commented 1 year ago

So this doesn't actually compile right now, I get:

error[E0277]: the size for values of type `dyn std::error::Error + Send + Sync` cannot be known at compilation time
    --> src/lib.rs:854:19
     |
854  |             Err(e)?;
     |                   ^ doesn't have a size known at compile-time
...
1067 |         try_fail_point!("tryfail");
     |         -------------------------- in this macro invocation

For which I'm a little confused about right now and hoping that this is obvious to you :smile:

BusyJay commented 1 year ago

I think that's because compiler can't figure out the correct type for Err(e).

cgwalters commented 1 year ago

Yeah, it seems like we should be able to structure the code to do this though. Anyways I started to pass 30 minutes of trying to figure it out and decided I should toss up the PR as is to see if the functionality was desired first - does it make sense to you? If so I'll circle back to working on this at some point.

cgwalters commented 1 year ago

OK cool, got this to work! In retrospect it was a bit obvious, I had to make an error type for the crate that can be then converted into other error types.

cgwalters commented 1 year ago

OK, cleaned up even more - we can now (IMO) remove the whole doc section which talks about how to manually deal with Result with fail_point! and just point at try_fail_point!. I also did some other tweaks like supporting std::io::Error directly.

cgwalters commented 1 year ago

OK for now I copied this macro into our project, see e.g. https://github.com/coreos/rpm-ostree/pull/4259

But since I think the eval bit is an implementation detail, and I'd like to use this in other crates, it'd be nice to have this PR merged when you get a chance; thanks!