refactor: rename ConnectionError to CloseReason (#1872)

The `neqo_transport::ConnectionError` enum contains the two non-error variants
`Error::NoError` and `CloseReason::Application(0)`. In other words,
`ConnectionError` contains variants that are not errors.

This commit renames `ConnectionError` to the more descriptive name
`CloseReason`.

See suggestion in #1866 (comment).

To ease the upgrade for downstream users, this commit adds a deprecated
`ConnectionError`, guiding users to rename to `CloseReason` via a deprecation warning.

``` rust
pub type ConnectionError = CloseReason;
```

neqo-bin/src/client/http09.rs

@@ -20,7 +20,7 @@ use std::{
 use neqo_common::{event::Provider, qdebug, qinfo, qwarn, Datagram};
 use neqo_crypto::{AuthenticationStatus, ResumptionToken};
 use neqo_transport::{
-    Connection, ConnectionError, ConnectionEvent, EmptyConnectionIdGenerator, Error, Output, State,
+    CloseReason, Connection, ConnectionEvent, EmptyConnectionIdGenerator, Error, Output, State,
     StreamId, StreamType,
 };
 use url::Url;
@@ -143,7 +143,7 @@ pub(crate) fn create_client(
 }
 
 impl TryFrom<&State> for CloseState {
-    type Error = ConnectionError;
+    type Error = CloseReason;
 
     fn try_from(value: &State) -> Result<Self, Self::Error> {
         let (state, error) = match value {
@@ -183,7 +183,7 @@ impl super::Client for Connection {
         }
     }
 
-    fn is_closed(&self) -> Result<CloseState, ConnectionError> {
+    fn is_closed(&self) -> Result<CloseState, CloseReason> {
         self.state().try_into()
     }
 

neqo-bin/src/client/http3.rs

@@ -22,8 +22,8 @@ use neqo_common::{event::Provider, hex, qdebug, qinfo, qwarn, Datagram, Header};
 use neqo_crypto::{AuthenticationStatus, ResumptionToken};
 use neqo_http3::{Error, Http3Client, Http3ClientEvent, Http3Parameters, Http3State, Priority};
 use neqo_transport::{
-    AppError, Connection, ConnectionError, EmptyConnectionIdGenerator, Error as TransportError,
-    Output, StreamId,
+    AppError, CloseReason, Connection, EmptyConnectionIdGenerator, Error as TransportError, Output,
+    StreamId,
 };
 use url::Url;
 
@@ -106,7 +106,7 @@ pub(crate) fn create_client(
 }
 
 impl TryFrom<Http3State> for CloseState {
-    type Error = ConnectionError;
+    type Error = CloseReason;
 
     fn try_from(value: Http3State) -> Result<Self, Self::Error> {
         let (state, error) = match value {
@@ -124,7 +124,7 @@ impl TryFrom<Http3State> for CloseState {
 }
 
 impl super::Client for Http3Client {
-    fn is_closed(&self) -> Result<CloseState, ConnectionError> {
+    fn is_closed(&self) -> Result<CloseState, CloseReason> {
         self.state().try_into()
     }
 

neqo-bin/src/client/mod.rs

@@ -27,7 +27,7 @@ use neqo_crypto::{
     init, Cipher, ResumptionToken,
 };
 use neqo_http3::Output;
-use neqo_transport::{AppError, ConnectionError, ConnectionId, Version};
+use neqo_transport::{AppError, CloseReason, ConnectionId, Version};
 use qlog::{events::EventImportance, streamer::QlogStreamer};
 use tokio::time::Sleep;
 use url::{Origin, Url};
@@ -80,11 +80,11 @@ impl From<neqo_transport::Error> for Error {
     }
 }
 
-impl From<neqo_transport::ConnectionError> for Error {
-    fn from(err: neqo_transport::ConnectionError) -> Self {
+impl From<neqo_transport::CloseReason> for Error {
+    fn from(err: neqo_transport::CloseReason) -> Self {
         match err {
-            ConnectionError::Transport(e) => Self::TransportError(e),
-            ConnectionError::Application(e) => Self::ApplicationError(e),
+            CloseReason::Transport(e) => Self::TransportError(e),
+            CloseReason::Application(e) => Self::ApplicationError(e),
         }
     }
 }
@@ -361,7 +361,7 @@ trait Client {
     fn close<S>(&mut self, now: Instant, app_error: AppError, msg: S)
     where
         S: AsRef<str> + Display;
-    fn is_closed(&self) -> Result<CloseState, ConnectionError>;
+    fn is_closed(&self) -> Result<CloseState, CloseReason>;
     fn stats(&self) -> neqo_transport::Stats;
 }
 

neqo-http3/src/connection.rs

@@ -17,7 +17,7 @@ use std::{
 use neqo_common::{qdebug, qerror, qinfo, qtrace, qwarn, Decoder, Header, MessageType, Role};
 use neqo_qpack::{decoder::QPackDecoder, encoder::QPackEncoder};
 use neqo_transport::{
-    streams::SendOrder, AppError, Connection, ConnectionError, DatagramTracking, State, StreamId,
+    streams::SendOrder, AppError, CloseReason, Connection, DatagramTracking, State, StreamId,
     StreamType, ZeroRttState,
 };
 
@@ -81,22 +81,22 @@ enum Http3RemoteSettingsState {
 /// - `ZeroRtt`: 0-RTT has been enabled and is active
 /// - Connected
 /// - GoingAway(StreamId): The connection has received a `GOAWAY` frame
-/// - Closing(ConnectionError): The connection is closed. The closing has been initiated by this end
-///   of the connection, e.g., the `CONNECTION_CLOSE` frame has been sent. In this state, the
+/// - Closing(CloseReason): The connection is closed. The closing has been initiated by this end of
+///   the connection, e.g., the `CONNECTION_CLOSE` frame has been sent. In this state, the
 ///   connection waits a certain amount of time to retransmit the `CONNECTION_CLOSE` frame if
 ///   needed.
-/// - Closed(ConnectionError): This is the final close state: closing has been initialized by the
-///   peer and an ack for the `CONNECTION_CLOSE` frame has been sent or the closing has been
-///   initiated by this end of the connection and the ack for the `CONNECTION_CLOSE` has been
-///   received or the waiting time has passed.
+/// - Closed(CloseReason): This is the final close state: closing has been initialized by the peer
+///   and an ack for the `CONNECTION_CLOSE` frame has been sent or the closing has been initiated by
+///   this end of the connection and the ack for the `CONNECTION_CLOSE` has been received or the
+///   waiting time has passed.
 #[derive(Debug, PartialEq, PartialOrd, Ord, Eq, Clone)]
 pub enum Http3State {
     Initializing,
     ZeroRtt,
     Connected,
     GoingAway(StreamId),
-    Closing(ConnectionError),
-    Closed(ConnectionError),
+    Closing(CloseReason),
+    Closed(CloseReason),
 }
 
 impl Http3State {
@@ -767,7 +767,7 @@ impl Http3Connection {
     /// This is called when an application closes the connection.
     pub fn close(&mut self, error: AppError) {
         qdebug!([self], "Close connection error {:?}.", error);
-        self.state = Http3State::Closing(ConnectionError::Application(error));
+        self.state = Http3State::Closing(CloseReason::Application(error));
         if (!self.send_streams.is_empty() || !self.recv_streams.is_empty()) && (error == 0) {
             qwarn!("close(0) called when streams still active");
         }

neqo-http3/src/connection_client.rs

@@ -1291,8 +1291,8 @@ mod tests {
     use neqo_crypto::{AllowZeroRtt, AntiReplay, ResumptionToken};
     use neqo_qpack::{encoder::QPackEncoder, QpackSettings};
     use neqo_transport::{
-        ConnectionError, ConnectionEvent, ConnectionParameters, Output, State, StreamId,
-        StreamType, Version, RECV_BUFFER_SIZE, SEND_BUFFER_SIZE,
+        CloseReason, ConnectionEvent, ConnectionParameters, Output, State, StreamId, StreamType,
+        Version, RECV_BUFFER_SIZE, SEND_BUFFER_SIZE,
     };
     use test_fixture::{
         anti_replay, default_server_h3, fixture_init, new_server, now,
@@ -1314,7 +1314,7 @@ mod tests {
     fn assert_closed(client: &Http3Client, expected: &Error) {
         match client.state() {
             Http3State::Closing(err) | Http3State::Closed(err) => {
-                assert_eq!(err, ConnectionError::Application(expected.code()));
+                assert_eq!(err, CloseReason::Application(expected.code()));
             }
             _ => panic!("Wrong state {:?}", client.state()),
         };
@@ -4419,7 +4419,7 @@ mod tests {
                 HSetting::new(HSettingType::BlockedStreams, 100),
                 HSetting::new(HSettingType::MaxHeaderListSize, 10000),
             ],
-            &Http3State::Closing(ConnectionError::Application(265)),
+            &Http3State::Closing(CloseReason::Application(265)),
             ENCODER_STREAM_DATA_WITH_CAP_INSTRUCTION,
         );
     }
@@ -4437,7 +4437,7 @@ mod tests {
                 HSetting::new(HSettingType::MaxTableCapacity, 100),
                 HSetting::new(HSettingType::MaxHeaderListSize, 10000),
             ],
-            &Http3State::Closing(ConnectionError::Application(265)),
+            &Http3State::Closing(CloseReason::Application(265)),
             ENCODER_STREAM_DATA_WITH_CAP_INSTRUCTION,
         );
     }
@@ -4474,7 +4474,7 @@ mod tests {
                 HSetting::new(HSettingType::BlockedStreams, 100),
                 HSetting::new(HSettingType::MaxHeaderListSize, 10000),
             ],
-            &Http3State::Closing(ConnectionError::Application(514)),
+            &Http3State::Closing(CloseReason::Application(514)),
             ENCODER_STREAM_DATA_WITH_CAP_INSTRUCTION,
         );
     }
@@ -4493,7 +4493,7 @@ mod tests {
                 HSetting::new(HSettingType::BlockedStreams, 100),
                 HSetting::new(HSettingType::MaxHeaderListSize, 10000),
             ],
-            &Http3State::Closing(ConnectionError::Application(265)),
+            &Http3State::Closing(CloseReason::Application(265)),
             ENCODER_STREAM_DATA_WITH_CAP_INSTRUCTION,
         );
     }
@@ -4531,7 +4531,7 @@ mod tests {
                 HSetting::new(HSettingType::BlockedStreams, 50),
                 HSetting::new(HSettingType::MaxHeaderListSize, 10000),
             ],
-            &Http3State::Closing(ConnectionError::Application(265)),
+            &Http3State::Closing(CloseReason::Application(265)),
             ENCODER_STREAM_DATA_WITH_CAP_INSTRUCTION,
         );
     }
@@ -4569,7 +4569,7 @@ mod tests {
                 HSetting::new(HSettingType::BlockedStreams, 100),
                 HSetting::new(HSettingType::MaxHeaderListSize, 5000),
             ],
-            &Http3State::Closing(ConnectionError::Application(265)),
+            &Http3State::Closing(CloseReason::Application(265)),
             ENCODER_STREAM_DATA_WITH_CAP_INSTRUCTION,
         );
     }
@@ -4626,7 +4626,7 @@ mod tests {
                 HSetting::new(HSettingType::BlockedStreams, 100),
                 HSetting::new(HSettingType::MaxHeaderListSize, 10000),
             ],
-            &Http3State::Closing(ConnectionError::Application(265)),
+            &Http3State::Closing(CloseReason::Application(265)),
             ENCODER_STREAM_DATA_WITH_CAP_INSTRUCTION,
         );
     }

neqo-http3/src/features/extended_connect/tests/webtransport/negotiation.rs

@@ -8,7 +8,7 @@ use std::time::Duration;
 
 use neqo_common::{event::Provider, Encoder};
 use neqo_crypto::AuthenticationStatus;
-use neqo_transport::{Connection, ConnectionError, StreamType};
+use neqo_transport::{CloseReason, Connection, StreamType};
 use test_fixture::{default_server_h3, now};
 
 use super::{connect, default_http3_client, default_http3_server, exchange_packets};
@@ -270,10 +270,7 @@ fn wrong_setting_value() {
     exchange_packets2(&mut client, &mut server);
     match client.state() {
         Http3State::Closing(err) | Http3State::Closed(err) => {
-            assert_eq!(
-                err,
-                ConnectionError::Application(Error::HttpSettings.code())
-            );
+            assert_eq!(err, CloseReason::Application(Error::HttpSettings.code()));
         }
         _ => panic!("Wrong state {:?}", client.state()),
     };

neqo-http3/src/server.rs

@@ -323,7 +323,7 @@ mod tests {
     use neqo_crypto::{AuthenticationStatus, ZeroRttCheckResult, ZeroRttChecker};
     use neqo_qpack::{encoder::QPackEncoder, QpackSettings};
     use neqo_transport::{
-        Connection, ConnectionError, ConnectionEvent, State, StreamId, StreamType, ZeroRttState,
+        CloseReason, Connection, ConnectionEvent, State, StreamId, StreamType, ZeroRttState,
     };
     use test_fixture::{
         anti_replay, default_client, fixture_init, now, CountingConnectionIdGenerator,
@@ -366,7 +366,7 @@ mod tests {
     }
 
     fn assert_closed(hconn: &mut Http3Server, expected: &Error) {
-        let err = ConnectionError::Application(expected.code());
+        let err = CloseReason::Application(expected.code());
         let closed = |e| matches!(e, Http3ServerEvent::StateChange{ state: Http3State::Closing(e) | Http3State::Closed(e), .. } if e == err);
         assert!(hconn.events().any(closed));
     }

neqo-http3/tests/httpconn.rs

@@ -17,7 +17,7 @@ use neqo_http3::{
     Header, Http3Client, Http3ClientEvent, Http3OrWebTransportStream, Http3Parameters, Http3Server,
     Http3ServerEvent, Http3State, Priority,
 };
-use neqo_transport::{ConnectionError, ConnectionParameters, Error, Output, StreamType};
+use neqo_transport::{CloseReason, ConnectionParameters, Error, Output, StreamType};
 use test_fixture::*;
 
 const RESPONSE_DATA: &[u8] = &[0x61, 0x62, 0x63];
@@ -448,7 +448,7 @@ fn fetch_noresponse_will_idletimeout() {
             if let Http3ClientEvent::StateChange(state) = event {
                 match state {
                     Http3State::Closing(error_code) | Http3State::Closed(error_code) => {
-                        assert_eq!(error_code, ConnectionError::Transport(Error::IdleTimeout));
+                        assert_eq!(error_code, CloseReason::Transport(Error::IdleTimeout));
                         done = true;
                     }
                     _ => {}

neqo-transport/src/connection/mod.rs

@@ -58,7 +58,7 @@ use crate::{
     },
     tracking::{AckTracker, PacketNumberSpace, RecvdPackets, SentPacket},
     version::{Version, WireVersion},
-    AppError, ConnectionError, Error, Res, StreamId,
+    AppError, CloseReason, Error, Res, StreamId,
 };
 
 mod dump;
@@ -889,7 +889,7 @@ impl Connection {
             let msg = format!("{v:?}");
             #[cfg(not(debug_assertions))]
             let msg = "";
-            let error = ConnectionError::Transport(v.clone());
+            let error = CloseReason::Transport(v.clone());
             match &self.state {
                 State::Closing { error: err, .. }
                 | State::Draining { error: err, .. }
@@ -960,9 +960,7 @@ impl Connection {
         let pto = self.pto();
         if self.idle_timeout.expired(now, pto) {
             qinfo!([self], "idle timeout expired");
-            self.set_state(State::Closed(ConnectionError::Transport(
-                Error::IdleTimeout,
-            )));
+            self.set_state(State::Closed(CloseReason::Transport(Error::IdleTimeout)));
             return;
         }
 
@@ -1206,7 +1204,7 @@ impl Connection {
             qdebug!([self], "Stateless reset: {}", hex(&d[d.len() - 16..]));
             self.state_signaling.reset();
             self.set_state(State::Draining {
-                error: ConnectionError::Transport(Error::StatelessReset),
+                error: CloseReason::Transport(Error::StatelessReset),
                 timeout: self.get_closing_period_time(now),
             });
             Err(Error::StatelessReset)
@@ -1283,7 +1281,7 @@ impl Connection {
         } else {
             qinfo!([self], "Version negotiation: failed with {:?}", supported);
             // This error goes straight to closed.
-            self.set_state(State::Closed(ConnectionError::Transport(
+            self.set_state(State::Closed(CloseReason::Transport(
                 Error::VersionNegotiation,
             )));
             Err(Error::VersionNegotiation)
@@ -2213,7 +2211,7 @@ impl Connection {
             );
             builder.set_limit(limit);
         }
-        // ConnectionError::Application is only allowed at 1RTT.
+        // CloseReason::Application is only allowed at 1RTT.
         let sanitized = if space == PacketNumberSpace::ApplicationData {
             None
         } else {
@@ -2429,7 +2427,7 @@ impl Connection {
 
     /// Close the connection.
     pub fn close(&mut self, now: Instant, app_error: AppError, msg: impl AsRef<str>) {
-        let error = ConnectionError::Application(app_error);
+        let error = CloseReason::Application(app_error);
         let timeout = self.get_closing_period_time(now);
         if let Some(path) = self.paths.primary() {
             self.state_signaling.close(path, error.clone(), 0, msg);
@@ -2848,7 +2846,7 @@ impl Connection {
                         FRAME_TYPE_CONNECTION_CLOSE_TRANSPORT,
                     )
                 };
-                let error = ConnectionError::Transport(detail);
+                let error = CloseReason::Transport(detail);
                 self.state_signaling
                     .drain(Rc::clone(path), error.clone(), frame_type, "");
                 self.set_state(State::Draining {