From 02fa7c8bb329d0a10411280bc01b039c52a2944d Mon Sep 17 00:00:00 2001 From: Darius Clark Date: Tue, 9 Jul 2024 13:41:21 -0400 Subject: [PATCH 1/4] feat: Add `ConnectionError` to `FromSwarm::ConnectionClosed` Resolves #5484. Pull-Request: #5485. --- Cargo.lock | 4 ++-- Cargo.toml | 4 ++-- protocols/gossipsub/CHANGELOG.md | 4 ++++ protocols/gossipsub/Cargo.toml | 2 +- protocols/gossipsub/src/behaviour/tests.rs | 1 + protocols/perf/CHANGELOG.md | 4 ++++ protocols/perf/Cargo.toml | 2 +- protocols/perf/src/client/behaviour.rs | 1 + swarm/CHANGELOG.md | 2 ++ swarm/src/behaviour.rs | 5 +++-- swarm/src/lib.rs | 1 + swarm/src/test.rs | 2 ++ 12 files changed, 24 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6c64ea3fea7..ec114491e48 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2801,7 +2801,7 @@ dependencies = [ [[package]] name = "libp2p-gossipsub" -version = "0.46.2" +version = "0.47.0" dependencies = [ "async-std", "asynchronous-codec", @@ -3051,7 +3051,7 @@ dependencies = [ [[package]] name = "libp2p-perf" -version = "0.3.1" +version = "0.4.0" dependencies = [ "anyhow", "clap", diff --git a/Cargo.toml b/Cargo.toml index ab660cc90e9..7fa6856c26b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -82,7 +82,7 @@ libp2p-core = { version = "0.41.3", path = "core" } libp2p-dcutr = { version = "0.11.1", path = "protocols/dcutr" } libp2p-dns = { version = "0.41.1", path = "transports/dns" } libp2p-floodsub = { version = "0.44.0", path = "protocols/floodsub" } -libp2p-gossipsub = { version = "0.46.2", path = "protocols/gossipsub" } +libp2p-gossipsub = { version = "0.47.0", path = "protocols/gossipsub" } libp2p-identify = { version = "0.45.0", path = "protocols/identify" } libp2p-identity = { version = "0.2.9" } libp2p-kad = { version = "0.46.0", path = "protocols/kad" } @@ -92,7 +92,7 @@ libp2p-metrics = { version = "0.14.2", path = "misc/metrics" } libp2p-mplex = { version = "0.41.0", path = "muxers/mplex" } libp2p-muxer-test-harness = { path = "muxers/test-harness" } libp2p-noise = { version = "0.44.0", path = "transports/noise" } -libp2p-perf = { version = "0.3.1", path = "protocols/perf" } +libp2p-perf = { version = "0.4.0", path = "protocols/perf" } libp2p-ping = { version = "0.44.2", path = "protocols/ping" } libp2p-plaintext = { version = "0.41.0", path = "transports/plaintext" } libp2p-pnet = { version = "0.24.0", path = "transports/pnet" } diff --git a/protocols/gossipsub/CHANGELOG.md b/protocols/gossipsub/CHANGELOG.md index 970db3f1ec3..7c43b98f0a7 100644 --- a/protocols/gossipsub/CHANGELOG.md +++ b/protocols/gossipsub/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.47.0 +- Add ConnectionError to FromSwarm::ConnectionClosed. + See [PR 5485](https://github.com/libp2p/rust-libp2p/pull/5485). + ## 0.46.2 - Use `web-time` instead of `instant`. See [PR 5347](https://github.com/libp2p/rust-libp2p/pull/5347). diff --git a/protocols/gossipsub/Cargo.toml b/protocols/gossipsub/Cargo.toml index f556443477f..f989e997bfb 100644 --- a/protocols/gossipsub/Cargo.toml +++ b/protocols/gossipsub/Cargo.toml @@ -3,7 +3,7 @@ name = "libp2p-gossipsub" edition = "2021" rust-version = { workspace = true } description = "Gossipsub protocol for libp2p" -version = "0.46.2" +version = "0.47.0" authors = ["Age Manning "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" diff --git a/protocols/gossipsub/src/behaviour/tests.rs b/protocols/gossipsub/src/behaviour/tests.rs index 6cad719b5ab..fe861a674dd 100644 --- a/protocols/gossipsub/src/behaviour/tests.rs +++ b/protocols/gossipsub/src/behaviour/tests.rs @@ -268,6 +268,7 @@ where connection_id, endpoint: &fake_endpoint, remaining_established: active_connections, + cause: None, })); } } diff --git a/protocols/perf/CHANGELOG.md b/protocols/perf/CHANGELOG.md index d83e8b48472..b347f21e9e0 100644 --- a/protocols/perf/CHANGELOG.md +++ b/protocols/perf/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.4.0 +- Add ConnectionError to FromSwarm::ConnectionClosed. + See [PR 5485](https://github.com/libp2p/rust-libp2p/pull/5485). + ## 0.3.1 - Use `web-time` instead of `instant`. See [PR 5347](https://github.com/libp2p/rust-libp2p/pull/5347). diff --git a/protocols/perf/Cargo.toml b/protocols/perf/Cargo.toml index 4f154ab1b08..abe58088caa 100644 --- a/protocols/perf/Cargo.toml +++ b/protocols/perf/Cargo.toml @@ -3,7 +3,7 @@ name = "libp2p-perf" edition = "2021" rust-version = { workspace = true } description = "libp2p perf protocol implementation" -version = "0.3.1" +version = "0.4.0" authors = ["Max Inden "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" diff --git a/protocols/perf/src/client/behaviour.rs b/protocols/perf/src/client/behaviour.rs index 880bcdd9c83..5e430f8f0c1 100644 --- a/protocols/perf/src/client/behaviour.rs +++ b/protocols/perf/src/client/behaviour.rs @@ -116,6 +116,7 @@ impl NetworkBehaviour for Behaviour { connection_id: _, endpoint: _, remaining_established, + .. }) => { if remaining_established == 0 { assert!(self.connected.remove(&peer_id)); diff --git a/swarm/CHANGELOG.md b/swarm/CHANGELOG.md index bdb0b1cd5d0..f4901947b8b 100644 --- a/swarm/CHANGELOG.md +++ b/swarm/CHANGELOG.md @@ -10,6 +10,8 @@ See [PR 5347](https://github.com/libp2p/rust-libp2p/pull/5347). - Add `#[track_caller]` on all `spawn` wrappers. See [PR 5465](https://github.com/libp2p/rust-libp2p/pull/5465). +- Add ConnectionError to FromSwarm::ConnectionClosed. + See [PR 5485](https://github.com/libp2p/rust-libp2p/pull/5485). ## 0.44.2 diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index fc9045dfc3f..8a8418739c8 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -32,8 +32,8 @@ use crate::connection::ConnectionId; use crate::dial_opts::DialOpts; use crate::listen_opts::ListenOpts; use crate::{ - ConnectionDenied, ConnectionHandler, DialError, ListenError, THandler, THandlerInEvent, - THandlerOutEvent, + ConnectionDenied, ConnectionError, ConnectionHandler, DialError, ListenError, THandler, + THandlerInEvent, THandlerOutEvent, }; use libp2p_core::{transport::ListenerId, ConnectedPoint, Endpoint, Multiaddr}; use libp2p_identity::PeerId; @@ -481,6 +481,7 @@ pub struct ConnectionClosed<'a> { pub peer_id: PeerId, pub connection_id: ConnectionId, pub endpoint: &'a ConnectedPoint, + pub cause: Option<&'a ConnectionError>, pub remaining_established: usize, } diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index fb02cdce392..31eb2aa28f5 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -909,6 +909,7 @@ where peer_id, connection_id: id, endpoint: &endpoint, + cause: error.as_ref(), remaining_established: num_established as usize, })); self.pending_swarm_events diff --git a/swarm/src/test.rs b/swarm/src/test.rs index 547277550bb..d49b504392a 100644 --- a/swarm/src/test.rs +++ b/swarm/src/test.rs @@ -301,6 +301,7 @@ where connection_id, endpoint, remaining_established, + cause, }: ConnectionClosed, ) { let mut other_closed_connections = self @@ -350,6 +351,7 @@ where connection_id, endpoint, remaining_established, + cause, })); } } From c19c1409824a68586c3779729329040a554f12db Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Wed, 10 Jul 2024 02:44:38 +0300 Subject: [PATCH 2/4] fix(webocket): Avoid panic when polling quicksink after errors Pull-Request: #5482. --- Cargo.lock | 3 +- Cargo.toml | 2 +- transports/websocket/CHANGELOG.md | 6 +++ transports/websocket/Cargo.toml | 3 +- transports/websocket/src/framed.rs | 2 +- transports/websocket/src/quicksink.rs | 72 ++++++++++++++++++--------- 6 files changed, 61 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ec114491e48..6a9dc56e987 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3492,7 +3492,7 @@ dependencies = [ [[package]] name = "libp2p-websocket" -version = "0.43.1" +version = "0.43.2" dependencies = [ "async-std", "either", @@ -3507,6 +3507,7 @@ dependencies = [ "rcgen", "rw-stream-sink", "soketto", + "thiserror", "tracing", "url", "webpki-roots 0.25.2", diff --git a/Cargo.toml b/Cargo.toml index 7fa6856c26b..84767573b9e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -112,7 +112,7 @@ libp2p-upnp = { version = "0.2.2", path = "protocols/upnp" } libp2p-webrtc = { version = "0.7.1-alpha", path = "transports/webrtc" } libp2p-webrtc-utils = { version = "0.2.1", path = "misc/webrtc-utils" } libp2p-webrtc-websys = { version = "0.3.0-alpha", path = "transports/webrtc-websys" } -libp2p-websocket = { version = "0.43.1", path = "transports/websocket" } +libp2p-websocket = { version = "0.43.2", path = "transports/websocket" } libp2p-websocket-websys = { version = "0.3.2", path = "transports/websocket-websys" } libp2p-webtransport-websys = { version = "0.3.0", path = "transports/webtransport-websys" } libp2p-yamux = { version = "0.45.2", path = "muxers/yamux" } diff --git a/transports/websocket/CHANGELOG.md b/transports/websocket/CHANGELOG.md index d206cbac6a1..419ff41c6fc 100644 --- a/transports/websocket/CHANGELOG.md +++ b/transports/websocket/CHANGELOG.md @@ -1,3 +1,9 @@ +## 0.43.2 + +- fix: Avoid websocket panic on polling after errors. See [PR 5482]. + +[PR 5482]: https://github.com/libp2p/rust-libp2p/pull/5482 + ## 0.43.1 ## 0.43.0 diff --git a/transports/websocket/Cargo.toml b/transports/websocket/Cargo.toml index b022d95ca47..f1b0a413115 100644 --- a/transports/websocket/Cargo.toml +++ b/transports/websocket/Cargo.toml @@ -3,7 +3,7 @@ name = "libp2p-websocket" edition = "2021" rust-version = { workspace = true } description = "WebSocket transport for libp2p" -version = "0.43.1" +version = "0.43.2" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" @@ -21,6 +21,7 @@ pin-project-lite = "0.2.14" rw-stream-sink = { workspace = true } soketto = "0.8.0" tracing = { workspace = true } +thiserror = "1.0.61" url = "2.5" webpki-roots = "0.25" diff --git a/transports/websocket/src/framed.rs b/transports/websocket/src/framed.rs index f6f99d18580..69a01fdbd46 100644 --- a/transports/websocket/src/framed.rs +++ b/transports/websocket/src/framed.rs @@ -571,7 +571,7 @@ fn location_to_multiaddr(location: &str) -> Result> { /// The websocket connection. pub struct Connection { receiver: BoxStream<'static, Result>, - sender: Pin + Send>>, + sender: Pin> + Send>>, _marker: std::marker::PhantomData, } diff --git a/transports/websocket/src/quicksink.rs b/transports/websocket/src/quicksink.rs index cb2c98b078f..4f620536ea1 100644 --- a/transports/websocket/src/quicksink.rs +++ b/transports/websocket/src/quicksink.rs @@ -30,14 +30,6 @@ // Ok::<_, io::Error>(stdout) // }); // ``` -// -// # Panics -// -// - If any of the [`Sink`] methods produce an error, the sink transitions -// to a failure state and none of its methods must be called afterwards or -// else a panic will occur. -// - If [`Sink::poll_close`] has been called, no other sink method must be -// called afterwards or else a panic will be caused. use futures::{ready, sink::Sink}; use pin_project_lite::pin_project; @@ -102,6 +94,15 @@ enum State { Failed, } +/// Errors the `Sink` may return. +#[derive(Debug, thiserror::Error)] +pub(crate) enum Error { + #[error("Error while sending over the sink, {0}")] + Send(E), + #[error("The Sink has closed")] + Closed, +} + pin_project! { /// `SinkImpl` implements the `Sink` trait. #[derive(Debug)] @@ -119,7 +120,7 @@ where F: FnMut(S, Action) -> T, T: Future>, { - type Error = E; + type Error = Error; fn poll_ready(self: Pin<&mut Self>, cx: &mut Context) -> Poll> { let mut this = self.project(); @@ -135,7 +136,7 @@ where Err(e) => { this.future.set(None); *this.state = State::Failed; - Poll::Ready(Err(e)) + Poll::Ready(Err(Error::Send(e))) } } } @@ -143,20 +144,19 @@ where Ok(_) => { this.future.set(None); *this.state = State::Closed; - panic!("SinkImpl::poll_ready called on a closing sink.") + Poll::Ready(Err(Error::Closed)) } Err(e) => { this.future.set(None); *this.state = State::Failed; - Poll::Ready(Err(e)) + Poll::Ready(Err(Error::Send(e))) } }, State::Empty => { assert!(this.param.is_some()); Poll::Ready(Ok(())) } - State::Closed => panic!("SinkImpl::poll_ready called on a closed sink."), - State::Failed => panic!("SinkImpl::poll_ready called after error."), + State::Closed | State::Failed => Poll::Ready(Err(Error::Closed)), } } @@ -193,7 +193,7 @@ where Err(e) => { this.future.set(None); *this.state = State::Failed; - return Poll::Ready(Err(e)); + return Poll::Ready(Err(Error::Send(e))); } }, State::Flushing => { @@ -207,7 +207,7 @@ where Err(e) => { this.future.set(None); *this.state = State::Failed; - return Poll::Ready(Err(e)); + return Poll::Ready(Err(Error::Send(e))); } } } @@ -221,11 +221,10 @@ where Err(e) => { this.future.set(None); *this.state = State::Failed; - return Poll::Ready(Err(e)); + return Poll::Ready(Err(Error::Send(e))); } }, - State::Closed => return Poll::Ready(Ok(())), - State::Failed => panic!("SinkImpl::poll_flush called after error."), + State::Closed | State::Failed => return Poll::Ready(Err(Error::Closed)), } } } @@ -253,7 +252,7 @@ where Err(e) => { this.future.set(None); *this.state = State::Failed; - return Poll::Ready(Err(e)); + return Poll::Ready(Err(Error::Send(e))); } }, State::Flushing => { @@ -266,7 +265,7 @@ where Err(e) => { this.future.set(None); *this.state = State::Failed; - return Poll::Ready(Err(e)); + return Poll::Ready(Err(Error::Send(e))); } } } @@ -280,11 +279,11 @@ where Err(e) => { this.future.set(None); *this.state = State::Failed; - return Poll::Ready(Err(e)); + return Poll::Ready(Err(Error::Send(e))); } }, State::Closed => return Poll::Ready(Ok(())), - State::Failed => panic!("SinkImpl::poll_closed called after error."), + State::Failed => return Poll::Ready(Err(Error::Closed)), } } } @@ -347,4 +346,31 @@ mod tests { assert_eq!(&expected[..], &actual[..]) }); } + + #[test] + fn error_does_not_panic() { + task::block_on(async { + let sink = make_sink(io::stdout(), |mut _stdout, _action| async move { + Err(io::Error::new(io::ErrorKind::Other, "oh no")) + }); + + futures::pin_mut!(sink); + + let result = sink.send("hello").await; + match result { + Err(crate::quicksink::Error::Send(e)) => { + assert_eq!(e.kind(), io::ErrorKind::Other); + assert_eq!(e.to_string(), "oh no") + } + _ => panic!("unexpected result: {:?}", result), + }; + + // Call send again, expect not to panic. + let result = sink.send("hello").await; + match result { + Err(crate::quicksink::Error::Closed) => {} + _ => panic!("unexpected result: {:?}", result), + }; + }) + } } From 8e28edf69a5f25d1d919962a70fa32203b48f8dd Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 15 Jul 2024 08:16:42 +0000 Subject: [PATCH 3/4] deps: bump obi1kenobi/cargo-semver-checks-action from 2.4 to 2.5 Pull-Request: #5499. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8f1fe263a2d..c8ff0521ce5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -310,7 +310,7 @@ jobs: - uses: actions/checkout@v4 - run: wget -q -O- https://github.com/obi1kenobi/cargo-semver-checks/releases/download/v0.31.0/cargo-semver-checks-x86_64-unknown-linux-gnu.tar.gz | tar -xz -C ~/.cargo/bin shell: bash - - uses: obi1kenobi/cargo-semver-checks-action@c7306483f698c511eaf7416d1bf2e1958c90140f # v2 + - uses: obi1kenobi/cargo-semver-checks-action@ca26a44cfb670b2078c8f757d06e696a7c3820cf # v2 rustfmt: runs-on: ubuntu-latest From e2e98ad5e8f8ab2c0ae037b69a3a7149a116bf02 Mon Sep 17 00:00:00 2001 From: EdwardJES <107906898+EdwardJES@users.noreply.github.com> Date: Wed, 17 Jul 2024 07:15:59 +1000 Subject: [PATCH 4/4] fix: rendezvous identify example This pr closes https://github.com/libp2p/rust-libp2p/issues/5388 by explicitly adding the local observed address, noting this is out of protocol behaviour in non-example cases Pull-Request: #5496. --- examples/rendezvous/src/bin/rzv-identify.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/rendezvous/src/bin/rzv-identify.rs b/examples/rendezvous/src/bin/rzv-identify.rs index 1d545592829..ff637aa6f49 100644 --- a/examples/rendezvous/src/bin/rzv-identify.rs +++ b/examples/rendezvous/src/bin/rzv-identify.rs @@ -76,8 +76,12 @@ async fn main() { } // once `/identify` did its job, we know our external address and can register SwarmEvent::Behaviour(MyBehaviourEvent::Identify(identify::Event::Received { + info, .. })) => { + // Register our external address. Needs to be done explicitly + // for this case, as it's a local address. + swarm.add_external_address(info.observed_addr); if let Err(error) = swarm.behaviour_mut().rendezvous.register( rendezvous::Namespace::from_static("rendezvous"), rendezvous_point,