Skip to content

ConnectErrors improvements #197

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

Merged
merged 3 commits into from
May 27, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 23 additions & 21 deletions src/client/legacy/connect/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,12 +490,14 @@ fn get_host_port<'u>(config: &Config, dst: &'u Uri) -> Result<(&'u str, u16), Co
if dst.scheme() != Some(&Scheme::HTTP) {
return Err(ConnectError {
msg: INVALID_NOT_HTTP.into(),
addr: None,
cause: None,
});
}
} else if dst.scheme().is_none() {
return Err(ConnectError {
msg: INVALID_MISSING_SCHEME.into(),
addr: None,
cause: None,
});
}
Expand All @@ -505,6 +507,7 @@ fn get_host_port<'u>(config: &Config, dst: &'u Uri) -> Result<(&'u str, u16), Co
None => {
return Err(ConnectError {
msg: INVALID_MISSING_HOST.into(),
addr: None,
cause: None,
})
}
Expand Down Expand Up @@ -642,18 +645,19 @@ impl<R: Resolve> Future for HttpConnecting<R> {

// Not publicly exported (so missing_docs doesn't trigger).
pub struct ConnectError {
msg: Box<str>,
msg: &'static str,
addr: Option<SocketAddr>,
cause: Option<Box<dyn StdError + Send + Sync>>,
}

impl ConnectError {
fn new<S, E>(msg: S, cause: E) -> ConnectError
fn new<E>(msg: &'static str, cause: E) -> ConnectError
where
S: Into<Box<str>>,
E: Into<Box<dyn StdError + Send + Sync>>,
{
ConnectError {
msg: msg.into(),
msg,
addr: None,
cause: Some(cause.into()),
}
}
Expand All @@ -665,9 +669,8 @@ impl ConnectError {
ConnectError::new("dns error", cause)
}

fn m<S, E>(msg: S) -> impl FnOnce(E) -> ConnectError
fn m<E>(msg: &'static str) -> impl FnOnce(E) -> ConnectError
where
S: Into<Box<str>>,
E: Into<Box<dyn StdError + Send + Sync>>,
{
move |cause| ConnectError::new(msg, cause)
Expand All @@ -676,26 +679,21 @@ impl ConnectError {

impl fmt::Debug for ConnectError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let mut b = f.debug_tuple("ConnectError");
b.field(&self.msg);
if let Some(ref addr) = self.addr {
b.field(addr);
}
if let Some(ref cause) = self.cause {
f.debug_tuple("ConnectError")
.field(&self.msg)
.field(cause)
.finish()
} else {
self.msg.fmt(f)
b.field(cause);
}
b.finish()
}
}

impl fmt::Display for ConnectError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str(&self.msg)?;

if let Some(ref cause) = self.cause {
write!(f, ": {cause}")?;
}

Ok(())
f.write_str(&self.msg)
}
}

Expand Down Expand Up @@ -773,9 +771,13 @@ impl ConnectingTcpRemote {
debug!("connected to {}", addr);
return Ok(tcp);
}
Err(e) => {
Err(mut e) => {
trace!("connect error for {}: {:?}", addr, e);
err = Some(e);
e.addr = Some(addr);
// only return the first error, we assume it's the most relevant
if err.is_none() {
err = Some(e);
}
}
}
}
Expand Down