Skip to content

refactor(swarm)!: refactor ListenUpgradeError and DialUpgradeError #3307

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.

Sign up for GitHub

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

Closed
jxs wants to merge 18 commits into from
Closed

refactor(swarm)!: refactor ListenUpgradeError and DialUpgradeError #3307

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
e63d5c7
swarm: remove unconstructed Timer variant
jxs Jan 4, 2023
fbc6b82
swarm: make ListenUpgradeError::error an UpgradeError
jxs Jan 4, 2023
11db523
swarm: make DialUpgradeError::error an UpgradeError
jxs Jan 4, 2023
a793fb0
dcutr: update to libp2p-swarm ListenUpgradeError
jxs Jan 5, 2023
05c5fd0
identify: update to libp2p-swarm ListenUpgradeError
jxs Jan 5, 2023
6758ccd
relay: update to libp2p-swarm ListenUpgradeError
jxs Jan 5, 2023
24d4c9d
metrics: update to libp2p-swarm ListenUpgradeError
jxs Jan 5, 2023
fb51e22
kad: update to libp2p-swarm ListenUpgradeError
jxs Jan 5, 2023
a03e26c
rendezvous: update to libp2p-swarm ListenUpgradeError
jxs Jan 5, 2023
a403407
ping: update to libp2p-swarm ListenUpgradeError
jxs Jan 5, 2023
bc2e6bb
gossipsub: update to libp2p-swarm ListenUpgradeError
jxs Jan 5, 2023
4a0e5ee
request-response: update to libp2p-swarm ListenUpgradeError
jxs Jan 5, 2023
021502c
examples: update file-sharing example to libp2p-swarm
jxs Jan 5, 2023
ec1cb53
dcutr: report Timeout as a variant of Error
jxs Jan 9, 2023
1bc39e2
identify: introduce Error enum
jxs Jan 9, 2023
d70280a
metrics: fix removal of identify Event::Timeout
jxs Jan 9, 2023
3a0db97
relay: introduce behaviour::OutboundError enum
jxs Jan 10, 2023
c2eda37
relay: remove unneeded inbound_hop::UpgradeError.
jxs Jan 10, 2023
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
2 changes: 2 additions & 0 deletions swarm/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# 0.42.0 [unreleased]

- Remove uncontructed variant `Timer` from `ConnectionHandlerUpgrErr`.
Make `ListenUpgradeError::error` an `UpgradeError` instead of `ConnectionHandlerUpgrErr`, in case of timeout expiration we
just log the error as we don't know which ConnectionHandler should handle the error.

- Update to `libp2p-core` `v0.39.0`.

Expand Down
21 changes: 7 additions & 14 deletions swarm/src/behaviour/toggle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@

use crate::behaviour::FromSwarm;
use crate::handler::{
AddressChange, ConnectionEvent, ConnectionHandler, ConnectionHandlerEvent,
ConnectionHandlerUpgrErr, DialUpgradeError, FullyNegotiatedInbound, FullyNegotiatedOutbound,
IntoConnectionHandler, KeepAlive, ListenUpgradeError, SubstreamProtocol,
AddressChange, ConnectionEvent, ConnectionHandler, ConnectionHandlerEvent, DialUpgradeError,
FullyNegotiatedInbound, FullyNegotiatedOutbound, IntoConnectionHandler, KeepAlive,
ListenUpgradeError, SubstreamProtocol,
};
use crate::upgrade::SendWrapper;
use crate::{NetworkBehaviour, NetworkBehaviourAction, PollParameters};
Expand Down Expand Up @@ -210,19 +210,12 @@ where
),
};

let err = match err {
ConnectionHandlerUpgrErr::Timeout => ConnectionHandlerUpgrErr::Timeout,
ConnectionHandlerUpgrErr::Upgrade(err) => {
ConnectionHandlerUpgrErr::Upgrade(err.map_err(|err| match err {
EitherError::A(e) => e,
EitherError::B(v) => void::unreachable(v),
}))
}
};

inner.on_connection_event(ConnectionEvent::ListenUpgradeError(ListenUpgradeError {
info,
error: err,
error: err.map_err(|err| match err {
EitherError::A(e) => e,
EitherError::B(v) => void::unreachable(v),
}),
}));
}
}
Expand Down
17 changes: 11 additions & 6 deletions swarm/src/connection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,12 +239,17 @@ where
));
continue;
}
Poll::Ready(Some((info, Err(error)))) => {
handler.on_connection_event(ConnectionEvent::ListenUpgradeError(
ListenUpgradeError { info, error },
));
continue;
}
Poll::Ready(Some((info, Err(error)))) => match error {
ConnectionHandlerUpgrErr::Upgrade(error) => {
handler.on_connection_event(ConnectionEvent::ListenUpgradeError(
ListenUpgradeError { info, error },
));
continue;
}
ConnectionHandlerUpgrErr::Timeout => {
log::debug!("Timeout expired during an inbound substream negotiation")
Copy link
Contributor

@thomaseizinger thomaseizinger Jan 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
log::debug!("Timeout expired during an inbound substream negotiation")
log::debug!("Failed to negotiate protocol on inbound stream within {seconds}s")

That would be more useful IMO.

Copy link
Member Author

@jxs jxs Jan 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seconds doesn't seem to exist in this scope, we probably can make ConnectionHandlerUpgrErr::Timeout have it . I will implement after clarifying #3307 (comment)

Copy link
Contributor

@thomaseizinger thomaseizinger Jan 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seconds doesn't seem to exist in this scope, we probably can make ConnectionHandlerUpgrErr::Timeout have it . I will implement after clarifying #3307 (comment)

Yes, it was only meant to be illustrative on what I'd like to have printed!

}
},
}

// Ask the handler whether it wants the connection (and the handler itself)
Expand Down
2 changes: 1 addition & 1 deletion swarm/src/handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ pub struct DialUpgradeError<OOI, OP: OutboundUpgradeSend> {
/// that upgrading an inbound substream to the given protocol has failed.
pub struct ListenUpgradeError<IOI, IP: InboundUpgradeSend> {
pub info: IOI,
pub error: ConnectionHandlerUpgrErr<IP::Error>,
pub error: UpgradeError<IP::Error>,
}

/// Configuration of inbound or outbound substream protocol(s)
Expand Down
34 changes: 8 additions & 26 deletions swarm/src/handler/either.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,56 +197,38 @@ where
},
))),
ConnectionEvent::ListenUpgradeError(ListenUpgradeError {
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Apply(EitherError::A(error))),
info: Either::Left(info),
}) => Ok(Either::Left(ConnectionEvent::ListenUpgradeError(
ListenUpgradeError {
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Apply(error)),
info,
},
))),
ConnectionEvent::ListenUpgradeError(ListenUpgradeError {
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Apply(EitherError::B(error))),
info: Either::Right(info),
}) => Ok(Either::Right(ConnectionEvent::ListenUpgradeError(
ListenUpgradeError {
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Apply(error)),
info,
},
))),
ConnectionEvent::ListenUpgradeError(ListenUpgradeError {
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(error)),
error: UpgradeError::Apply(EitherError::A(error)),
info: Either::Left(info),
}) => Ok(Either::Left(ConnectionEvent::ListenUpgradeError(
ListenUpgradeError {
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(error)),
error: UpgradeError::Apply(error),
info,
},
))),
ConnectionEvent::ListenUpgradeError(ListenUpgradeError {
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(error)),
error: UpgradeError::Apply(EitherError::B(error)),
info: Either::Right(info),
}) => Ok(Either::Right(ConnectionEvent::ListenUpgradeError(
ListenUpgradeError {
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(error)),
error: UpgradeError::Apply(error),
info,
},
))),
ConnectionEvent::ListenUpgradeError(ListenUpgradeError {
error: ConnectionHandlerUpgrErr::Timeout,
error: UpgradeError::Select(error),
info: Either::Left(info),
}) => Ok(Either::Left(ConnectionEvent::ListenUpgradeError(
ListenUpgradeError {
error: ConnectionHandlerUpgrErr::Timeout,
error: UpgradeError::Select(error),
info,
},
))),
ConnectionEvent::ListenUpgradeError(ListenUpgradeError {
error: ConnectionHandlerUpgrErr::Timeout,
error: UpgradeError::Select(error),
info: Either::Right(info),
}) => Ok(Either::Right(ConnectionEvent::ListenUpgradeError(
ListenUpgradeError {
error: ConnectionHandlerUpgrErr::Timeout,
error: UpgradeError::Select(error),
info,
},
))),
Expand Down
48 changes: 12 additions & 36 deletions swarm/src/handler/multi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@
//! indexed by some key.

use crate::handler::{
AddressChange, ConnectionEvent, ConnectionHandler, ConnectionHandlerEvent,
ConnectionHandlerUpgrErr, DialUpgradeError, FullyNegotiatedInbound, FullyNegotiatedOutbound,
IntoConnectionHandler, KeepAlive, ListenUpgradeError, SubstreamProtocol,
AddressChange, ConnectionEvent, ConnectionHandler, ConnectionHandlerEvent, DialUpgradeError,
FullyNegotiatedInbound, FullyNegotiatedOutbound, IntoConnectionHandler, KeepAlive,
ListenUpgradeError, SubstreamProtocol,
};
use crate::upgrade::{InboundUpgradeSend, OutboundUpgradeSend, UpgradeInfoSend};
use crate::NegotiatedSubstream;
Expand Down Expand Up @@ -92,35 +92,19 @@ where
>,
) {
match error {
ConnectionHandlerUpgrErr::Timeout => {
UpgradeError::Select(NegotiationError::Failed) => {
for (k, h) in &mut self.handlers {
if let Some(i) = info.take(k) {
h.on_connection_event(ConnectionEvent::ListenUpgradeError(
ListenUpgradeError {
info: i,
error: ConnectionHandlerUpgrErr::Timeout,
error: UpgradeError::Select(NegotiationError::Failed),
},
));
}
}
}
ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(NegotiationError::Failed)) => {
for (k, h) in &mut self.handlers {
if let Some(i) = info.take(k) {
h.on_connection_event(ConnectionEvent::ListenUpgradeError(
ListenUpgradeError {
info: i,
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(
NegotiationError::Failed,
)),
},
));
}
}
}
ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(
NegotiationError::ProtocolError(e),
)) => match e {
UpgradeError::Select(NegotiationError::ProtocolError(e)) => match e {
ProtocolError::IoError(e) => {
for (k, h) in &mut self.handlers {
if let Some(i) = info.take(k) {
Expand All @@ -130,9 +114,7 @@ where
h.on_connection_event(ConnectionEvent::ListenUpgradeError(
ListenUpgradeError {
info: i,
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(
e,
)),
error: UpgradeError::Select(e),
},
));
}
Expand All @@ -145,9 +127,7 @@ where
h.on_connection_event(ConnectionEvent::ListenUpgradeError(
ListenUpgradeError {
info: i,
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(
e,
)),
error: UpgradeError::Select(e),
},
));
}
Expand All @@ -160,9 +140,7 @@ where
h.on_connection_event(ConnectionEvent::ListenUpgradeError(
ListenUpgradeError {
info: i,
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(
e,
)),
error: UpgradeError::Select(e),
},
));
}
Expand All @@ -176,22 +154,20 @@ where
h.on_connection_event(ConnectionEvent::ListenUpgradeError(
ListenUpgradeError {
info: i,
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(
e,
)),
error: UpgradeError::Select(e),
},
));
}
}
}
},
ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Apply((k, e))) => {
UpgradeError::Apply((k, e)) => {
if let Some(h) = self.handlers.get_mut(&k) {
if let Some(i) = info.take(&k) {
h.on_connection_event(ConnectionEvent::ListenUpgradeError(
ListenUpgradeError {
info: i,
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Apply(e)),
error: UpgradeError::Apply(e),
},
));
}
Expand Down
39 changes: 10 additions & 29 deletions swarm/src/handler/select.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,39 +226,20 @@ where
>,
) {
match error {
ConnectionHandlerUpgrErr::Timeout => {
UpgradeError::Select(NegotiationError::Failed) => {
self.proto1
.on_connection_event(ConnectionEvent::ListenUpgradeError(ListenUpgradeError {
info: i1,
error: ConnectionHandlerUpgrErr::Timeout,
error: UpgradeError::Select(NegotiationError::Failed),
}));

self.proto2
.on_connection_event(ConnectionEvent::ListenUpgradeError(ListenUpgradeError {
info: i2,
error: ConnectionHandlerUpgrErr::Timeout,
error: UpgradeError::Select(NegotiationError::Failed),
}));
}
ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(NegotiationError::Failed)) => {
self.proto1
.on_connection_event(ConnectionEvent::ListenUpgradeError(ListenUpgradeError {
info: i1,
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(
NegotiationError::Failed,
)),
}));

self.proto2
.on_connection_event(ConnectionEvent::ListenUpgradeError(ListenUpgradeError {
info: i2,
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(
NegotiationError::Failed,
)),
}));
}
ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(
NegotiationError::ProtocolError(e),
)) => {
UpgradeError::Select(NegotiationError::ProtocolError(e)) => {
let (e1, e2);
match e {
ProtocolError::IoError(e) => {
Expand All @@ -283,26 +264,26 @@ where
self.proto1
.on_connection_event(ConnectionEvent::ListenUpgradeError(ListenUpgradeError {
info: i1,
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(e1)),
error: UpgradeError::Select(e1),
}));
self.proto2
.on_connection_event(ConnectionEvent::ListenUpgradeError(ListenUpgradeError {
info: i2,
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Select(e2)),
error: UpgradeError::Select(e2),
}));
}
ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Apply(EitherError::A(e))) => {
UpgradeError::Apply(EitherError::A(e)) => {
self.proto1
.on_connection_event(ConnectionEvent::ListenUpgradeError(ListenUpgradeError {
info: i1,
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Apply(e)),
error: UpgradeError::Apply(e),
}));
}
ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Apply(EitherError::B(e))) => {
UpgradeError::Apply(EitherError::B(e)) => {
self.proto2
.on_connection_event(ConnectionEvent::ListenUpgradeError(ListenUpgradeError {
info: i2,
error: ConnectionHandlerUpgrErr::Upgrade(UpgradeError::Apply(e)),
error: UpgradeError::Apply(e),
}));
}
}
Expand Down