chore(snownet): embed more context in WireGuard errors (#9687)

rust/Cargo.lock

@@ -6804,6 +6804,7 @@ dependencies = [
 name = "snownet"
 version = "0.1.0"
 dependencies = [
+ "anyhow",
  "boringtun",
  "bufferpool",
  "bytecodec",

rust/connlib/snownet/Cargo.toml

@@ -5,6 +5,7 @@ edition = { workspace = true }
 license = { workspace = true }
 
 [dependencies]
+anyhow = { workspace = true }
 boringtun = { workspace = true }
 bufferpool = { workspace = true }
 bytecodec = { workspace = true }

rust/connlib/snownet/src/lib.rs

@@ -16,7 +16,7 @@ pub use allocation::RelaySocket;
 #[allow(deprecated)] // Rust bug: `expect` doesn't seem to work on imports?
 pub use node::{Answer, Offer};
 pub use node::{
-    Client, ClientNode, Credentials, Error, Event, HANDSHAKE_TIMEOUT, NoTurnServers, Node, Server,
+    Client, ClientNode, Credentials, Event, HANDSHAKE_TIMEOUT, NoTurnServers, Node, Server,
     ServerNode, Transmit,
 };
 pub use stats::{ConnectionStats, NodeStats};

rust/connlib/snownet/src/node.rs

@@ -3,6 +3,7 @@ use crate::candidate_set::CandidateSet;
 use crate::index::IndexLfsr;
 use crate::stats::{ConnectionStats, NodeStats};
 use crate::utils::{channel_data_packet_buffer, earliest};
+use anyhow::{Context, Result, anyhow};
 use boringtun::noise::errors::WireGuardError;
 use boringtun::noise::{Tunn, TunnResult};
 use boringtun::x25519::PublicKey;
@@ -136,22 +137,6 @@ pub struct Node<T, TId, RId> {
     rng: StdRng,
 }
 
-#[derive(thiserror::Error, Debug)]
-pub enum Error {
-    #[error("Unknown interface")]
-    UnknownInterface,
-    #[error("Failed to decapsulate: {0}")]
-    Decapsulate(boringtun::noise::errors::WireGuardError),
-    #[error("Failed to encapsulate: {0}")]
-    Encapsulate(boringtun::noise::errors::WireGuardError),
-    #[error("Packet has unknown format")]
-    UnknownPacketFormat,
-    #[error("Not connected")]
-    NotConnected,
-    #[error("Invalid local address: {0}")]
-    BadLocalAddress(#[from] str0m::error::IceError),
-}
-
 #[derive(thiserror::Error, Debug)]
 #[error("No TURN servers available")]
 pub struct NoTurnServers {}
@@ -337,7 +322,7 @@ where
     ///  └─────────┘      │  │         └────┘
     ///                   └──┘
     /// ```
-    pub fn add_local_host_candidate(&mut self, address: SocketAddr) -> Result<(), Error> {
+    pub fn add_local_host_candidate(&mut self, address: SocketAddr) -> Result<()> {
         self.add_local_as_host_candidate(address)?;
 
         Ok(())
@@ -410,7 +395,7 @@ where
         from: SocketAddr,
         packet: &[u8],
         now: Instant,
-    ) -> Result<Option<(TId, IpPacket)>, Error> {
+    ) -> Result<Option<(TId, IpPacket)>> {
         self.add_local_as_host_candidate(local)?;
 
         let (from, packet, relayed) = match self.allocations_try_handle(from, local, packet, now) {
@@ -446,11 +431,11 @@ where
         connection: TId,
         packet: IpPacket,
         now: Instant,
-    ) -> Result<Option<Transmit>, Error> {
+    ) -> Result<Option<Transmit>> {
         let conn = self
             .connections
             .get_established_mut(&connection)
-            .ok_or(Error::NotConnected)?;
+            .with_context(|| format!("Unknown connection {connection}"))?;
 
         if self.mode.is_server() && !conn.state.has_nominated_socket() {
             tracing::debug!(
@@ -466,7 +451,8 @@ where
 
         // Encode the packet with an offset of 4 bytes, in case we need to wrap it in a channel-data message.
         let Some(packet_len) = conn
-            .encapsulate(packet, &mut buffer[4..], now)?
+            .encapsulate(packet, &mut buffer[4..], now)
+            .with_context(|| format!("cid={connection}"))?
             .map(|p| p.len())
         // Mapping to len() here terminate the mutable borrow of buffer, allowing re-borrowing further down.
         else {
@@ -489,7 +475,9 @@ where
             }
             ConnectionState::Connected { peer_socket, .. } => peer_socket,
             ConnectionState::Idle { peer_socket } => peer_socket,
-            ConnectionState::Failed => return Err(Error::NotConnected),
+            ConnectionState::Failed => {
+                return Err(anyhow!("Connection {connection} failed"));
+            }
         };
 
         match *socket {
@@ -783,8 +771,9 @@ where
     ///
     /// Receiving traffic on a certain interface means we at least have a connection to a relay via this interface.
     /// Thus, it is also a viable interface to attempt a connection to a gateway.
-    fn add_local_as_host_candidate(&mut self, local: SocketAddr) -> Result<(), Error> {
-        let host_candidate = Candidate::host(local, Protocol::Udp)?;
+    fn add_local_as_host_candidate(&mut self, local: SocketAddr) -> Result<()> {
+        let host_candidate =
+            Candidate::host(local, Protocol::Udp).context("Failed to parse host candidate")?;
 
         if !self.shared_candidates.insert(host_candidate.clone()) {
             return Ok(());
@@ -868,7 +857,7 @@ where
         destination: SocketAddr,
         packet: &[u8],
         now: Instant,
-    ) -> ControlFlow<Result<(), Error>> {
+    ) -> ControlFlow<Result<()>> {
         let Ok(message) = StunMessage::parse(packet) else {
             return ControlFlow::Continue(());
         };
@@ -899,7 +888,7 @@ where
         from: SocketAddr,
         packet: &[u8],
         now: Instant,
-    ) -> ControlFlow<Result<(), Error>, (TId, IpPacket)> {
+    ) -> ControlFlow<Result<()>, (TId, IpPacket)> {
         for (cid, conn) in self.connections.iter_established_mut() {
             if !conn.accepts(&from) {
                 continue;
@@ -927,7 +916,9 @@ where
 
             return match control_flow {
                 ControlFlow::Continue(c) => ControlFlow::Continue((cid, c)),
-                ControlFlow::Break(b) => ControlFlow::Break(b),
+                ControlFlow::Break(b) => ControlFlow::Break(
+                    b.with_context(|| format!("cid={cid} length={}", packet.len())),
+                ),
             };
         }
 
@@ -939,7 +930,7 @@ where
             return ControlFlow::Break(Ok(()));
         }
 
-        ControlFlow::Break(Err(Error::UnknownPacketFormat))
+        ControlFlow::Break(Err(anyhow!("Packet has unknown format")))
     }
 
     fn allocations_drain_events(&mut self) {
@@ -2086,14 +2077,14 @@ where
         packet: IpPacket,
         buffer: &'b mut [u8],
         now: Instant,
-    ) -> Result<Option<&'b [u8]>, Error> {
+    ) -> Result<Option<&'b [u8]>> {
         let _guard = self.span.enter();
 
         self.state.on_outgoing(&mut self.agent, &packet, now);
 
         let len = match self.tunnel.encapsulate_at(packet.packet(), buffer, now) {
             TunnResult::Done => return Ok(None),
-            TunnResult::Err(e) => return Err(Error::Encapsulate(e)),
+            TunnResult::Err(e) => return Err(anyhow::Error::new(e)),
             TunnResult::WriteToNetwork(packet) => packet.len(),
             TunnResult::WriteToTunnelV4(_, _) | TunnResult::WriteToTunnelV6(_, _) => {
                 unreachable!("never returned from encapsulate")
@@ -2110,7 +2101,7 @@ where
         allocations: &mut BTreeMap<RId, Allocation>,
         transmits: &mut VecDeque<Transmit>,
         now: Instant,
-    ) -> ControlFlow<Result<(), Error>, IpPacket> {
+    ) -> ControlFlow<Result<()>, IpPacket> {
         let _guard = self.span.enter();
         let mut ip_packet = IpPacketBuf::new();
 
@@ -2119,7 +2110,7 @@ where
             .decapsulate_at(Some(src), packet, ip_packet.buf(), now)
         {
             TunnResult::Done => ControlFlow::Break(Ok(())),
-            TunnResult::Err(e) => ControlFlow::Break(Err(Error::Decapsulate(e))),
+            TunnResult::Err(e) => ControlFlow::Break(Err(anyhow::Error::new(e))),
 
             // For WriteToTunnel{V4,V6}, boringtun returns the source IP of the packet that was tunneled to us.
             // I am guessing this was done for convenience reasons.

rust/connlib/tunnel/src/client.rs

@@ -415,7 +415,7 @@ impl ClientState {
             packet.as_ref(),
             now,
         )
-        .inspect_err(|e| tracing::debug!(%local, num_bytes = %packet.len(), "Failed to decapsulate incoming packet: {}", err_with_src(e)))
+        .inspect_err(|e| tracing::debug!(%local, num_bytes = %packet.len(), "Failed to decapsulate: {e:#}"))
         .ok()??;
 
         if self.tcp_dns_client.accepts(&packet) {
@@ -547,7 +547,7 @@ impl ClientState {
         let transmit = self
             .node
             .encapsulate(gid, packet, now)
-            .inspect_err(|e| tracing::debug!(%gid, "Failed to encapsulate: {}", err_with_src(e)))
+            .inspect_err(|e| tracing::debug!(%gid, "Failed to encapsulate: {e:#}"))
             .ok()??;
 
         Some(transmit)

rust/connlib/tunnel/src/gateway.rs

@@ -500,7 +500,7 @@ fn encrypt_packet(
 ) -> Result<Option<Transmit>> {
     let transmit = node
         .encapsulate(cid, packet, now)
-        .context("Failed to encapsulate packet")?;
+        .context("Failed to encapsulate")?;
 
     Ok(transmit)
 }