From 8793603205a7680e9e89492386d72035ae94285e Mon Sep 17 00:00:00 2001 From: Craig Macomber Date: Sat, 14 Feb 2026 13:34:42 -0800 Subject: [PATCH 1/6] Add more docs and test coverage --- matchbox_socket/src/webrtc_socket/error.rs | 7 ++- matchbox_socket/src/webrtc_socket/socket.rs | 57 ++++++++++++++++++--- 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/matchbox_socket/src/webrtc_socket/error.rs b/matchbox_socket/src/webrtc_socket/error.rs index c4f671f0..065350a4 100644 --- a/matchbox_socket/src/webrtc_socket/error.rs +++ b/matchbox_socket/src/webrtc_socket/error.rs @@ -2,14 +2,17 @@ use crate::webrtc_socket::messages::PeerEvent; use cfg_if::cfg_if; /// An error that can occur when getting a socket's channel through -/// `get_channel`, `take_channel` or `try_update_peers`. +/// [`get_channel`](crate::webrtc_socket::WebRtcSocket::get_channel), +/// [`take_channel`](crate::webrtc_socket::WebRtcSocket::take_channel) or +/// [`try_update_peers`](crate::webrtc_socket::WebRtcSocket::try_update_peers). #[derive(Debug, thiserror::Error)] pub enum ChannelError { /// Can occur if trying to get a channel with an Id that was not added while building the /// socket #[error("This channel was never created")] NotFound, - /// The channel has already been taken and is no longer on the socket + /// The channel has already been taken and is no longer on the socket. + /// See [Web] #[error("This channel has already been taken and is no longer on the socket")] Taken, /// Channel might have been opened but later closed, or never opened in the first place. diff --git a/matchbox_socket/src/webrtc_socket/socket.rs b/matchbox_socket/src/webrtc_socket/socket.rs index f10e7e72..ef4f0483 100644 --- a/matchbox_socket/src/webrtc_socket/socket.rs +++ b/matchbox_socket/src/webrtc_socket/socket.rs @@ -279,6 +279,8 @@ pub enum PeerState { } /// Used to send and receive packets on a given WebRTC channel. Must be created as part of a /// [`WebRtcSocket`]. +/// This corespondents to a collection of lower level channels, one for each [PeerState::Connected] +/// peer (identified by their PeerId). #[derive(Debug)] pub struct WebRtcChannel { config: ChannelConfig, @@ -472,6 +474,9 @@ pub struct WebRtcSocket { id_rx: futures_channel::oneshot::Receiver, peer_state_rx: futures_channel::mpsc::UnboundedReceiver<(PeerId, PeerState)>, peers: HashMap, + /// The channels, in the order specified by the builder. + /// These are [Some] even before any connection is made: the only transition to [None] when + /// [ChannelError::Taken]. channels: Vec>, } @@ -711,7 +716,8 @@ impl WebRtcSocket { .ok_or(ChannelError::Taken) } - /// Takes the [`WebRtcChannel`] of a given id. + /// Takes the [`WebRtcChannel`] at the specified index. + /// The channels are indexed based on the order they were added to the builder. /// /// ``` /// use matchbox_socket::*; @@ -725,9 +731,9 @@ impl WebRtcSocket { /// ``` /// /// See also: [`WebRtcSocket::channel`] - pub fn take_channel(&mut self, channel: usize) -> Result { + pub fn take_channel(&mut self, channel_index: usize) -> Result { self.channels - .get_mut(channel) + .get_mut(channel_index) .ok_or(ChannelError::NotFound)? .take() .ok_or(ChannelError::Taken) @@ -735,12 +741,12 @@ impl WebRtcSocket { /// Takes the [`WebRtcChannel`] of a given [`PeerId`]. pub fn take_channel_by_id(&mut self, id: PeerId) -> Result { - let pos = self + let peer_index = self .connected_peers() .position(|peer_id| peer_id == id) .ok_or(ChannelError::NotFound)?; - self.take_channel(pos) + self.take_channel(peer_index) } /// Converts the [`WebRtcChannel`] of a given [`PeerId`] into a [`RawPeerChannel`]. @@ -906,7 +912,7 @@ fn compat_read_write( #[cfg(test)] mod test { - use crate::{ChannelConfig, Error, WebRtcSocketBuilder}; + use crate::{ChannelConfig, ChannelError, Error, WebRtcSocket, WebRtcSocketBuilder}; #[futures_test::test] async fn unreachable_server() { @@ -937,4 +943,43 @@ mod test { Error::ConnectionFailed { .. }, )); } + + #[test] + #[should_panic(expected = "Must have added at least one channel")] + fn no_channels() { + let (_socket, _fut) = WebRtcSocketBuilder::new("wss://example.invalid").build(); + } + + #[test] + fn builder() { + let mut builder = WebRtcSocket::builder("wss://example.invalid"); + builder = builder.add_reliable_channel(); + builder = builder.add_unreliable_channel(); + let (socket, _fut) = builder.build(); + assert_eq!(socket.channels.len(), 2); + // Channels are populated immediately, even before connecting. + let config_0 = &socket.channels[0].as_ref().unwrap().config; + let config_1 = &socket.channels[1].as_ref().unwrap().config; + assert_eq!(config_0.ordered, true); + assert_eq!(config_0.max_retransmits, None); + assert_eq!(config_1.ordered, false); + assert_eq!(config_1.max_retransmits, Some(0)); + } + + #[test] + fn take_channel() { + let mut builder = WebRtcSocket::builder("wss://example.invalid"); + builder = builder.add_reliable_channel(); + let (mut socket, _fut) = builder.build(); + assert!(socket.channels[0].is_some()); + let mut taken = socket.take_channel(0).unwrap(); + assert!(socket.channels[0].is_none()); + assert!(matches!( + socket.take_channel(0).unwrap_err(), + ChannelError::Taken, + )); + assert!(!taken.is_closed()); + taken.close(); + assert!(taken.is_closed()); + } } From 8899aad828eb02139252c3f077f3e7f302cf8523 Mon Sep 17 00:00:00 2001 From: Craig Macomber Date: Sat, 14 Feb 2026 14:10:42 -0800 Subject: [PATCH 2/6] Remove broken [web] comment. --- matchbox_socket/src/webrtc_socket/error.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/matchbox_socket/src/webrtc_socket/error.rs b/matchbox_socket/src/webrtc_socket/error.rs index 065350a4..4e0bec19 100644 --- a/matchbox_socket/src/webrtc_socket/error.rs +++ b/matchbox_socket/src/webrtc_socket/error.rs @@ -12,7 +12,6 @@ pub enum ChannelError { #[error("This channel was never created")] NotFound, /// The channel has already been taken and is no longer on the socket. - /// See [Web] #[error("This channel has already been taken and is no longer on the socket")] Taken, /// Channel might have been opened but later closed, or never opened in the first place. From 8e88d8f6e19aff352455d985e9ebc208742d2274 Mon Sep 17 00:00:00 2001 From: Craig Macomber Date: Sat, 14 Feb 2026 14:13:36 -0800 Subject: [PATCH 3/6] Fix lints --- matchbox_socket/src/webrtc_socket/socket.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/matchbox_socket/src/webrtc_socket/socket.rs b/matchbox_socket/src/webrtc_socket/socket.rs index ef4f0483..ab2c36c9 100644 --- a/matchbox_socket/src/webrtc_socket/socket.rs +++ b/matchbox_socket/src/webrtc_socket/socket.rs @@ -960,9 +960,9 @@ mod test { // Channels are populated immediately, even before connecting. let config_0 = &socket.channels[0].as_ref().unwrap().config; let config_1 = &socket.channels[1].as_ref().unwrap().config; - assert_eq!(config_0.ordered, true); + assert!(config_0.ordered); assert_eq!(config_0.max_retransmits, None); - assert_eq!(config_1.ordered, false); + assert!(!config_1.ordered); assert_eq!(config_1.max_retransmits, Some(0)); } From 8c7d53180d3e085a8180a1af107933fa2c9af1d1 Mon Sep 17 00:00:00 2001 From: Craig Date: Sat, 14 Feb 2026 17:12:26 -0800 Subject: [PATCH 4/6] Apply suggestion from @johanhelsing Co-authored-by: Johan Klokkhammer Helsing --- matchbox_socket/src/webrtc_socket/socket.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matchbox_socket/src/webrtc_socket/socket.rs b/matchbox_socket/src/webrtc_socket/socket.rs index ab2c36c9..7914dd95 100644 --- a/matchbox_socket/src/webrtc_socket/socket.rs +++ b/matchbox_socket/src/webrtc_socket/socket.rs @@ -279,7 +279,7 @@ pub enum PeerState { } /// Used to send and receive packets on a given WebRTC channel. Must be created as part of a /// [`WebRtcSocket`]. -/// This corespondents to a collection of lower level channels, one for each [PeerState::Connected] +/// This corresponds to a collection of lower level channels, one for each [PeerState::Connected] /// peer (identified by their PeerId). #[derive(Debug)] pub struct WebRtcChannel { From 0d41e3ff2239283fb03560537c01cc969c918044 Mon Sep 17 00:00:00 2001 From: Craig Date: Sat, 14 Feb 2026 17:13:21 -0800 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Johan Klokkhammer Helsing --- matchbox_socket/src/webrtc_socket/socket.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matchbox_socket/src/webrtc_socket/socket.rs b/matchbox_socket/src/webrtc_socket/socket.rs index 7914dd95..22d11879 100644 --- a/matchbox_socket/src/webrtc_socket/socket.rs +++ b/matchbox_socket/src/webrtc_socket/socket.rs @@ -475,7 +475,7 @@ pub struct WebRtcSocket { peer_state_rx: futures_channel::mpsc::UnboundedReceiver<(PeerId, PeerState)>, peers: HashMap, /// The channels, in the order specified by the builder. - /// These are [Some] even before any connection is made: the only transition to [None] when + /// These are [`Some`] even before any connection is made: the only transition to [`None`] when /// [ChannelError::Taken]. channels: Vec>, } From 885ff1b6045f6cc1e897afda4a3812c033ed0ccd Mon Sep 17 00:00:00 2001 From: Craig Date: Sun, 15 Feb 2026 15:58:07 -0800 Subject: [PATCH 6/6] Apply suggestions from code review Co-authored-by: Johan Klokkhammer Helsing --- matchbox_socket/src/webrtc_socket/socket.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/matchbox_socket/src/webrtc_socket/socket.rs b/matchbox_socket/src/webrtc_socket/socket.rs index 22d11879..8974eddd 100644 --- a/matchbox_socket/src/webrtc_socket/socket.rs +++ b/matchbox_socket/src/webrtc_socket/socket.rs @@ -279,7 +279,7 @@ pub enum PeerState { } /// Used to send and receive packets on a given WebRTC channel. Must be created as part of a /// [`WebRtcSocket`]. -/// This corresponds to a collection of lower level channels, one for each [PeerState::Connected] +/// This corresponds to a collection of lower level channels, one for each [`PeerState::Connected`] /// peer (identified by their PeerId). #[derive(Debug)] pub struct WebRtcChannel { @@ -476,7 +476,7 @@ pub struct WebRtcSocket { peers: HashMap, /// The channels, in the order specified by the builder. /// These are [`Some`] even before any connection is made: the only transition to [`None`] when - /// [ChannelError::Taken]. + /// [`ChannelError::Taken`]. channels: Vec>, }