From 6ca6da4add81eda0014be56bd210c5c3c1740bee Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 4 Feb 2022 11:06:55 +0100 Subject: [PATCH 1/2] chore(client): refactor the response handling in Client::send_request No need to map futures as we already await in that method anyway. --- src/client/client.rs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/client/client.rs b/src/client/client.rs index 9baead7349..160dd03d6d 100644 --- a/src/client/client.rs +++ b/src/client/client.rs @@ -281,18 +281,15 @@ where authority_form(req.uri_mut()); } - let fut = pooled + let mut res = pooled .send_request_retryable(req) - .map_err(ClientError::map_with_reused(pooled.is_reused())); + .await + .map_err(ClientError::map_with_reused(pooled.is_reused()))?; // If the Connector included 'extra' info, add to Response... - let extra_info = pooled.conn_info.extra.clone(); - let fut = fut.map_ok(move |mut res| { - if let Some(extra) = extra_info { - extra.set(res.extensions_mut()); - } - res - }); + if let Some(extra) = &pooled.conn_info.extra { + extra.set(res.extensions_mut()); + } // As of futures@0.1.21, there is a race condition in the mpsc // channel, such that sending when the receiver is closing can @@ -302,11 +299,9 @@ where // To counteract this, we must check if our senders 'want' channel // has been closed after having tried to send. If so, error out... if pooled.is_closed() { - return fut.await; + return Ok(res); } - let mut res = fut.await?; - // If pooled is HTTP/2, we can toss this reference immediately. // // when pooled is dropped, it will try to insert back into the From 351e0df6d8f45ad7e2bd220f4410b0cf20dcdbd0 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 4 Feb 2022 11:14:28 +0100 Subject: [PATCH 2/2] feat(client): include connection info in Client::send_request errors --- src/client/client.rs | 15 ++++++++++----- src/error.rs | 24 +++++++++++++++++++++++- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/client/client.rs b/src/client/client.rs index 160dd03d6d..bf4db79fde 100644 --- a/src/client/client.rs +++ b/src/client/client.rs @@ -251,7 +251,7 @@ where if req.version() == Version::HTTP_2 { warn!("Connection is HTTP/1, but request requires HTTP/2"); return Err(ClientError::Normal( - crate::Error::new_user_unsupported_version(), + crate::Error::new_user_unsupported_version().with_client_connect_info(pooled.conn_info.clone()), )); } @@ -281,10 +281,15 @@ where authority_form(req.uri_mut()); } - let mut res = pooled - .send_request_retryable(req) - .await - .map_err(ClientError::map_with_reused(pooled.is_reused()))?; + let mut res = match pooled.send_request_retryable(req).await { + Err((err, orig_req)) => { + return Err(ClientError::map_with_reused(pooled.is_reused())(( + err.with_client_connect_info(pooled.conn_info.clone()), + orig_req, + ))); + } + Ok(res) => res, + }; // If the Connector included 'extra' info, add to Response... if let Some(extra) = &pooled.conn_info.extra { diff --git a/src/error.rs b/src/error.rs index 468f24cb7a..5beedeb8b2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,4 +1,7 @@ //! Error and Result module. + +#[cfg(all(feature = "client", any(feature = "http1", feature = "http2")))] +use crate::client::connect::Connected; use std::error::Error as StdError; use std::fmt; @@ -15,6 +18,8 @@ pub struct Error { struct ErrorImpl { kind: Kind, cause: Option, + #[cfg(all(feature = "client", any(feature = "http1", feature = "http2")))] + connect_info: Option, } #[derive(Debug)] @@ -210,9 +215,20 @@ impl Error { self.inner.cause } + /// Returns the info of the client connection on which this error occurred. + #[cfg(all(feature = "client", any(feature = "http1", feature = "http2")))] + pub fn client_connect_info(&self) -> Option<&Connected> { + self.inner.connect_info.as_ref() + } + pub(super) fn new(kind: Kind) -> Error { Error { - inner: Box::new(ErrorImpl { kind, cause: None }), + inner: Box::new(ErrorImpl { + kind, + cause: None, + #[cfg(all(feature = "client", any(feature = "http1", feature = "http2")))] + connect_info: None, + }), } } @@ -221,6 +237,12 @@ impl Error { self } + #[cfg(all(feature = "client", any(feature = "http1", feature = "http2")))] + pub(super) fn with_client_connect_info(mut self, connect_info: Connected) -> Error { + self.inner.connect_info = Some(connect_info); + self + } + #[cfg(any(all(feature = "http1", feature = "server"), feature = "ffi"))] pub(super) fn kind(&self) -> &Kind { &self.inner.kind