feat: Better error reporting when features are disabled

core/src/identity.rs

@@ -323,7 +323,7 @@ impl TryFrom<keys_proto::PublicKey> for PublicKey {
             #[cfg(any(not(feature = "rsa"), target_arch = "wasm32"))]
             keys_proto::KeyType::Rsa => {
                 log::debug!("support for RSA was disabled at compile-time");
-                Err(DecodingError::new("Unsupported"))
+                Err(DecodingError::new("RSA keys are unsupported"))
             }
             #[cfg(feature = "secp256k1")]
             keys_proto::KeyType::Secp256k1 => {
@@ -332,7 +332,7 @@ impl TryFrom<keys_proto::PublicKey> for PublicKey {
             #[cfg(not(feature = "secp256k1"))]
             keys_proto::KeyType::Secp256k1 => {
                 log::debug!("support for secp256k1 was disabled at compile-time");
-                Err(DecodingError::new("Unsupported"))
+                Err(DecodingError::new("secp256k1 keys are unsupported"))
             }
             #[cfg(feature = "ecdsa")]
             keys_proto::KeyType::Ecdsa => {
@@ -341,7 +341,7 @@ impl TryFrom<keys_proto::PublicKey> for PublicKey {
             #[cfg(not(feature = "ecdsa"))]
             keys_proto::KeyType::Ecdsa => {
                 log::debug!("support for ECDSA was disabled at compile-time");
-                Err(DecodingError::new("Unsupported"))
+                Err(DecodingError::new("ECDSA keys are unsupported"))
             }
         }
     }

core/src/upgrade/error.rs

@@ -55,8 +55,8 @@ where
 {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            UpgradeError::Select(e) => write!(f, "select error: {}", e),
-            UpgradeError::Apply(e) => write!(f, "upgrade apply error: {}", e),
+            UpgradeError::Select(_) => write!(f, "Multistream select failed"),
+            UpgradeError::Apply(_) => write!(f, "Handshake failed"),
         }
     }
 }

examples/ipfs-kad.rs

@@ -23,16 +23,15 @@
 //! You can pass as parameter a base58 peer ID to search for. If you don't pass any parameter, a
 //! peer ID will be generated randomly.
 
-use async_std::task;
 use futures::StreamExt;
 use libp2p::kad::record::store::MemoryStore;
 use libp2p::kad::{GetClosestPeersError, Kademlia, KademliaConfig, KademliaEvent, QueryResult};
 use libp2p::{
     development_transport, identity,
     swarm::{Swarm, SwarmEvent},
-    Multiaddr, PeerId,
+    PeerId,
 };
-use std::{env, error::Error, str::FromStr, time::Duration};
+use std::{env, error::Error, time::Duration};
 
 const BOOTNODES: [&str; 4] = [
     "QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN",
@@ -63,58 +62,54 @@ async fn main() -> Result<(), Box<dyn Error>> {
         // Add the bootnodes to the local routing table. `libp2p-dns` built
         // into the `transport` resolves the `dnsaddr` when Kademlia tries
         // to dial these nodes.
-        let bootaddr = Multiaddr::from_str("/dnsaddr/bootstrap.libp2p.io")?;
         for peer in &BOOTNODES {
-            behaviour.add_address(&PeerId::from_str(peer)?, bootaddr.clone());
+            behaviour.add_address(&peer.parse()?, "/dnsaddr/bootstrap.libp2p.io".parse()?);
         }
 
         Swarm::new(transport, behaviour, local_peer_id)
     };
 
     // Order Kademlia to search for a peer.
-    let to_search: PeerId = if let Some(peer_id) = env::args().nth(1) {
-        peer_id.parse()?
-    } else {
-        identity::Keypair::generate_ed25519().public().into()
-    };
+    let to_search = env::args()
+        .nth(1)
+        .map(|p| p.parse())
+        .transpose()?
+        .unwrap_or_else(PeerId::random);
 
-    println!("Searching for the closest peers to {:?}", to_search);
+    println!("Searching for the closest peers to {}", to_search);
     swarm.behaviour_mut().get_closest_peers(to_search);
 
-    // Kick it off!
-    task::block_on(async move {
-        loop {
-            let event = swarm.select_next_some().await;
-            if let SwarmEvent::Behaviour(KademliaEvent::OutboundQueryCompleted {
-                result: QueryResult::GetClosestPeers(result),
-                ..
-            }) = event
-            {
-                match result {
-                    Ok(ok) => {
-                        if !ok.peers.is_empty() {
-                            println!("Query finished with closest peers: {:#?}", ok.peers)
-                        } else {
-                            // The example is considered failed as there
-                            // should always be at least 1 reachable peer.
-                            println!("Query finished with no closest peers.")
-                        }
+    loop {
+        let event = swarm.select_next_some().await;
+        if let SwarmEvent::Behaviour(KademliaEvent::OutboundQueryCompleted {
+            result: QueryResult::GetClosestPeers(result),
+            ..
+        }) = event
+        {
+            match result {
+                Ok(ok) => {
+                    if !ok.peers.is_empty() {
+                        println!("Query finished with closest peers: {:#?}", ok.peers)
+                    } else {
+                        // The example is considered failed as there
+                        // should always be at least 1 reachable peer.
+                        println!("Query finished with no closest peers.")
                     }
-                    Err(GetClosestPeersError::Timeout { peers, .. }) => {
-                        if !peers.is_empty() {
-                            println!("Query timed out with closest peers: {:#?}", peers)
-                        } else {
-                            // The example is considered failed as there
-                            // should always be at least 1 reachable peer.
-                            println!("Query timed out with no closest peers.");
-                        }
+                }
+                Err(GetClosestPeersError::Timeout { peers, .. }) => {
+                    if !peers.is_empty() {
+                        println!("Query timed out with closest peers: {:#?}", peers)
+                    } else {
+                        // The example is considered failed as there
+                        // should always be at least 1 reachable peer.
+                        println!("Query timed out with no closest peers.");
                     }
-                };
+                }
+            };
 
-                break;
-            }
+            break;
         }
+    }
 
-        Ok(())
-    })
+    Ok(())
 }

swarm/src/lib.rs

@@ -1480,17 +1480,45 @@ impl fmt::Display for DialError {
                 f,
                 "Dial error: Pending connection attempt has been aborted."
             ),
-            DialError::InvalidPeerId(multihash) => write!(f, "Dial error: multihash {:?} is not a PeerId", multihash),
-            DialError::WrongPeerId { obtained, endpoint} => write!(f, "Dial error: Unexpected peer ID {} at {:?}.", obtained, endpoint),
+            DialError::InvalidPeerId(multihash) => {
+                write!(f, "Dial error: multihash {:?} is not a PeerId", multihash)
+            }
+            DialError::WrongPeerId { obtained, endpoint } => write!(
+                f,
+                "Dial error: Unexpected peer ID {} at {:?}.",
+                obtained, endpoint
+            ),
             DialError::ConnectionIo(e) => write!(
                 f,
-                "Dial error: An I/O error occurred on the connection: {:?}.", e
+                "Dial error: An I/O error occurred on the connection: {:?}.",
+                e
             ),
-            DialError::Transport(e) => write!(f, "An error occurred while negotiating the transport protocol(s) on a connection: {:?}.", e),
+            DialError::Transport(errors) => {
+                write!(f, "Failed to negotiate transport protocol(s): [")?;
+
+                for (addr, error) in errors {
+                    write!(f, "({addr}")?;
+                    print_error_chain(f, error)?;
+                    write!(f, ")")?;
+                }
+                write!(f, "]")?;
+
+                Ok(())
+            }
         }
     }
 }
 
+fn print_error_chain(f: &mut fmt::Formatter<'_>, e: &dyn error::Error) -> fmt::Result {
+    write!(f, ": {e}")?;
+
+    if let Some(source) = e.source() {
+        print_error_chain(f, source)?;
+    }
+
+    Ok(())
+}
+
 impl error::Error for DialError {
     fn source(&self) -> Option<&(dyn error::Error + 'static)> {
         match self {
@@ -1620,10 +1648,13 @@ mod tests {
     use libp2p::core::{identity, multiaddr, transport, upgrade};
     use libp2p::plaintext;
     use libp2p::yamux;
+    use libp2p_core::either::EitherError;
     use libp2p_core::multiaddr::multiaddr;
+    use libp2p_core::transport::memory::MemoryTransportError;
     use libp2p_core::transport::TransportEvent;
-    use libp2p_core::Endpoint;
+    use libp2p_core::{Endpoint, UpgradeError};
     use quickcheck::*;
+    use void::Void;
 
     // Test execution state.
     // Connection => Disconnecting => Connecting.
@@ -2502,4 +2533,23 @@ mod tests {
             e => panic!("Unexpected swarm event {:?}.", e),
         }
     }
+
+    #[test]
+    fn dial_error_prints_sources() {
+        // This constitutes a fairly typical error for chained transports.
+        let error = DialError::Transport(vec![(
+            "/ip4/127.0.0.1/tcp/80".parse().unwrap(),
+            TransportError::Other(io::Error::new(
+                io::ErrorKind::Other,
+                EitherError::<_, Void>::A(EitherError::<Void, _>::B(UpgradeError::Apply(
+                    MemoryTransportError::Unreachable,
+                ))),
+            )),
+        )]);
+
+        let string = format!("{error}");
+
+        // Unfortunately, we have some "empty" errors that lead to multiple colons without text but that is the best we can do.
+        assert_eq!("Failed to negotiate transport protocol(s): [(/ip4/127.0.0.1/tcp/80: : Handshake failed: No listener on the given port.)]", string)
+    }
 }

transports/noise/CHANGELOG.md

@@ -5,9 +5,11 @@
 - Introduce `NoiseAuthenticated::xx` constructor, assuming a X25519 DH key exchange. An XX key exchange and X25519 keys
   are the most common way of using noise in libp2p and thus deserve a convenience constructor. See [PR 2887].
 - Add `NoiseConfig::with_prologue` which allows users to set the noise prologue of the handshake. See [PR 2903].
+- Introduce more variants to `NoiseError` to better differentiate between failure cases during authentication. See [PR XXXX].
 
 [PR 2887]: https://github.com/libp2p/rust-libp2p/pull/2887
 [PR 2903]: https://github.com/libp2p/rust-libp2p/pull/2903
+[PR XXXX]: https://github.com/libp2p/rust-libp2p/pull/XXXX
 
 # 0.39.0
 

transports/noise/Cargo.toml

@@ -19,6 +19,7 @@ prost = "0.11"
 rand = "0.8.3"
 sha2 = "0.10.0"
 static_assertions = "1"
+thiserror = "1.0.37"
 x25519-dalek = "1.1.0"
 zeroize = "1"
 

transports/noise/src/io/framed.rs

@@ -94,28 +94,23 @@ impl<T> NoiseFramed<T, snow::HandshakeState> {
     where
         C: Protocol<C> + AsRef<[u8]>,
     {
-        let dh_remote_pubkey = match self.session.get_remote_static() {
-            None => None,
-            Some(k) => match C::public_from_bytes(k) {
-                Err(e) => return Err(e),
-                Ok(dh_pk) => Some(dh_pk),
-            },
+        let dh_remote_pubkey = self
+            .session
+            .get_remote_static()
+            .map(C::public_from_bytes)
+            .transpose()?;
+
+        let io = NoiseFramed {
+            session: self.session.into_transport_mode()?,
+            io: self.io,
+            read_state: ReadState::Ready,
+            write_state: WriteState::Ready,
+            read_buffer: self.read_buffer,
+            write_buffer: self.write_buffer,
+            decrypt_buffer: self.decrypt_buffer,
         };
-        match self.session.into_transport_mode() {
-            Err(e) => Err(e.into()),
-            Ok(s) => {
-                let io = NoiseFramed {
-                    session: s,
-                    io: self.io,
-                    read_state: ReadState::Ready,
-                    write_state: WriteState::Ready,
-                    read_buffer: self.read_buffer,
-                    write_buffer: self.write_buffer,
-                    decrypt_buffer: self.decrypt_buffer,
-                };
-                Ok((dh_remote_pubkey, NoiseOutput::new(io)))
-            }
-        }
+
+        Ok((dh_remote_pubkey, NoiseOutput::new(io)))
     }
 }