Expose connection info in errors (#337)

Add `local_addr` and `remote_addr` to `Error` which expose the local and remote network addresses of the last used connection for a request before the error occurred, if known.

Fixes #336.

src/error.rs

@@ -1,6 +1,11 @@
 //! Types for error handling.
 
-use std::{error::Error as StdError, fmt, io, sync::Arc};
+use std::{error::Error as StdError, fmt, io, net::SocketAddr, sync::Arc};
+
+use http::Response;
+use once_cell::sync::OnceCell;
+
+use crate::ResponseExt;
 
 /// A non-exhaustive list of error types that can occur while sending an HTTP
 /// request or receiving an HTTP response.
@@ -165,6 +170,8 @@ struct Inner {
     kind: ErrorKind,
     context: Option<String>,
     source: Option<Box<dyn SourceError>>,
+    local_addr: OnceCell<SocketAddr>,
+    remote_addr: OnceCell<SocketAddr>,
 }
 
 impl Error {
@@ -186,9 +193,26 @@ impl Error {
             kind,
             context,
             source: Some(Box::new(source)),
+            local_addr: OnceCell::new(),
+            remote_addr: OnceCell::new(),
         }))
     }
 
+    /// Create a new error from a given error kind and response.
+    pub(crate) fn with_response<B>(kind: ErrorKind, response: &Response<B>) -> Self {
+        let error = Self::from(kind);
+
+        if let Some(addr) = response.local_addr() {
+            let _ = error.0.local_addr.set(addr);
+        }
+
+        if let Some(addr) = response.remote_addr() {
+            let _ = error.0.remote_addr.set(addr);
+        }
+
+        error
+    }
+
     /// Statically cast a given error into an Isahc error, converting if
     /// necessary.
     ///
@@ -346,6 +370,34 @@ impl Error {
             _ => false,
         }
     }
+
+    /// Get the local socket address of the last-used connection involved in
+    /// this error, if known.
+    ///
+    /// If the request that caused this error failed to create a local socket
+    /// for connecting then this will return `None`.
+    pub fn local_addr(&self) -> Option<SocketAddr> {
+        self.0.local_addr.get().cloned()
+    }
+
+    /// Get the remote socket address of the last-used connection involved in
+    /// this error, if known.
+    ///
+    /// If the request that caused this error failed to connect to any server,
+    /// then this will return `None`.
+    pub fn remote_addr(&self) -> Option<SocketAddr> {
+        self.0.remote_addr.get().cloned()
+    }
+
+    pub(crate) fn with_local_addr(self, addr: SocketAddr) -> Self {
+        let _ = self.0.local_addr.set(addr);
+        self
+    }
+
+    pub(crate) fn with_remote_addr(self, addr: SocketAddr) -> Self {
+        let _ = self.0.remote_addr.set(addr);
+        self
+    }
 }
 
 impl StdError for Error {
@@ -370,6 +422,8 @@ impl fmt::Debug for Error {
                 "source_type",
                 &self.0.source.as_ref().map(|e| e.type_name()),
             )
+            .field("local_addr", &self.0.local_addr.get())
+            .field("remote_addr", &self.0.remote_addr.get())
             .finish()
     }
 }
@@ -390,6 +444,8 @@ impl From<ErrorKind> for Error {
             kind,
             context: None,
             source: None,
+            local_addr: OnceCell::new(),
+            remote_addr: OnceCell::new(),
         }))
     }
 }

src/handler.rs

@@ -214,6 +214,18 @@ impl RequestHandler {
 
     /// Set the final result for this transfer.
     pub(crate) fn set_result(&mut self, result: Result<(), Error>) {
+        let result = result.map_err(|mut e| {
+            if let Some(addr) = self.get_local_addr() {
+                e = e.with_local_addr(addr);
+            }
+
+            if let Some(addr) = self.get_primary_addr() {
+                e = e.with_remote_addr(addr);
+            }
+
+            e
+        });
+
         if self.shared.result.set(result).is_err() {
             tracing::debug!("attempted to set error multiple times");
         }

src/redirect.rs

@@ -77,7 +77,7 @@ impl Interceptor for RedirectInterceptor {
                 if let Some(location) = get_redirect_location(&effective_uri, &response) {
                     // If we've reached the limit, return an error as requested.
                     if redirect_count >= limit {
-                        return Err(ErrorKind::TooManyRedirects.into());
+                        return Err(Error::with_response(ErrorKind::TooManyRedirects, &response));
                     }
 
                     // Set referer header.
@@ -113,7 +113,10 @@ impl Interceptor for RedirectInterceptor {
                     // There's not really a good way of handling this gracefully, so
                     // we just return an error so that the user knows about it.
                     if !request_body.reset() {
-                        return Err(ErrorKind::RequestBodyNotRewindable.into());
+                        return Err(Error::with_response(
+                            ErrorKind::RequestBodyNotRewindable,
+                            &response,
+                        ));
                     }
 
                     // Update the request to point to the new URI.

src/response.rs

@@ -274,7 +274,7 @@ impl<R: Read> ReadResponseExt<R> for Response<R> {
 
     #[cfg(feature = "text-decoding")]
     fn text(&mut self) -> io::Result<String> {
-        crate::text::Decoder::for_response(&self).decode_reader(self.body_mut())
+        crate::text::Decoder::for_response(self).decode_reader(self.body_mut())
     }
 
     #[cfg(feature = "json")]
@@ -430,7 +430,7 @@ impl<R: AsyncRead + Unpin> AsyncReadResponseExt<R> for Response<R> {
 
     #[cfg(feature = "text-decoding")]
     fn text(&mut self) -> crate::text::TextFuture<'_, &mut R> {
-        crate::text::Decoder::for_response(&self).decode_reader_async(self.body_mut())
+        crate::text::Decoder::for_response(self).decode_reader_async(self.body_mut())
     }
 
     #[cfg(feature = "json")]

tests/net.rs

@@ -34,7 +34,7 @@ fn local_addr_returns_expected_address() {
 }
 
 #[test]
-fn remote_addr_returns_expected_address_expected_address() {
+fn remote_addr_returns_expected_address() {
     let m = mock!();
 
     let response = isahc::get(m.url()).unwrap();
@@ -43,6 +43,24 @@ fn remote_addr_returns_expected_address_expected_address() {
     assert_eq!(response.remote_addr(), Some(m.addr()));
 }
 
+#[test]
+fn local_and_remote_addr_returns_expected_addresses_on_error() {
+    let server = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).unwrap();
+    let addr = server.local_addr().unwrap();
+
+    thread::spawn(move || {
+        let (mut client, _) = server.accept().unwrap();
+        client.write_all(b"foobar").unwrap();
+        client.flush().unwrap();
+    });
+
+    let error = isahc::get(format!("http://localhost:{}", addr.port())).unwrap_err();
+
+    assert_eq!(error.remote_addr(), Some(addr));
+    assert_eq!(error.local_addr().unwrap().ip(), Ipv4Addr::LOCALHOST);
+    assert!(error.local_addr().unwrap().port() > 0);
+}
+
 #[test]
 fn ipv4_only_will_not_connect_to_ipv6() {
     if !is_ipv6_supported() {

tests/redirects.rs

@@ -2,9 +2,6 @@ use isahc::{config::RedirectPolicy, prelude::*, Body, HttpClient, Request};
 use test_case::test_case;
 use testserver::mock;
 
-#[macro_use]
-mod utils;
-
 #[test]
 fn response_301_no_follow() {
     let m = mock! {
@@ -216,13 +213,15 @@ fn redirect_non_rewindable_body_returns_error() {
     // Create a streaming body of unknown size.
     let upload_stream = Body::from_reader(Body::from_bytes_static(b"hello world"));
 
-    let result = Request::post(m1.url())
+    let error = Request::post(m1.url())
         .redirect_policy(RedirectPolicy::Follow)
         .body(upload_stream)
         .unwrap()
-        .send();
+        .send()
+        .unwrap_err();
 
-    assert_matches!(result, Err(e) if e == isahc::error::ErrorKind::RequestBodyNotRewindable);
+    assert_eq!(error, isahc::error::ErrorKind::RequestBodyNotRewindable);
+    assert_eq!(error.remote_addr(), Some(m1.addr()));
     assert_eq!(m1.request().method, "POST");
 }
 
@@ -235,14 +234,16 @@ fn redirect_limit_is_respected() {
         }
     };
 
-    let result = Request::get(m.url())
+    let error = Request::get(m.url())
         .redirect_policy(RedirectPolicy::Limit(5))
         .body(())
         .unwrap()
-        .send();
+        .send()
+        .unwrap_err();
 
     // Request should error with too many redirects.
-    assert_matches!(result, Err(e) if e == isahc::error::ErrorKind::TooManyRedirects);
+    assert_eq!(error, isahc::error::ErrorKind::TooManyRedirects);
+    assert_eq!(error.remote_addr(), Some(m.addr()));
 
     // After request (limit + 1) that returns a redirect should error.
     assert_eq!(m.requests().len(), 6);