aerospike-client-rust
aerospike-client-rust copied to clipboard
`error-chain` -> `thiserror` port
@khaf i believe i'm almost done here but need some guidance on a few things
- what to do about
log_error_chain? thethiserrorErrortype doesn't expose a.backtracemethod likeerror-chaindoes, although i believe we can get a similar effect withBacktrace::force_capture - how to handle error contexts? this was builtin to
error-chainwith the.chain_errmethod, whichthiserrordoesn't have an analog for, and whileanyhowdoes have "context" methods which serve the same purpose, they returnanyhow::Errorwrappers as opposed to raw the concrete error fromthiserror, which relates to the last concern ... - i went ahead and replaced the
bail!calls withreturn Errto avoid either 1. creating a custombail!just for this library, or 2. having to introduceanyhowwhich has a generalbail!, which would similarly have the negative of introducing another crate that add another layer to thethiserrorerror enum
@databasedav Thanks for the quick turn around! I looked around for a few hours and from what I understand thus far:
- There is no easy workaround.
thiserroruses an unstable feature for backtraces that until final release can only be used via nightly. I have not been able to come up with a solution myself. Maybe @jonas32 can chime in. - We have to implement it ourselves. This example could be helpful: https://github.com/jsvana/async-minecraft-ping/pull/8/files
- No need for
bail!. The idea with moving tothiserrorwas to have idiomatic error handling.
@khaf thoughts on using error-stack? i'm not a big fan of how the "context nesting" strategy pollutes the library level with having to deal with the Generic/WithContext error wrappers, embedding the context api into the error struct with a trait is much cleaner/more idiomatic imo
@databasedav I'm pretty sure I had answered this right away, but seeing that my response is missing, I assume I left to research and reflect and forgot to come back to it. Sorry about that.
I have nothing against the error-stack. As long as the library does not impose runtime cost, leaking abstractions and limitation upon us, I'm fine with it.
I'm open to the idea, let me know how it goes.
@databasedav I have updated this PR to finish the work you started. The code compiles now.
Having looked at error-stack, it shares an issue with anyhow in that it shows up in the public API. It is also too complicated for our needs, and has a much smaller user base in comparison to thiserror. The current implementation is not complete, since I'd like to add backtrace and source to all errors to easy debugging, but I think we are on the right track.
To chain errors, I have introduced a Wrapped option, which boxes errors inside in a chain. I will have to rename it to something better (Chain maybe? Open to suggestions) and implement some public API to allow easy introspection.
Let me know what you think.