starred/hickory-dns

Fork 0

mirror of https://github.com/hickory-dns/hickory-dns.git synced 2026-04-25 03:05:51 +03:00

[GH-ISSUE #880] The error types should implement `std::error::Error` #553

New issue

Closed

opened 2026-03-15 23:07:01 +03:00 by kerem · 10 comments

kerem commented

2026-03-15 23:07:01 +03:00

Owner

Originally created by @rotty on GitHub (Oct 11, 2019).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/880

Describe the bug
When using the error types from trust-dns with crates that rely on std::error::Error being implemented, such as the new anyhow crate, errors like this are encountered:

error[E0277]: the trait bound `trust_dns::error::client_error::Error: std::error::Error` is not satisfied

To Reproduce
Try to use the trust-dns error types with any code that requires an std::error::Error trait bound.

Expected behavior
I would expect the error types to implement std::error::Error; the impls needed should be relatively trivial.

System:
Largely irrelevant, but I used rustc v1.38.

Version:
Crate: Probably all/most of them.
Version: 0.17.0

Additional context
I would be willing to try my hand on preparing a PR to add the missing trait impls.

Originally created by @rotty on GitHub (Oct 11, 2019). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/880 **Describe the bug** When using the error types from `trust-dns` with crates that rely on `std::error::Error` being implemented, such as the new `anyhow` crate, errors like this are encountered: ``` error[E0277]: the trait bound `trust_dns::error::client_error::Error: std::error::Error` is not satisfied ``` **To Reproduce** Try to use the `trust-dns` error types with any code that requires an `std::error::Error` trait bound. **Expected behavior** I would expect the error types to implement `std::error::Error`; the impls needed should be relatively trivial. **System:** Largely irrelevant, but I used `rustc` v1.38. **Version:** Crate: Probably all/most of them. Version: 0.17.0 **Additional context** I would be willing to try my hand on preparing a PR to add the missing trait impls.

kerem

2026-03-15 23:07:01 +03:00

closed this issue
added the
crate:all

dependencies
labels

kerem commented

2026-03-15 23:07:18 +03:00

Author

Owner

@rotty commented on GitHub (Oct 11, 2019):

Uh, I've now noticed that adding std::error::Error implementations is not even possible, as they conflict with the blanket implementation inside the failure crate:

error[E0119]: conflicting implementations of trait `failure::Fail` for type `error::ProtoError`:
   --> crates/proto/src/error.rs:218:1
    |
218 | impl Fail for ProtoError {
    | ^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: conflicting implementation in crate `failure`:
            - impl<E> failure::Fail for E
              where E : 'static, E: std::error::Error, E: std::marker::Send, E: std::marker::Sync;

I'm wondering if the use of failure in a library, especially a potentially widely-used one (such as the trust-dns proto and client crates), is the best choice in the long run; it seems the error crate story in rust might converge to crates built on std::error::Error, at least that was my impression from these reddit threads:

Not being able to provide a std::error::Error implementation would be quite a limitation, if crates such as anyhow get more use. Would you be open to discuss the possibility to change the how handling is done, at least for the lower-level trust-dns crates, so that they can implement std::error::Error? I'd be willing to try my hand at a PR, once it is determined what the best path forward would be.

@rotty commented on GitHub (Oct 11, 2019): Uh, I've now noticed that adding `std::error::Error` implementations is not even possible, as they conflict with the blanket implementation inside the `failure` crate: ``` error[E0119]: conflicting implementations of trait `failure::Fail` for type `error::ProtoError`: --> crates/proto/src/error.rs:218:1 | 218 | impl Fail for ProtoError { | ^^^^^^^^^^^^^^^^^^^^^^^^ | = note: conflicting implementation in crate `failure`: - impl<E> failure::Fail for E where E : 'static, E: std::error::Error, E: std::marker::Send, E: std::marker::Sync; ``` I'm wondering if the use of `failure` in a library, especially a potentially widely-used one (such as the `trust-dns` proto and client crates), is the best choice in the long run; it seems the error crate story in rust might converge to crates built on `std::error::Error`, at least that was my impression from these reddit threads: - [`anyhow` announcement](https://old.reddit.com/r/rust/comments/deksvj/anyhowerror_anyhowresultt_a_minimal_trait_object/) - [`2019 Q4: Error patterns - `Snafu` vs `err-derive + anyhow`](https://old.reddit.com/r/rust/comments/dfs1zk/2019_q4_error_patterns_snafu_vs_errderive_anyhow/) Not being able to provide a `std::error::Error` implementation would be quite a limitation, if crates such as `anyhow` get more use. Would you be open to discuss the possibility to change the how handling is done, at least for the lower-level `trust-dns` crates, so that they can implement `std::error::Error`? I'd be willing to try my hand at a PR, once it is determined what the best path forward would be.

kerem commented

2026-03-15 23:07:23 +03:00

Author

Owner

@rotty commented on GitHub (Oct 11, 2019):

See https://github.com/rotty/tdns-cli/commit/c5e487e724542ee21e00a813e4e7da54af52f214 to get an idea of how it would affect the code when a crate decides to use anyhow in combination with trust-dns currently. Essentially, you'd have to sprinkle .compat() calls everywhere, combined with the use of the turbofish, in some cases.

This issue is not specific to trust-dns, it affects all crates that expose their use of failure::Fail, and hence cannot implement std::error::Error; this note in the current version failure's README seems relevant as well:

Failure is currently evolving as a library. First of all there is work going on in Rust itself to fix the error trait secondarily the original plan for Failure towards 1.0 is unlikely to happen in the current form.

As such the original master branch towards 1.0 of failure was removed and master now represents the future iteration steps of 0.1 until it's clear what happens in the stdlib.

@rotty commented on GitHub (Oct 11, 2019): See <https://github.com/rotty/tdns-cli/commit/c5e487e724542ee21e00a813e4e7da54af52f214> to get an idea of how it would affect the code when a crate decides to use `anyhow` in combination with `trust-dns` currently. Essentially, you'd have to sprinkle `.compat()` calls everywhere, combined with the use of the turbofish, in some cases. This issue is not specific to `trust-dns`, it affects all crates that expose their use of `failure::Fail`, and hence cannot implement `std::error::Error`; this note in the current version `failure`'s README seems relevant as well: > Failure is currently evolving as a library. First of all there is work going on in Rust itself to fix the error trait secondarily the original plan for Failure towards 1.0 is unlikely to happen in the current form. > > As such the original master branch towards 1.0 of failure was removed and master now represents the future iteration steps of 0.1 until it's clear what happens in the stdlib.

kerem commented

2026-03-15 23:07:28 +03:00

Author

Owner

@bluejekyll commented on GitHub (Oct 11, 2019):

I'm open to the idea of moving away from Failure, though I really want to maintain some (optional) support for backtrace in errors (that's the largest benefit we get from it at the moment). So if there are options for only using Error (the types are pretty close to being Error compatible), but keeping some optional way of having backtrace (maybe making failure itself an optional dep), I'm open to that.

@bluejekyll commented on GitHub (Oct 11, 2019): See also: https://github.com/bluejekyll/trust-dns/pull/855 I'm open to the idea of moving away from Failure, though I really want to maintain some (optional) support for backtrace in errors (that's the largest benefit we get from it at the moment). So if there are options for only using Error (the types are pretty close to being Error compatible), but keeping some optional way of having backtrace (maybe making failure itself an optional dep), I'm open to that.

kerem commented

2026-03-15 23:07:33 +03:00

Author

Owner

@rotty commented on GitHub (Oct 11, 2019):

Understood. I think it should be definitely possible to keep backtrace support; we just need to choose a crate that supports that. A potential PR will need to break API in any case though, as Fail no longer would be part of the public API. This will have the advantage that the error handling crate's API should then no longer be exposed publicly, and could be switched at will. I'll hold off working on the code side of things until #849 lands, and investigate crate options in the meantime; I hope to come up with a proposal, or rather a summary of available options, this weekend.

@rotty commented on GitHub (Oct 11, 2019): Understood. I think it should be definitely possible to keep backtrace support; we just need to choose a crate that supports that. A potential PR will need to break API in any case though, as `Fail` no longer would be part of the public API. This will have the advantage that the error handling crate's API should then no longer be exposed publicly, and could be switched at will. I'll hold off working on the code side of things until #849 lands, and investigate crate options in the meantime; I hope to come up with a proposal, or rather a summary of available options, this weekend.

kerem commented

2026-03-15 23:07:38 +03:00

Author

Owner

@rotty commented on GitHub (Oct 11, 2019):

Ah, and an additional requirement based on #855 is that the replacement crate would have to have optional backtrace support.

@rotty commented on GitHub (Oct 11, 2019): Ah, and an additional requirement based on #855 is that the replacement crate would have to have *optional* backtrace support.

kerem commented

2026-03-15 23:07:43 +03:00

Author

Owner

@rotty commented on GitHub (Nov 22, 2019):

Well, if you procrastinate long enough, someone else might happen to do the work for you; Yoshua Wuyts has provided an extensive overview over the currently available error handling crates. From reading that alone (no actual research done), it seems like snafu will tick all the boxes -- backtrace support, helping with defining structural errors, and being based on std::error::Error. However, it might make sense to put this issue on hold for a bit more, since it seems backtrace support is well on track for stabilization (see rust-lang/rust#53487). I imagine that for instance thiserror might include backtrace support once it is stabilized, giving more options to choose from. Maybe failure will also make a move when the backtrace std API is stabilized.

@rotty commented on GitHub (Nov 22, 2019): Well, if you procrastinate long enough, someone else might happen to do the work for you; Yoshua Wuyts has provided [an extensive overview over the currently available error handling crates](https://blog.yoshuawuyts.com/error-handling-survey/). From reading that alone (no actual research done), it seems like `snafu` will tick all the boxes -- backtrace support, helping with defining structural errors, and being based on `std::error::Error`. However, it might make sense to put this issue on hold for a bit more, since it seems backtrace support is well on track for stabilization (see rust-lang/rust#53487). I imagine that for instance `thiserror` might include backtrace support once it is stabilized, giving more options to choose from. Maybe `failure` will also make a move when the backtrace std API is stabilized.

kerem commented

2026-03-15 23:07:48 +03:00

Author

Owner

@bluejekyll commented on GitHub (Nov 22, 2019):

if you procrastinate long enough

This is pretty low priority for me, so It's pretty much up to someone else that really wants it, especially if they want it sooner rather than later.

@bluejekyll commented on GitHub (Nov 22, 2019): > if you procrastinate long enough This is pretty low priority for me, so It's pretty much up to someone else that really wants it, especially if they want it sooner rather than later.

kerem commented

2026-03-15 23:07:53 +03:00

Author

Owner

@rotty commented on GitHub (Nov 23, 2019):

if you procrastinate long enough

This is pretty low priority for me, so It's pretty much up to someone else that really wants it, especially if they want it sooner rather than later.

Just to be clear, I was referring to myself as procrastinating, as I said "I hope to come up with a proposal, or rather a summary of available options, this weekend.", which I clearly failed to do, and now the survey has been done in a much more comprehensive way than I would have done it in -- hurray!

This issue is quite low-priority for me as well, FWIW; I just wanted to indicate that I'm still interested in this, and may give it at shot at some point, but that it's maybe a good idea to see how backtrace stabilization plays out.

@rotty commented on GitHub (Nov 23, 2019): > > if you procrastinate long enough > > This is pretty low priority for me, so It's pretty much up to someone else that really wants it, especially if they want it sooner rather than later. Just to be clear, I was referring to myself as procrastinating, as I said "I hope to come up with a proposal, or rather a summary of available options, this weekend.", which I clearly failed to do, and now the survey has been done in a much more comprehensive way than I would have done it in -- hurray! This issue is quite low-priority for me as well, FWIW; I just wanted to indicate that I'm still interested in this, and may give it at shot at some point, but that it's maybe a good idea to see how backtrace stabilization plays out.

kerem commented

2026-03-15 23:07:58 +03:00

Author

Owner

@bluejekyll commented on GitHub (Nov 23, 2019):

Ah, me misreading things... 😂

Yes. I’d really love it if you have the time to look into this. Honestly we’ve gone through so many iterations on Error types, I’m very tempted to have us just drop back to only using std::Error. If backtrace does get stabilized, that would be great.

@bluejekyll commented on GitHub (Nov 23, 2019): Ah, me misreading things... 😂 Yes. I’d really love it if you have the time to look into this. Honestly we’ve gone through so many iterations on Error types, I’m very tempted to have us just drop back to only using std::Error. If backtrace does get stabilized, that would be great.

kerem commented

2026-03-15 23:08:03 +03:00

Author

Owner

@bluejekyll commented on GitHub (Oct 28, 2023):

This issue can be closed, we moved away from Failure a long time ago in favor of thiserror

@bluejekyll commented on GitHub (Oct 28, 2023): This issue can be closed, we moved away from `Failure` a long time ago in favor of `thiserror`

kerem referenced this issue

2026-03-16 02:10:50 +03:00

[PR #571] [MERGED] disable AXFR by default #1516