RUST-257 Properly fallback when unable to connect to first addre… (#123)

Author: saghm

src/cmap/conn/stream.rs

@@ -1,6 +1,6 @@
 use std::{
     io::{self, Read, Write},
-    net::{TcpStream, ToSocketAddrs},
+    net::{SocketAddr, TcpStream, ToSocketAddrs},
     sync::Arc,
     time::Duration,
 };
@@ -10,7 +10,7 @@ use derivative::Derivative;
 use webpki::DNSNameRef;
 
 use crate::{
-    error::Result,
+    error::{ErrorKind, Result},
     options::{StreamAddress, TlsOptions},
 };
 
@@ -38,27 +38,52 @@ pub(super) enum Stream {
     ),
 }
 
+fn try_connect(address: &SocketAddr, timeout: Duration) -> Result<TcpStream> {
+    // The URI options spec requires that the default connect timeout is 10 seconds, but that 0
+    // should indicate no timeout.
+    let stream = if timeout == Duration::from_secs(0) {
+        TcpStream::connect(address)?
+    } else {
+        TcpStream::connect_timeout(address, timeout)?
+    };
+
+    Ok(stream)
+}
+
+fn connect_stream(address: &StreamAddress, connect_timeout: Option<Duration>) -> Result<TcpStream> {
+    let timeout = connect_timeout.unwrap_or(DEFAULT_CONNECT_TIMEOUT);
+
+    let mut socket_addrs: Vec<_> = address.to_socket_addrs()?.collect();
+
+    if socket_addrs.is_empty() {
+        return Err(ErrorKind::NoDnsResults(address.clone()).into());
+    }
+
+    // After considering various approaches, we decided to do what other drivers do, namely try each
+    // of the addresses in sequence with a preference for IPv4.
+    socket_addrs.sort_by_key(|addr| if addr.is_ipv4() { 0 } else { 1 });
+
+    let mut connect_error = None;
+
+    for address in &socket_addrs {
+        connect_error = match try_connect(address, timeout) {
+            Ok(stream) => return Ok(stream),
+            Err(err) => Some(err),
+        };
+    }
+
+    Err(connect_error.unwrap_or_else(|| ErrorKind::NoDnsResults(address.clone()).into()))
+}
+
 impl Stream {
     /// Creates a new stream connected to `address`.
     pub(super) fn connect(
         host: StreamAddress,
         connect_timeout: Option<Duration>,
         tls_options: Option<TlsOptions>,
     ) -> Result<Self> {
-        let timeout = connect_timeout.unwrap_or(DEFAULT_CONNECT_TIMEOUT);
-
-        // The URI options spec requires that the default is 10 seconds, but that 0 should indicate
-        // no timeout.
-        let inner = if timeout == Duration::from_secs(0) {
-            TcpStream::connect(&host)?
-        } else {
-            let mut socket_addrs: Vec<_> = host.to_socket_addrs()?.collect();
-            socket_addrs.sort_by_key(|addr| if addr.is_ipv4() { 0 } else { 1 });
-
-            TcpStream::connect_timeout(&socket_addrs[0], timeout)?
-        };
+        let inner = connect_stream(&host, connect_timeout)?;
         inner.set_nodelay(true)?;
-        let inner = inner;
 
         match tls_options {
             Some(cfg) => {

src/error.rs

@@ -124,6 +124,9 @@ pub enum ErrorKind {
     #[error(display = "{}", _0)]
     Io(#[error(source)] std::io::Error),
 
+    #[error(display = "No DNS results for domain {}", _0)]
+    NoDnsResults(StreamAddress),
+
     /// A database operation failed to send or receive a reply.
     #[error(
         display = "A database operation failed to send or receive a reply: {}",