Treat connection limit errors as pending connection errors. (#1546)

* Treat connection limit errors as pending connection errors.

* Remove handler from network event.

Co-authored-by: Pierre Krieger <pierre.krieger1708@gmail.com>

core/src/connection/error.rs

@@ -29,10 +29,6 @@ pub enum ConnectionError<THandlerErr> {
     // TODO: Eventually this should also be a custom error?
     IO(io::Error),
 
-    /// The connection was dropped because the connection limit
-    /// for a peer has been reached.
-    ConnectionLimit(ConnectionLimit),
-
     /// The connection handler produced an error.
     Handler(THandlerErr),
 }
@@ -48,8 +44,6 @@ where
                 write!(f, "Connection error: I/O error: {}", err),
             ConnectionError::Handler(err) =>
                 write!(f, "Connection error: Handler error: {}", err),
-            ConnectionError::ConnectionLimit(l) =>
-                write!(f, "Connection error: Connection limit: {}.", l)
         }
     }
 }
@@ -63,7 +57,6 @@ where
         match self {
             ConnectionError::IO(err) => Some(err),
             ConnectionError::Handler(err) => Some(err),
-            ConnectionError::ConnectionLimit(..) => None,
         }
     }
 }
@@ -78,6 +71,10 @@ pub enum PendingConnectionError<TTransErr> {
     /// match the one that was expected or is otherwise invalid.
     InvalidPeerId,
 
+    /// The connection was dropped because the connection limit
+    /// for a peer has been reached.
+    ConnectionLimit(ConnectionLimit),
+
     /// An I/O error occurred on the connection.
     // TODO: Eventually this should also be a custom error?
     IO(io::Error),
@@ -96,6 +93,8 @@ where
                 write!(f, "Pending connection: Transport error: {}", err),
             PendingConnectionError::InvalidPeerId =>
                 write!(f, "Pending connection: Invalid peer ID."),
+            PendingConnectionError::ConnectionLimit(l) =>
+                write!(f, "Connection error: Connection limit: {}.", l),
         }
     }
 }
@@ -110,6 +109,7 @@ where
             PendingConnectionError::IO(err) => Some(err),
             PendingConnectionError::Transport(err) => Some(err),
             PendingConnectionError::InvalidPeerId => None,
+            PendingConnectionError::ConnectionLimit(..) => None,
         }
     }
 }

core/src/connection/pool.rs

@@ -112,7 +112,7 @@ pub enum PoolEvent<'a, TInEvent, TOutEvent, THandler, TTransErr, THandlerErr, TC
         error: PendingConnectionError<TTransErr>,
         /// The handler that was supposed to handle the connection,
         /// if the connection failed before the handler was consumed.
-        handler: THandler,
+        handler: Option<THandler>,
         /// The (expected) peer of the failed connection.
         peer: Option<TPeerId>,
         /// A reference to the pool that managed the connection.
@@ -558,7 +558,7 @@ where
                             id,
                             endpoint,
                             error,
-                            handler,
+                            handler: Some(handler),
                             peer,
                             pool: self
                         })
@@ -588,13 +588,13 @@ where
                                             .map_or(0, |conns| conns.len());
                         if let Err(e) = self.limits.check_established(current) {
                             let connected = entry.close();
-                            let num_established = u32::try_from(e.current).unwrap();
-                            return Poll::Ready(PoolEvent::ConnectionError {
+                            return Poll::Ready(PoolEvent::PendingConnectionError {
                                 id,
-                                connected,
-                                error: ConnectionError::ConnectionLimit(e),
-                                num_established,
-                                pool: self,
+                                endpoint: connected.endpoint,
+                                error: PendingConnectionError::ConnectionLimit(e),
+                                handler: None,
+                                peer,
+                                pool: self
                             })
                         }
                         // Peer ID checks must already have happened. See `add_pending`.

core/src/network.rs

@@ -512,7 +512,7 @@ fn on_connection_failed<'a, TTrans, TInEvent, TOutEvent, THandler, TConnInfo, TP
     id: ConnectionId,
     endpoint: ConnectedPoint,
     error: PendingConnectionError<TTrans::Error>,
-    handler: THandler,
+    handler: Option<THandler>,
 ) -> (Option<DialingOpts<TPeerId, THandler>>, NetworkEvent<'a, TTrans, TInEvent, TOutEvent, THandler, TConnInfo, TPeerId>)
 where
     TTrans: Transport,
@@ -533,8 +533,9 @@ where
         let num_remain = u32::try_from(attempt.next.len()).unwrap();
         let failed_addr = attempt.current.clone();
 
-        let opts =
+        let (opts, attempts_remaining) =
             if num_remain > 0 {
+                if let Some(handler) = handler {
                     let next_attempt = attempt.next.remove(0);
                     let opts = DialingOpts {
                         peer: peer_id.clone(),
@@ -542,13 +543,19 @@ where
                         address: next_attempt,
                         remaining: attempt.next
                     };
-                Some(opts)
+                    (Some(opts), num_remain)
                 } else {
-                None
+                    // The error is "fatal" for the dialing attempt, since
+                    // the handler was already consumed. All potential
+                    // remaining connection attempts are thus void.
+                    (None, 0)
+                }
+            } else {
+                (None, 0)
             };
 
         (opts, NetworkEvent::DialError {
-            attempts_remaining: num_remain,
+            attempts_remaining,
             peer_id,
             multiaddr: failed_addr,
             error,
@@ -560,7 +567,6 @@ where
                 (None, NetworkEvent::UnknownPeerDialError {
                     multiaddr: address,
                     error,
-                    handler,
                 }),
             ConnectedPoint::Listener { local_addr, send_back_addr } =>
                 (None, NetworkEvent::IncomingConnectionError {

core/src/network/event.rs

@@ -146,10 +146,6 @@ where
 
         /// The error that happened.
         error: PendingConnectionError<TTrans::Error>,
-
-        /// The handler that was passed to `dial()`, if the
-        /// connection failed before the handler was consumed.
-        handler: THandler,
     },
 
     /// An established connection produced an event.