RUST-739 Stop re-exporting unstable types in ErrorKind

This also consolidates various redundant error cases.

Author: patrickfreed

src/client/options/mod.rs

@@ -149,22 +149,28 @@ impl StreamAddress {
         let hostname = match parts.next() {
             Some(part) => part,
             None => {
-                return Err(ErrorKind::InvalidHostname {
-                    hostname: address.to_string(),
+                return Err(ErrorKind::ArgumentError {
+                    message: format!("invalid server address: \"{}\"", address),
                 }
                 .into())
             }
         };
 
         let port = match parts.next() {
             Some(part) => {
-                let port = u16::from_str(part).map_err(|_| ErrorKind::InvalidHostname {
-                    hostname: address.to_string(),
+                let port = u16::from_str(part).map_err(|_| ErrorKind::ArgumentError {
+                    message: format!(
+                        "port must be valid 16-bit unsigned integer, instead got: {}",
+                        part
+                    ),
                 })?;
 
                 if parts.next().is_some() {
-                    return Err(ErrorKind::InvalidHostname {
-                        hostname: address.to_string(),
+                    return Err(ErrorKind::ArgumentError {
+                        message: format!(
+                            "address \"{}\" contains more than one unescaped ':'",
+                            address
+                        ),
                     }
                     .into());
                 }
@@ -592,9 +598,8 @@ impl TlsOptions {
         if let Some(path) = self.ca_file_path {
             store
                 .add_pem_file(&mut BufReader::new(File::open(&path)?))
-                .map_err(|_| ErrorKind::ParseError {
-                    data_type: "PEM-encoded root certificate".to_string(),
-                    file_path: path,
+                .map_err(|_| ErrorKind::TlsConfigError {
+                    message: format!("Unable to parse PEM-encoded root certificate from {}", path),
                 })?;
         } else {
             store.add_server_trust_anchors(&TLS_SERVER_ROOTS);
@@ -607,9 +612,11 @@ impl TlsOptions {
             let certs = match pemfile::certs(&mut file) {
                 Ok(certs) => certs,
                 Err(()) => {
-                    return Err(ErrorKind::ParseError {
-                        data_type: "PEM-encoded client certificate".to_string(),
-                        file_path: path,
+                    return Err(ErrorKind::TlsConfigError {
+                        message: format!(
+                            "Unable to parse PEM-encoded client certificate from {}",
+                            path
+                        ),
                     }
                     .into())
                 }
@@ -619,16 +626,19 @@ impl TlsOptions {
             let key = match pemfile::rsa_private_keys(&mut file) {
                 Ok(key) => key,
                 Err(()) => {
-                    return Err(ErrorKind::ParseError {
-                        data_type: "PEM-encoded RSA key".to_string(),
-                        file_path: path,
+                    return Err(ErrorKind::TlsConfigError {
+                        message: format!("Unable to parse PEM-encoded RSA key from {}", path),
                     }
                     .into())
                 }
             };
 
             // TODO: Get rid of unwrap.
-            config.set_single_client_cert(certs, key.into_iter().next().unwrap())?;
+            config
+                .set_single_client_cert(certs, key.into_iter().next().unwrap())
+                .map_err(|e| ErrorKind::TlsConfigError {
+                    message: e.to_string(),
+                })?;
         }
 
         Ok(config)

src/cmap/conn/mod.rs

@@ -256,7 +256,7 @@ impl Connection {
     /// Gets the connection's StreamDescription.
     pub(crate) fn stream_description(&self) -> Result<&StreamDescription> {
         self.stream_description.as_ref().ok_or_else(|| {
-            ErrorKind::OperationError {
+            ErrorKind::InternalError {
                 message: "Stream checked out but not handshaked".to_string(),
             }
             .into()

src/cmap/conn/wire/header.rs

@@ -18,7 +18,7 @@ impl OpCode {
             1 => Ok(OpCode::Reply),
             2004 => Ok(OpCode::Query),
             2013 => Ok(OpCode::Message),
-            other => Err(ErrorKind::OperationError {
+            other => Err(ErrorKind::ResponseError {
                 message: format!("Invalid wire protocol opcode: {}", other),
             }
             .into()),

src/cmap/conn/wire/message.rs

@@ -90,7 +90,7 @@ impl Message {
         if length_remaining == 4 && flags.contains(MessageFlags::CHECKSUM_PRESENT) {
             checksum = Some(reader.read_u32().await?);
         } else if length_remaining != 0 {
-            return Err(ErrorKind::OperationError {
+            return Err(ErrorKind::ResponseError {
                 message: format!(
                     "The server indicated that the reply would be {} bytes long, but it instead \
                      was {}",
@@ -193,7 +193,7 @@ impl MessageSection {
         }
 
         if length_remaining != count_reader.bytes_read() as i32 {
-            return Err(ErrorKind::OperationError {
+            return Err(ErrorKind::ResponseError {
                 message: format!(
                     "The server indicated that the reply would be {} bytes long, but it instead \
                      was {}",

src/cmap/establish/handshake/mod.rs

@@ -278,7 +278,7 @@ impl From<ClientOptions> for HandshakerOptions {
 /// If the given command is not an isMaster, this function will return an error.
 pub(crate) async fn is_master(command: Command, conn: &mut Connection) -> Result<IsMasterReply> {
     if !command.name.eq_ignore_ascii_case("ismaster") {
-        return Err(ErrorKind::OperationError {
+        return Err(ErrorKind::InternalError {
             message: format!("invalid ismaster command: {}", command.name),
         }
         .into());

src/error.rs

@@ -189,6 +189,10 @@ impl Error {
             }
         }
     }
+
+    pub(crate) fn from_resolve_error(error: trust_dns_resolver::error::ResolveError) -> Self {
+        ErrorKind::DnsResolveError { message: error.to_string() }.into()
+    }
 }
 
 impl<E> From<E> for Error
@@ -240,12 +244,8 @@ impl std::ops::Deref for Error {
 #[derive(Clone, Debug, Error)]
 #[non_exhaustive]
 pub enum ErrorKind {
-    /// Wrapper around [`std::net::AddrParseError`](https://doc.rust-lang.org/std/net/struct.AddrParseError.html).
-    #[error("{0}")]
-    AddrParse(#[from] std::net::AddrParseError),
-
-    /// An invalid argument was provided to a database operation.
-    #[error("An invalid argument was provided to a database operation: {message}")]
+    /// An invalid argument was provided.
+    #[error("An invalid argument was provided: {message}")]
     #[non_exhaustive]
     ArgumentError { message: String },
 
@@ -271,45 +271,19 @@ pub enum ErrorKind {
     #[error("Command failed {0}")]
     CommandError(CommandError),
 
-    // `trust_dns` does not implement the `Error` trait on their errors, so we have to manually
-    // implement `From` rather than using the `source annotation.
-    /// Wrapper around `trust_dns_resolver::error::ResolveError`.
-    #[error("{0}")]
-    DnsResolve(trust_dns_resolver::error::ResolveError),
+    /// An error occurred during DNS resolution.
+    #[error("An error occurred during DNS resolution: {message}")]
+    #[non_exhaustive]
+    DnsResolveError { message: String },
 
     #[error("Internal error: {message}")]
     #[non_exhaustive]
     InternalError { message: String },
 
-    /// Wrapper around `webpki::InvalidDNSNameError`.
-    #[error("{0}")]
-    InvalidDnsName(#[from] webpki::InvalidDNSNameError),
-
-    /// A hostname could not be parsed.
-    #[error("Unable to parse hostname: {hostname}")]
-    #[non_exhaustive]
-    InvalidHostname { hostname: String },
-
     /// Wrapper around [`std::io::Error`](https://doc.rust-lang.org/std/io/struct.Error.html).
     #[error("{0}")]
     Io(Arc<std::io::Error>),
 
-    #[error("No DNS results for domain {0}")]
-    NoDnsResults(StreamAddress),
-
-    /// A database operation failed to send or receive a reply.
-    #[error("A database operation failed to send or receive a reply: {message}")]
-    #[non_exhaustive]
-    OperationError { message: String },
-
-    /// Data from a file could not be parsed.
-    #[error("Unable to parse {data_type} data from {file_path}")]
-    #[non_exhaustive]
-    ParseError {
-        data_type: String,
-        file_path: String,
-    },
-
     /// The connection pool for a server was cleared during operation execution due to
     /// a concurrent error, causing the operation to fail.
     #[error("{message}")]
@@ -326,22 +300,13 @@ pub enum ErrorKind {
     #[non_exhaustive]
     ServerSelectionError { message: String },
 
-    /// An error occurred during SRV record lookup.
-    #[error("An error occurred during SRV record lookup: {message}")]
-    #[non_exhaustive]
-    SrvLookupError { message: String },
-
     /// The Client does not support sessions.
     #[error("Attempted to start a session on a deployment that does not support sessions")]
     SessionsNotSupported,
 
-    #[error("{0}")]
-    RustlsConfig(#[from] rustls::TLSError),
-
-    /// An error occurred during TXT record lookup
-    #[error("An error occurred during TXT record lookup: {message}")]
+    #[error("{message}")]
     #[non_exhaustive]
-    TxtLookupError { message: String },
+    TlsConfigError { message: String },
 
     /// The Client timed out while checking out a connection from connection pool.
     #[error(
@@ -355,12 +320,6 @@ pub enum ErrorKind {
     WriteError(WriteFailure),
 }
 
-impl From<trust_dns_resolver::error::ResolveError> for ErrorKind {
-    fn from(error: trust_dns_resolver::error::ResolveError) -> Self {
-        Self::DnsResolve(error)
-    }
-}
-
 impl ErrorKind {
     pub(crate) fn is_non_timeout_network_error(&self) -> bool {
         matches!(self, ErrorKind::Io(ref io_err) if io_err.kind() != std::io::ErrorKind::TimedOut)

src/operation/find_and_modify/mod.rs

@@ -103,24 +103,13 @@ where
     const NAME: &'static str = "findAndModify";
 
     fn build(&self, description: &StreamDescription) -> Result<Command> {
-        if self.options.hint.is_some() {
-            match description.max_wire_version {
-                Some(version) if version < 8 => {
-                    return Err(ErrorKind::OperationError {
-                        message: "Specifying a hint is not supported on server versions < 4.4"
-                            .to_string(),
-                    }
-                    .into());
-                }
-                None => {
-                    return Err(ErrorKind::OperationError {
-                        message: "Specifying a hint is not supported on server versions < 4.4"
-                            .to_string(),
-                    }
-                    .into());
-                }
-                _ => {}
+        if self.options.hint.is_some() && description.max_wire_version.unwrap_or(0) < 8 {
+            return Err(ErrorKind::ArgumentError {
+                message: "Specifying a hint to find_one_and_x is not supported on server versions \
+                          < 4.4"
+                    .to_string(),
             }
+            .into());
         }
 
         let mut body: Document = doc! {

src/operation/mod.rs

@@ -107,7 +107,10 @@ pub(crate) trait Operation {
 
 /// Appends a serializable struct to the input document.
 /// The serializable struct MUST serialize to a Document, otherwise an error will be thrown.
-pub(crate) fn append_options<T: Serialize>(doc: &mut Document, options: Option<&T>) -> Result<()> {
+pub(crate) fn append_options<T: Serialize + Debug>(
+    doc: &mut Document,
+    options: Option<&T>,
+) -> Result<()> {
     match options {
         Some(options) => {
             let temp_doc = bson::to_bson(options)?;
@@ -116,8 +119,8 @@ pub(crate) fn append_options<T: Serialize>(doc: &mut Document, options: Option<&
                     doc.extend(d);
                     Ok(())
                 }
-                _ => Err(ErrorKind::OperationError {
-                    message: "options did not serialize to a Document".to_string(),
+                _ => Err(ErrorKind::InternalError {
+                    message: format!("options did not serialize to a Document: {:?}", options),
                 }
                 .into()),
             }

src/runtime/resolver.rs

@@ -1,10 +1,11 @@
 use trust_dns_resolver::{
     config::ResolverConfig,
+    error::ResolveErrorKind,
     lookup::{SrvLookup, TxtLookup},
     IntoName,
 };
 
-use crate::error::Result;
+use crate::error::{Error, Result};
 
 /// An async runtime agnostic DNS resolver.
 pub(crate) struct AsyncResolver {
@@ -20,15 +21,21 @@ impl AsyncResolver {
         #[cfg(feature = "tokio-runtime")]
         let resolver = match config {
             Some(config) => {
-                trust_dns_resolver::TokioAsyncResolver::tokio(config, Default::default())?
+                trust_dns_resolver::TokioAsyncResolver::tokio(config, Default::default())
+                    .map_err(Error::from_resolve_error)?
             }
-            None => trust_dns_resolver::TokioAsyncResolver::tokio_from_system_conf()?,
+            None => trust_dns_resolver::TokioAsyncResolver::tokio_from_system_conf()
+                .map_err(Error::from_resolve_error)?,
         };
 
         #[cfg(feature = "async-std-runtime")]
         let resolver = match config {
-            Some(config) => async_std_resolver::resolver(config, Default::default()).await?,
-            None => async_std_resolver::resolver_from_system_conf().await?,
+            Some(config) => async_std_resolver::resolver(config, Default::default())
+                .await
+                .map_err(Error::from_resolve_error)?,
+            None => async_std_resolver::resolver_from_system_conf()
+                .await
+                .map_err(Error::from_resolve_error)?,
         };
 
         Ok(Self { resolver })
@@ -37,12 +44,22 @@ impl AsyncResolver {
 
 impl AsyncResolver {
     pub async fn srv_lookup<N: IntoName>(&self, query: N) -> Result<SrvLookup> {
-        let lookup = self.resolver.srv_lookup(query).await?;
+        let lookup = self
+            .resolver
+            .srv_lookup(query)
+            .await
+            .map_err(Error::from_resolve_error)?;
         Ok(lookup)
     }
 
-    pub async fn txt_lookup<N: IntoName>(&self, query: N) -> Result<TxtLookup> {
-        let lookup = self.resolver.txt_lookup(query).await?;
-        Ok(lookup)
+    pub async fn txt_lookup<N: IntoName>(&self, query: N) -> Result<Option<TxtLookup>> {
+        let lookup_result = self.resolver.txt_lookup(query).await;
+        match lookup_result {
+            Ok(lookup) => Ok(Some(lookup)),
+            Err(e) => match e.kind() {
+                ResolveErrorKind::NoRecordsFound { .. } => Ok(None),
+                _ => Err(Error::from_resolve_error(e)),
+            },
+        }
     }
 }