refactor: [#680] http return errors instead of panicking#826
refactor: [#680] http return errors instead of panicking#826josecelano merged 1 commit intotorrust:developfrom
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #826 +/- ##
===========================================
- Coverage 78.92% 78.90% -0.03%
===========================================
Files 163 163
Lines 9212 9215 +3
===========================================
Hits 7271 7271
- Misses 1941 1944 +3 ☔ View full report in Codecov by Sentry. |
af2c3c7 to
895efe9
Compare
josecelano
left a comment
There was a problem hiding this comment.
Hi @ngthhu, I'm going to merge this becuase this is better than what we have currently. However, It would be good to open a discussion to make your choice explicit and document it. You decided to use the anyhow crate. I'm not sure if in this case that's the best option.
We could use two alternatives:
- Enums
- Type-erased errors
Even anyhow recommends using thiserror crate "if you are a library that wants to design your own dedicated error type(s) so that on failures the caller gets exactly the information that you choose".
In this case, it seems clients might be interested in handling the different errors, but looking at another similar library like request they also used a BoxError. Maybe because there could be a lot of them and it would be hard to maintain an Enum.
Since this is not part of the public API (yet) it's not critical. If we decide to publish a crate we have to think about it carefully.
So it looks like a good choice to me.
To know more about "Enum" vs "Type-erased" errors I recommend the chapter 2 (ERROR HANDLING) of the Jon Gjengset' book:
https://rust-for-rustaceans.com/
|
ACK 895efe9 |
|
Thank your @ngthhu, and remember to sign commits 🙏🏼. |
Refactor: HTTP tracker client returns errors instead of panicking.