RUST-268 Improve server selection timeout error messages (#116)

Author: patrickfreed

src/client/mod.rs

@@ -213,7 +213,13 @@ impl Client {
         }
 
         Err(ErrorKind::ServerSelectionError {
-            message: "timed out while trying to select server".into(),
+            message: self
+                .inner
+                .topology
+                .read()
+                .unwrap()
+                .description
+                .server_selection_timeout_error_message(&criteria),
         }
         .into())
     }

src/error.rs

@@ -149,7 +149,7 @@ pub enum ErrorKind {
     ResponseError { message: String },
 
     /// The Client was not able to select a server for the operation.
-    #[error(display = ": {}", message)]
+    #[error(display = "{}", message)]
     ServerSelectionError { message: String },
 
     /// An error occurred during SRV record lookup.

src/sdam/description/server.rs

@@ -155,6 +155,14 @@ impl ServerDescription {
         description
     }
 
+    /// Whether this server is "available" as per the definition in the server selection spec.
+    pub(crate) fn is_available(&self) -> bool {
+        match self.server_type {
+            ServerType::Unknown => false,
+            _ => true,
+        }
+    }
+
     pub(crate) fn compatibility_error_message(&self) -> Option<String> {
         if let Ok(Some(ref reply)) = self.reply {
             let is_master_min_wire_version = reply.command_response.min_wire_version.unwrap_or(0);

src/sdam/description/topology/server_selection/mod.rs

@@ -1,7 +1,7 @@
 #[cfg(test)]
 mod test;
 
-use std::time::Duration;
+use std::{fmt, time::Duration};
 
 use rand::seq::IteratorRandom;
 
@@ -21,6 +21,24 @@ use crate::{
 const DEFAULT_LOCAL_THRESHOLD: Duration = Duration::from_millis(15);
 
 impl TopologyDescription {
+    pub(crate) fn server_selection_timeout_error_message(
+        &self,
+        criteria: &SelectionCriteria,
+    ) -> String {
+        if self.has_available_servers() {
+            format!(
+                "Server selection timeout: None of the available servers suitable for criteria \
+                 {:?}. Topology: {}",
+                criteria, self
+            )
+        } else {
+            format!(
+                "Server selection timeout: No available servers. Topology: {}",
+                self
+            )
+        }
+    }
+
     pub(crate) fn select_server<'a>(
         &'a self,
         criteria: &'a SelectionCriteria,
@@ -56,6 +74,10 @@ impl TopologyDescription {
         Ok(suitable_servers.into_iter().choose(&mut rand::thread_rng()))
     }
 
+    fn has_available_servers(&self) -> bool {
+        self.servers.values().any(|server| server.is_available())
+    }
+
     fn suitable_servers<'a>(
         &'a self,
         read_preference: &'a ReadPreference,
@@ -290,6 +312,16 @@ impl TopologyDescription {
     }
 }
 
+impl fmt::Display for TopologyDescription {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{{ Type: {:?}, Servers: [ ", self.topology_type)?;
+        for server_info in self.servers.values().map(ServerInfo::new) {
+            write!(f, "{}, ", server_info)?;
+        }
+        write!(f, "] }}")
+    }
+}
+
 fn filter_servers_by_tag_sets(servers: &mut Vec<&ServerDescription>, tag_sets: &[TagSet]) {
     if tag_sets.is_empty() {
         return;

src/sdam/public.rs

@@ -1,4 +1,4 @@
-use std::time::Duration;
+use std::{fmt, time::Duration};
 
 use bson::UtcDateTime;
 
@@ -82,3 +82,55 @@ impl<'a> ServerInfo<'a> {
         self.command_response_getter(|r| r.tags.as_ref())
     }
 }
+
+impl<'a> fmt::Display for ServerInfo<'a> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> std::result::Result<(), fmt::Error> {
+        write!(
+            f,
+            "{{ Address: {}, Type: {:?}",
+            self.address(),
+            self.server_type()
+        )?;
+
+        match self.description.reply {
+            Ok(_) => {
+                if let Some(avg_rtt) = self.average_round_trip_time() {
+                    write!(f, ", Average RTT: {:?}", avg_rtt)?;
+                }
+
+                if let Some(last_update_time) = self.last_update_time() {
+                    write!(f, ", Last Update Time: {:?}", last_update_time)?;
+                }
+
+                if let Some(max_wire_version) = self.max_wire_version() {
+                    write!(f, ", Max Wire Version: {}", max_wire_version)?;
+                }
+
+                if let Some(min_wire_version) = self.min_wire_version() {
+                    write!(f, ", Min Wire Version: {}", min_wire_version)?;
+                }
+
+                if let Some(rs_name) = self.replica_set_name() {
+                    write!(f, ", Replica Set Name: {}", rs_name)?;
+                }
+
+                if let Some(rs_version) = self.replica_set_version() {
+                    write!(f, ", Replica Set Version: {}", rs_version)?;
+                }
+
+                if let Some(tags) = self.tags() {
+                    write!(f, ", Tags: {:?}", tags)?;
+                }
+
+                if let Some(compatibility_error) = self.description.compatibility_error_message() {
+                    write!(f, ", Compatiblity Error: {}", compatibility_error)?;
+                }
+            }
+            Err(ref e) => {
+                write!(f, ", Error: {}", e)?;
+            }
+        }
+
+        write!(f, " }}")
+    }
+}

src/test/client.rs

@@ -1,4 +1,4 @@
-use std::borrow::Cow;
+use std::{borrow::Cow, collections::HashMap, time::Duration};
 
 use bson::{bson, doc, Bson};
 use serde::Deserialize;
@@ -9,6 +9,7 @@ use crate::{
         auth::{AuthMechanism, Credential},
         ClientOptions,
     },
+    selection_criteria::{ReadPreference, SelectionCriteria},
     test::{CLIENT, LOCK},
     Client,
 };
@@ -57,6 +58,40 @@ fn metadata_sent_in_handshake() {
     assert_eq!(metadata.client.driver.name, "mrd");
 }
 
+#[test]
+fn server_selection_timeout_message() {
+    let _guard = LOCK.run_concurrently();
+
+    if !CLIENT.is_replica_set() {
+        return;
+    }
+
+    let mut tag_set = HashMap::new();
+    tag_set.insert("asdfasdf".to_string(), "asdfadsf".to_string());
+
+    let unsatisfiable_read_preference = ReadPreference::Secondary {
+        tag_sets: Some(vec![tag_set]),
+        max_staleness: None,
+    };
+
+    let mut options = CLIENT.options.clone();
+    options.server_selection_timeout = Some(Duration::from_millis(500));
+
+    let client = Client::with_options(options).unwrap();
+    let db = client.database("test");
+    let error = db
+        .run_command(
+            doc! { "isMaster": 1 },
+            SelectionCriteria::ReadPreference(unsatisfiable_read_preference),
+        )
+        .expect_err("should fail with server selection timeout error");
+
+    let error_description = format!("{}", error);
+    for host in CLIENT.options.hosts.iter() {
+        assert!(error_description.contains(format!("{}", host).as_str()));
+    }
+}
+
 #[test]
 #[function_name::named]
 fn list_databases() {

src/test/util/mod.rs

@@ -112,6 +112,10 @@ impl TestClient {
         self.options.credential.is_some()
     }
 
+    pub fn is_replica_set(&self) -> bool {
+        self.options.repl_set_name.is_some()
+    }
+
     #[allow(dead_code)]
     pub fn server_version_eq(&self, major: u64, minor: u64) -> bool {
         self.server_version.major == major && self.server_version.minor == minor