KizzyCode/http-tiny-rust

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: ErrorKind should indeed implement std::error::Error so that the resulting error type also implements Error. 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 implements From<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 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 🀞🏼

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)?;

πŸ˜€