From 85dd6dad826449a92975ccbb815b9aaebfede310 Mon Sep 17 00:00:00 2001
From: Sean McArthur <sean@seanmonstar.com>
Date: Thu, 15 Aug 2024 06:39:12 -0700
Subject: [PATCH] dns: improve error message for hickory-dns and warn in docs
 (#2389)

---
 src/async_impl/client.rs | 30 ++++++++----------------------
 src/dns/hickory.rs       | 29 ++++++++++++++++++++---------
 2 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/src/async_impl/client.rs b/src/async_impl/client.rs
index 3bb650244..0a7281c42 100644
--- a/src/async_impl/client.rs
+++ b/src/async_impl/client.rs
@@ -1720,14 +1720,7 @@ impl ClientBuilder {
         self
     }
 
-    /// Enables the [hickory-dns](hickory_resolver) async resolver instead of a default threadpool
-    /// using `getaddrinfo`.
-    ///
-    /// If the `hickory-dns` feature is turned on, the default option is enabled.
-    ///
-    /// # Optional
-    ///
-    /// This requires the optional `hickory-dns` feature to be enabled
+    #[doc(hidden)]
     #[cfg(feature = "hickory-dns")]
     #[cfg_attr(docsrs, doc(cfg(feature = "hickory-dns")))]
     #[deprecated(note = "use `hickory_dns` instead")]
@@ -1744,6 +1737,11 @@ impl ClientBuilder {
     /// # Optional
     ///
     /// This requires the optional `hickory-dns` feature to be enabled
+    ///
+    /// # Warning
+    ///
+    /// The hickory resolver does not work exactly the same, or on all the platforms
+    /// that the default resolver does
     #[cfg(feature = "hickory-dns")]
     #[cfg_attr(docsrs, doc(cfg(feature = "hickory-dns")))]
     pub fn hickory_dns(mut self, enable: bool) -> ClientBuilder {
@@ -1751,22 +1749,10 @@ impl ClientBuilder {
         self
     }
 
-    /// Disables the hickory-dns async resolver.
-    ///
-    /// This method exists even if the optional `hickory-dns` feature is not enabled.
-    /// This can be used to ensure a `Client` doesn't use the hickory-dns async resolver
-    /// even if another dependency were to enable the optional `hickory-dns` feature.
+    #[doc(hidden)]
     #[deprecated(note = "use `no_hickory_dns` instead")]
     pub fn no_trust_dns(self) -> ClientBuilder {
-        #[cfg(feature = "hickory-dns")]
-        {
-            self.hickory_dns(false)
-        }
-
-        #[cfg(not(feature = "hickory-dns"))]
-        {
-            self
-        }
+        self.no_hickory_dns()
     }
 
     /// Disables the hickory-dns async resolver.
diff --git a/src/dns/hickory.rs b/src/dns/hickory.rs
index 44b943827..a94160b2d 100644
--- a/src/dns/hickory.rs
+++ b/src/dns/hickory.rs
@@ -1,11 +1,12 @@
 //! DNS resolution via the [hickory-resolver](https://github.com/hickory-dns/hickory-dns) crate
 
 use hickory_resolver::{
-    config::LookupIpStrategy, lookup_ip::LookupIpIntoIter, system_conf, TokioAsyncResolver,
+    config::LookupIpStrategy, error::ResolveError, lookup_ip::LookupIpIntoIter, system_conf,
+    TokioAsyncResolver,
 };
 use once_cell::sync::OnceCell;
 
-use std::io;
+use std::fmt;
 use std::net::SocketAddr;
 use std::sync::Arc;
 
@@ -24,6 +25,9 @@ struct SocketAddrs {
     iter: LookupIpIntoIter,
 }
 
+#[derive(Debug)]
+struct HickoryDnsSystemConfError(ResolveError);
+
 impl Resolve for HickoryDnsResolver {
     fn resolve(&self, name: Name) -> Resolving {
         let resolver = self.clone();
@@ -51,13 +55,20 @@ impl Iterator for SocketAddrs {
 /// which reads from `/etc/resolve.conf`. The options are
 /// overridden to look up for both IPv4 and IPv6 addresses
 /// to work with "happy eyeballs" algorithm.
-fn new_resolver() -> io::Result<TokioAsyncResolver> {
-    let (config, mut opts) = system_conf::read_system_conf().map_err(|e| {
-        io::Error::new(
-            io::ErrorKind::Other,
-            format!("error reading DNS system conf: {e}"),
-        )
-    })?;
+fn new_resolver() -> Result<TokioAsyncResolver, HickoryDnsSystemConfError> {
+    let (config, mut opts) = system_conf::read_system_conf().map_err(HickoryDnsSystemConfError)?;
     opts.ip_strategy = LookupIpStrategy::Ipv4AndIpv6;
     Ok(TokioAsyncResolver::tokio(config, opts))
 }
+
+impl fmt::Display for HickoryDnsSystemConfError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str("error reading DNS system conf for hickory-dns")
+    }
+}
+
+impl std::error::Error for HickoryDnsSystemConfError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        Some(&self.0)
+    }
+}