Suggestion: use `thiserror` for your Error type
Closed this issue Β· 5 comments
Hi there!
I'm writing a very minimal HTTP server to serve up some files and not much else, and came across your crate as a solution to parsing/generating headers. It's working well for me, but it appears that the error type you use in your Result doesn't implement std::error::Error and thus can't be used with ?.
My suggestion is to use thiserror to define your error type, which looks like it's become standard across the Rust ecosystem. This should give you good compatibility, fix the issue, and make it easier to maintain in the long run.
Hey @philpax, thank you for your suggestion!
First of all, nice catch: ErrorKind should indeed implement std::error::Error so that the resulting error type also implements Error. I'll fix this today or tomorrow ππ»
However AFAIK this is unrelated to whether an error can be used with an ? operator or not; the important thing is that your error type implements From<http_tiny::error::Error> β then the ?-operator should automatically convert my error type into your's.
As for thiserror β the main reason I wrote ebacktrace is to automatically capture error-backtraces if RUST_BACKTRACE is set, because this makes debugging (especially with 3rd-party error reports) much easier.
I'll take a look if it makes sense to build something similar with thiserror; but until then I'll probably stick with ebacktrace.
I hope that at some point Error::backtrace will become stable β this would probably resolve this immediately, rendering ebacktrace obsolete π€πΌ
Hey there! Appreciate you answering so quickly π
First of all, nice catch:
ErrorKindshould indeed implementstd::error::Errorso that the resulting error type also implementsError. I'll fix this today or tomorrow ππ»
Awesome, thank you!
However AFAIK this is unrelated to whether an error can be used with an
?operator or not; the important thing is that your error type implementsFrom<http_tiny::error::Error>β then the?-operator should automatically convert my error type into your's.
Yep - I'm using anyhow, which supports implicit conversion from anything that implements std::error::Error. Very handy crate!
As for
thiserrorβ the main reason I wroteebacktraceis to automatically capture error-backtraces ifRUST_BACKTRACEis set, because this makes debugging (especially with 3rd-party error reports) much easier. I'll take a look if it makes sense to build something similar withthiserror; but until then I'll probably stick withebacktrace.I hope that at some point
Error::backtracewill become stable β this would probably resolve this immediately, renderingebacktraceobsolete π€πΌ
Interesting! My understanding is that thiserror supports capturing the backtrace if you include it in your type:
The Error traitβs backtrace() method is implemented to return whichever field has a type named Backtrace, if any.
(from https://docs.rs/thiserror/latest/thiserror/)
I'd suggest giving it a shot and seeing how it fares. Implementing the Error trait should resolve my immediate issue, so it's no biggie for me, but thiserror is pretty nice to use in general π
Interesting! My understanding is that thiserror supports capturing the backtrace if you include it in your type:
The Error traitβs backtrace() method is implemented to return whichever field has a type named Backtrace, if any.
(from https://docs.rs/thiserror/latest/thiserror/)I'd suggest giving it a shot and seeing how it fares. Implementing the Error trait should resolve my immediate issue, so it's no biggie for me, but thiserror is pretty nice to use in general π
Ok; I should definitively take a closer look at this β as far as I can tell, this requires std::backtrace (which is not stable yet); but maybe there's a way to use another backtrace type instead. Or I could just use a custom field and turn the struct into a thiserror-error (I guess it does not really matter; as long as the backtrace is there and gets printed)^^
So, 1.1.0 has been published on crates.io π
Thank you! This is the kind of diff I love to see:
- .write_all(&mut self.stream)
- .ok()
- .context("failed to write header")?;
+ .write_all(&mut self.stream)?;π