From 931af0e865421f8fe97afefa62e02d69a97722bd Mon Sep 17 00:00:00 2001
From: Martin Algesten <martin@algesten.se>
Date: Sat, 8 Mar 2025 09:38:12 +0100
Subject: [PATCH] Doc and Error::Other for bespoke connector chains

---
 CHANGELOG.md                     |  2 ++
 src/error.rs                     | 27 ++++++++++++++++++++++++++-
 src/lib.rs                       |  5 +++++
 src/unversioned/transport/mod.rs |  9 +++++++++
 4 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 497b0a0c..d567b158 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,7 @@
 # Unreleased
 
+  * Improved errors and doc for bespoke transports (#1032)
+
 # 3.0.8
 
   * Fix incorrect parsing bug "missing http version" (#1026)
diff --git a/src/error.rs b/src/error.rs
index 19fa4a6a..ba7eabfe 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -49,6 +49,17 @@ pub enum Error {
     InvalidProxyUrl,
 
     /// A connection failed.
+    ///
+    /// This is a fallback error when there is no other explanation as to
+    /// why a connector chain didn't produce a connection. The idea is that the connector
+    /// chain would return some other [`Error`] rather than rely om this value.
+    ///
+    /// Typically bespoke connector chains should, as far as possible, map their underlying
+    /// errors to [`Error::Io`] and use the [`io::ErrorKind`] to provide a reason.
+    ///
+    /// A bespoke chain is allowed to map to this value, but that provides very little
+    /// information to the user as to why the connection failed. One way to mitigate that
+    /// would be to rely on the `log` crate to provide additional information.
     ConnectionFailed,
 
     /// A send body (Such as `&str`) is larger than the `content-length` header.
@@ -148,7 +159,20 @@ pub enum Error {
     /// This typically indicates a fault in bespoke `Connector` chains.
     TlsRequired,
 
-    /// hoot made no progress and there is no more input to read.
+    /// Some other error occured.
+    ///
+    /// This is an escape hatch for bespoke connector chains having errors that don't naturally
+    /// map to any other error. For connector chains we recommend:
+    ///
+    /// 1. Map to [`Error::Io`] as far as possible.
+    /// 2. Map to other [`Error`] where reasonable.
+    /// 3. Fall back on [`Error::Other`].
+    /// 4. As a last resort [`Error::ConnectionFailed`].
+    ///
+    /// ureq does not produce this error using the default connectors.
+    Other(Box<dyn std::error::Error + Send + Sync>),
+
+    /// ureq-proto made no progress and there is no more input to read.
     ///
     /// We should never see this value.
     #[doc(hidden)]
@@ -237,6 +261,7 @@ impl fmt::Display for Error {
             Error::Json(v) => write!(f, "json: {}", v),
             Error::ConnectProxyFailed(v) => write!(f, "CONNECT proxy failed: {}", v),
             Error::TlsRequired => write!(f, "TLS required, but transport is unsecured"),
+            Error::Other(v) => write!(f, "other: {}", v),
             Error::BodyStalled => write!(f, "body data reading stalled"),
         }
     }
diff --git a/src/lib.rs b/src/lib.rs
index 449898b0..1ca81b37 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1207,5 +1207,10 @@ pub(crate) mod test {
         let response = post("https://yaz").send(&data).unwrap();
         let owned_reader = response.into_parts().1.into_reader();
         is_sync(owned_reader);
+
+        let err = Error::HostNotFound;
+        is_send(err);
+        let err = Error::HostNotFound;
+        is_sync(err);
     }
 }
diff --git a/src/unversioned/transport/mod.rs b/src/unversioned/transport/mod.rs
index d584a0b2..6137a6f4 100644
--- a/src/unversioned/transport/mod.rs
+++ b/src/unversioned/transport/mod.rs
@@ -83,6 +83,15 @@ pub use crate::timings::NextTimeout;
 ///
 /// The built-in [`DefaultConnector`] provides SOCKS, TCP sockets and TLS wrapping.
 ///
+/// # Errors
+///
+/// When writing a bespoke connector chain we recommend handling errors like this:
+///
+/// 1. Map to [`Error::Io`] as far as possible.
+/// 2. Map to any other [`Error`] where reasonable.
+/// 3. Fall back on [`Error::Other`] preserving the original error.
+/// 4. As a last resort [`Error::ConnectionFailed`] + logging.
+///
 /// # Example
 ///
 /// ```