Skip to content

Commit

Permalink
refactor: rename ConnectionError to CloseReason
Browse files Browse the repository at this point in the history
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 mozilla#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;
```
  • Loading branch information
mxinden committed May 3, 2024
1 parent 87bf852 commit 2db4096
Show file tree
Hide file tree
Showing 25 changed files with 122 additions and 119 deletions.
6 changes: 3 additions & 3 deletions neqo-bin/src/client/http09.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Connection, CloseReason, ConnectionEvent, EmptyConnectionIdGenerator, Error, Output, State,
StreamId, StreamType,
};
use url::Url;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
}

Expand Down
6 changes: 3 additions & 3 deletions neqo-bin/src/client/http3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ 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,
AppError, Connection, CloseReason, EmptyConnectionIdGenerator, Error as TransportError,
Output, StreamId,
};
use url::Url;
Expand Down Expand Up @@ -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 {
Expand All @@ -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()
}

Expand Down
12 changes: 6 additions & 6 deletions neqo-bin/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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),
}
}
}
Expand Down Expand Up @@ -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;
}

Expand Down
12 changes: 6 additions & 6 deletions neqo-http3/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, Connection, CloseReason, DatagramTracking, State, StreamId,
StreamType, ZeroRttState,
};

Expand Down Expand Up @@ -81,11 +81,11 @@ 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
/// - 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
/// - 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.
Expand All @@ -95,8 +95,8 @@ pub enum Http3State {
ZeroRtt,
Connected,
GoingAway(StreamId),
Closing(ConnectionError),
Closed(ConnectionError),
Closing(CloseReason),
Closed(CloseReason),
}

impl Http3State {
Expand Down Expand Up @@ -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");
}
Expand Down
18 changes: 9 additions & 9 deletions neqo-http3/src/connection_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1291,7 +1291,7 @@ mod tests {
use neqo_crypto::{AllowZeroRtt, AntiReplay, ResumptionToken};
use neqo_qpack::{encoder::QPackEncoder, QpackSettings};
use neqo_transport::{
ConnectionError, ConnectionEvent, ConnectionParameters, Output, State, StreamId,
CloseReason, ConnectionEvent, ConnectionParameters, Output, State, StreamId,
StreamType, Version, RECV_BUFFER_SIZE, SEND_BUFFER_SIZE,
};
use test_fixture::{
Expand All @@ -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()),
};
Expand Down Expand Up @@ -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,
);
}
Expand All @@ -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,
);
}
Expand Down Expand Up @@ -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,
);
}
Expand All @@ -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,
);
}
Expand Down Expand Up @@ -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,
);
}
Expand Down Expand Up @@ -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,
);
}
Expand Down Expand Up @@ -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,
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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::{Connection, CloseReason, StreamType};
use test_fixture::{default_server_h3, now};

use super::{connect, default_http3_client, default_http3_server, exchange_packets};
Expand Down Expand Up @@ -272,7 +272,7 @@ fn wrong_setting_value() {
Http3State::Closing(err) | Http3State::Closed(err) => {
assert_eq!(
err,
ConnectionError::Application(Error::HttpSettings.code())
CloseReason::Application(Error::HttpSettings.code())
);
}
_ => panic!("Wrong state {:?}", client.state()),
Expand Down
4 changes: 2 additions & 2 deletions neqo-http3/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Connection, CloseReason, ConnectionEvent, State, StreamId, StreamType, ZeroRttState,
};
use test_fixture::{
anti_replay, default_client, fixture_init, now, CountingConnectionIdGenerator,
Expand Down Expand Up @@ -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));
}
Expand Down
4 changes: 2 additions & 2 deletions neqo-http3/tests/httpconn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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;
}
_ => {}
Expand Down
16 changes: 8 additions & 8 deletions neqo-transport/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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, .. }
Expand Down Expand Up @@ -960,7 +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(
self.set_state(State::Closed(CloseReason::Transport(
Error::IdleTimeout,
)));
return;
Expand Down Expand Up @@ -1206,7 +1206,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)
Expand Down Expand Up @@ -1283,7 +1283,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)
Expand Down Expand Up @@ -2213,7 +2213,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 {
Expand Down Expand Up @@ -2429,7 +2429,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);
Expand Down Expand Up @@ -2848,7 +2848,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 {
Expand Down
Loading

0 comments on commit 2db4096

Please sign in to comment.