refactor DNS resolver construction

- System Conf is read as `ClientBuilder::build()` time, providing the
  error earlier.
- If there is an error reading the resolve system conf, a better error
  is reported.
- Resolver only needs to lock a mutex once to spawn the background task,
  instead of every single `resolve` call.

src/async_impl/client.rs

@@ -112,7 +112,8 @@ impl ClientBuilder {
     ///
     /// # Errors
     ///
-    /// This method fails if TLS backend cannot be initialized.
+    /// This method fails if TLS backend cannot be initialized, or the resolver
+    /// cannot load the system configuration.
     pub fn build(self) -> ::Result<Client> {
         let config = self.config;
         let proxies = Arc::new(config.proxies);
@@ -208,7 +209,7 @@ impl ClientBuilder {
             }
 
             #[cfg(not(feature = "tls"))]
-            Connector::new(proxies.clone())
+            Connector::new(proxies.clone())?
         };
 
         let hyper_client = ::hyper::Client::builder()
@@ -362,13 +363,15 @@ impl Client {
     ///
     /// # Panics
     ///
-    /// This method panics if TLS backend cannot be created or
+    /// This method panics if TLS backend cannot initialized, or the resolver
-    /// initialized. Use `Client::builder()` if you wish to handle the failure
+    /// cannot load the system configuration.
-    /// as an `Error` instead of panicking.
+    ///
+    /// Use `Client::builder()` if you wish to handle the failure as an `Error`
+    /// instead of panicking.
     pub fn new() -> Client {
         ClientBuilder::new()
             .build()
-            .expect("TLS failed to initialize")
+            .expect("Client::new()")
     }
 
     /// Creates a `ClientBuilder` to configure a `Client`.

src/client.rs

@@ -75,7 +75,8 @@ impl ClientBuilder {
     ///
     /// # Errors
     ///
-    /// This method fails if native TLS backend cannot be initialized.
+    /// This method fails if TLS backend cannot be initialized, or the resolver
+    /// cannot load the system configuration.
     pub fn build(self) -> ::Result<Client> {
         ClientHandle::new(self).map(|handle| Client {
             inner: handle,
@@ -300,13 +301,15 @@ impl Client {
     ///
     /// # Panic
     ///
-    /// This method panics if native TLS backend cannot be created or
+    /// This method panics if TLS backend cannot initialized, or the resolver
-    /// initialized. Use `Client::builder()` if you wish to handle the failure
+    /// cannot load the system configuration.
-    /// as an `Error` instead of panicking.
+    ///
+    /// Use `Client::builder()` if you wish to handle the failure as an `Error`
+    /// instead of panicking.
     pub fn new() -> Client {
         ClientBuilder::new()
             .build()
-            .expect("Client failed to initialize")
+            .expect("Client::new()")
     }
 
     /// Creates a `ClientBuilder` to configure a `Client`.

src/connect.rs

@@ -35,19 +35,19 @@ enum Inner {
 
 impl Connector {
     #[cfg(not(feature = "tls"))]
-    pub(crate) fn new(proxies: Arc<Vec<Proxy>>) -> Connector {
+    pub(crate) fn new(proxies: Arc<Vec<Proxy>>) -> ::Result<Connector> {
-        let http = http_connector();
+        let http = http_connector()?;
-        Connector {
+        Ok(Connector {
             proxies,
             inner: Inner::Http(http)
-        }
+        })
     }
 
     #[cfg(feature = "default-tls")]
     pub(crate) fn new_default_tls(tls: TlsConnectorBuilder, proxies: Arc<Vec<Proxy>>) -> ::Result<Connector> {
         let tls = try_!(tls.build());
 
-        let mut http = http_connector();
+        let mut http = http_connector()?;
         http.enforce_http(false);
         let http = ::hyper_tls::HttpsConnector::from((http, tls.clone()));
 
@@ -59,7 +59,7 @@ impl Connector {
 
     #[cfg(feature = "rustls-tls")]
     pub(crate) fn new_rustls_tls(tls: rustls::ClientConfig, proxies: Arc<Vec<Proxy>>) -> ::Result<Connector> {
-        let mut http = http_connector();
+        let mut http = http_connector()?;
         http.enforce_http(false);
         let http = ::hyper_rustls::HttpsConnector::from((http, tls.clone()));
 
@@ -70,9 +70,10 @@ impl Connector {
     }
 }
 
-fn http_connector() -> HttpConnector {
+fn http_connector() -> ::Result<HttpConnector> {
-    let http = HttpConnector::new_with_resolver(TrustDnsResolver::new());
+    TrustDnsResolver::new()
-    http
+        .map(HttpConnector::new_with_resolver)
+        .map_err(::error::dns_system_conf)
 }
 
 impl Connect for Connector {

src/dns.rs

@@ -1,62 +1,75 @@
-use std::io;
+use std::{io, vec};
-use std::sync::{Arc, Mutex};
+use std::net::IpAddr;
+use std::sync::{Arc, Mutex, Once};
 
 use futures::{future, Future};
 use hyper::client::connect::dns as hyper_dns;
-use trust_dns_resolver::AsyncResolver;
+use tokio;
+use trust_dns_resolver::{system_conf, AsyncResolver, BackgroundLookupIp};
 
 // If instead the type were just `AsyncResolver`, it breaks the default recursion limit
 // for the compiler to determine if `reqwest::Client` is `Send`. This is because `AsyncResolver`
 // has **a lot** of internal generic types that pushes us over the limit.
 //
 // "Erasing" the internal resolver type saves us from this limit.
-type ErasedResolver = Box<Fn(hyper_dns::Name) -> ::trust_dns_resolver::BackgroundLookupIp + Send + Sync>;
+type ErasedResolver = Box<dyn Fn(hyper_dns::Name) -> BackgroundLookupIp + Send + Sync>;
+type Background = Box<dyn Future<Item=(), Error=()> + Send>;
 
 #[derive(Clone)]
 pub(crate) struct TrustDnsResolver {
-    inner: Arc<Mutex<Option<ErasedResolver>>>,
+    inner: Arc<Inner>,
+}
+
+struct Inner {
+    background: Mutex<Option<Background>>,
+    once: Once,
+    resolver: ErasedResolver,
 }
 
 impl TrustDnsResolver {
-    pub(crate) fn new() -> Self {
+    pub(crate) fn new() -> io::Result<Self> {
-        TrustDnsResolver {
+        let (conf, opts) = system_conf::read_system_conf()?;
-            inner: Arc::new(Mutex::new(None)),
+        let (resolver, bg) = AsyncResolver::new(conf, opts);
-        }
+
+        let resolver: ErasedResolver = Box::new(move |name| {
+            resolver.lookup_ip(name.as_str())
+        });
+        let background = Mutex::new(Some(Box::new(bg) as Background));
+        let once = Once::new();
+
+        Ok(TrustDnsResolver {
+            inner: Arc::new(Inner {
+                background,
+                once,
+                resolver,
+            }),
+        })
     }
 }
 
 impl hyper_dns::Resolve for TrustDnsResolver {
-    type Addrs = ::std::vec::IntoIter<::std::net::IpAddr>;
+    type Addrs = vec::IntoIter<IpAddr>;
     type Future = Box<Future<Item=Self::Addrs, Error=io::Error> + Send>;
 
     fn resolve(&self, name: hyper_dns::Name) -> Self::Future {
         let inner = self.inner.clone();
         Box::new(future::lazy(move || {
-            let mut inner = inner.lock().expect("lock resolver");
+            inner.once.call_once(|| {
-            if inner.is_none() {
                 // The `bg` (background) future needs to be spawned onto an executor,
                 // but a `reqwest::Client` may be constructed before an executor is ready.
                 // So, the `bg` future cannot be spawned *until* the executor starts to
                 // `poll` this future.
-                match AsyncResolver::from_system_conf() {
+                let bg = inner
-                    Ok((resolver, bg)) => {
+                    .background
-                        ::tokio::spawn(bg);
+                    .lock()
-                        *inner = Some(Box::new(move |name| {
+                    .expect("resolver background lock")
-                            resolver.lookup_ip(name.as_str())
+                    .take()
-                        }));
+                    .expect("background only taken once");
-                    },
-                    Err(err) => {
-                        return future::Either::A(
-                            future::err(io::Error::new(io::ErrorKind::Other, err.to_string()))
-                        );
-                    }
-                }
-            }
 
-            future::Either::B((inner
+                tokio::spawn(bg);
-                .as_mut()
+            });
-                .expect("resolver is set"))(name)
+
-                //.lookup_ip(name.as_str())
+            (inner.resolver)(name)
                 .map(|lookup| {
                     lookup
                         .iter()
@@ -65,9 +78,8 @@ impl hyper_dns::Resolve for TrustDnsResolver {
                 })
                 .map_err(|err| {
                     io::Error::new(io::ErrorKind::Other, err.to_string())
-                }))
+                })
         }))
     }
 }
 
-

src/error.rs

@@ -140,6 +140,7 @@ impl Error {
             Kind::NativeTls(ref e) => Some(e),
             #[cfg(feature = "rustls-tls")]
             Kind::Rustls(ref e) => Some(e),
+            Kind::DnsSystemConf(ref e) => Some(e),
             Kind::Io(ref e) => Some(e),
             Kind::UrlEncoded(ref e) => Some(e),
             Kind::Json(ref e) => Some(e),
@@ -237,6 +238,9 @@ impl fmt::Display for Error {
             Kind::NativeTls(ref e) => fmt::Display::fmt(e, f),
             #[cfg(feature = "rustls-tls")]
             Kind::Rustls(ref e) => fmt::Display::fmt(e, f),
+            Kind::DnsSystemConf(ref e) => {
+                write!(f, "failed to load DNS system conf: {}", e)
+            },
             Kind::Io(ref e) => fmt::Display::fmt(e, f),
             Kind::UrlEncoded(ref e) => fmt::Display::fmt(e, f),
             Kind::Json(ref e) => fmt::Display::fmt(e, f),
@@ -268,6 +272,7 @@ impl StdError for Error {
             Kind::NativeTls(ref e) => e.description(),
             #[cfg(feature = "rustls-tls")]
             Kind::Rustls(ref e) => e.description(),
+            Kind::DnsSystemConf(_) => "failed to load DNS system conf",
             Kind::Io(ref e) => e.description(),
             Kind::UrlEncoded(ref e) => e.description(),
             Kind::Json(ref e) => e.description(),
@@ -291,6 +296,7 @@ impl StdError for Error {
             Kind::NativeTls(ref e) => e.cause(),
             #[cfg(feature = "rustls-tls")]
             Kind::Rustls(ref e) => e.cause(),
+            Kind::DnsSystemConf(ref e) => e.cause(),
             Kind::Io(ref e) => e.cause(),
             Kind::UrlEncoded(ref e) => e.cause(),
             Kind::Json(ref e) => e.cause(),
@@ -316,6 +322,7 @@ pub(crate) enum Kind {
     NativeTls(::native_tls::Error),
     #[cfg(feature = "rustls-tls")]
     Rustls(::rustls::TLSError),
+    DnsSystemConf(io::Error),
     Io(io::Error),
     UrlEncoded(::serde_urlencoded::ser::Error),
     Json(::serde_json::Error),
@@ -489,6 +496,10 @@ pub(crate) fn url_bad_scheme(url: Url) -> Error {
     Error::new(Kind::UrlBadScheme, Some(url))
 }
 
+pub(crate) fn dns_system_conf(io: io::Error) -> Error {
+    Error::new(Kind::DnsSystemConf(io), None)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/lib.rs

@@ -13,7 +13,7 @@
 //!
 //! - Plain bodies, [JSON](#json), [urlencoded](#forms), [multipart](multipart)
 //! - Customizable [redirect policy](#redirect-policy)
-//! - HTTP [Proxies](proxies)
+//! - HTTP [Proxies](#proxies)
 //! - Uses system-native [TLS](#tls)
 //! - Cookies (only rudimentary support, full support is TODO)
 //!