-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Include connection info in Client::send_request errors #2749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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()), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This also kind of reminds me of how in Finagle, the exceptions/failures there have "sources" and/or "remote info". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @seanmonstar, not familiar with finagle... Can you point to the library doc or impl. Also would be informative to know an example use you considered best practice or at least idiomatic. |
||
)); | ||
} | ||
|
||
|
@@ -281,18 +281,20 @@ where | |
authority_form(req.uri_mut()); | ||
} | ||
|
||
let fut = pooled | ||
.send_request_retryable(req) | ||
.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... | ||
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 [email protected], there is a race condition in the mpsc | ||
// channel, such that sending when the receiver is closing can | ||
|
@@ -302,11 +304,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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<Cause>, | ||
#[cfg(all(feature = "client", any(feature = "http1", feature = "http2")))] | ||
connect_info: Option<Connected>, | ||
} | ||
|
||
#[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> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I realize they're not implemented yet, but this made me think of the errors RFCs about "generic context" access, and would seem like a good fit eventually. rust-lang/rfcs#2895 and rust-lang/rfcs#3192 |
||
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 | ||
|
Uh oh!
There was an error while loading. Please reload this page.