Skip to content

Commit

Permalink
fix(client): exit with non-zero on error (#1786)
Browse files Browse the repository at this point in the history
* fix(client): exit with non-zero on error

When a connection closes with an error, surface the error to the user and exit
with non-zero.

* Trigger CI
  • Loading branch information
mxinden authored Apr 8, 2024
1 parent a65d945 commit 32a2a59
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 13 deletions.
11 changes: 7 additions & 4 deletions neqo-bin/src/client/http09.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ use std::{
use neqo_common::{event::Provider, qdebug, qinfo, qwarn, Datagram};
use neqo_crypto::{AuthenticationStatus, ResumptionToken};
use neqo_transport::{
Connection, ConnectionEvent, EmptyConnectionIdGenerator, Error, Output, State, StreamId,
StreamType,
Connection, ConnectionError, ConnectionEvent, EmptyConnectionIdGenerator, Error, Output, State,
StreamId, StreamType,
};
use url::Url;

Expand Down Expand Up @@ -149,8 +149,11 @@ impl super::Client for Connection {
self.close(now, app_error, msg);
}

fn is_closed(&self) -> bool {
matches!(self.state(), State::Closed(..))
fn is_closed(&self) -> Option<ConnectionError> {
if let State::Closed(err) = self.state() {
return Some(err.clone());
}
None
}

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

Expand Down Expand Up @@ -111,8 +112,11 @@ pub(crate) fn create_client(
}

impl super::Client for Http3Client {
fn is_closed(&self) -> bool {
matches!(self.state(), Http3State::Closed(..))
fn is_closed(&self) -> Option<ConnectionError> {
if let Http3State::Closed(err) = self.state() {
return Some(err);
}
None
}

fn process(&mut self, dgram: Option<&Datagram>, now: Instant) -> Output {
Expand Down
26 changes: 22 additions & 4 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, ConnectionId, Error as TransportError, Version};
use neqo_transport::{AppError, ConnectionError, ConnectionId, Error as TransportError, Version};
use qlog::{events::EventImportance, streamer::QlogStreamer};
use tokio::time::Sleep;
use url::{Origin, Url};
Expand All @@ -46,6 +46,7 @@ pub enum Error {
IoError(io::Error),
QlogError,
TransportError(neqo_transport::Error),
ApplicationError(neqo_transport::AppError),
CryptoError(neqo_crypto::Error),
}

Expand Down Expand Up @@ -79,6 +80,15 @@ impl From<neqo_transport::Error> for Error {
}
}

impl From<neqo_transport::ConnectionError> for Error {
fn from(err: neqo_transport::ConnectionError) -> Self {
match err {
ConnectionError::Transport(e) => Self::TransportError(e),
ConnectionError::Application(e) => Self::ApplicationError(e),
}
}
}

impl Display for Error {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "Error: {self:?}")?;
Expand Down Expand Up @@ -371,7 +381,11 @@ trait Client {
fn close<S>(&mut self, now: Instant, app_error: AppError, msg: S)
where
S: AsRef<str> + Display;
fn is_closed(&self) -> bool;
/// Returns [`Some(_)`] if the connection is closed.
///
/// Note that connection was closed without error on
/// [`Some(ConnectionError::Transport(TransportError::NoError))`].
fn is_closed(&self) -> Option<ConnectionError>;
fn stats(&self) -> neqo_transport::Stats;
}

Expand Down Expand Up @@ -406,11 +420,15 @@ impl<'a, H: Handler> Runner<'a, H> {

self.process(None).await?;

if self.client.is_closed() {
if let Some(reason) = self.client.is_closed() {
if self.args.stats {
qinfo!("{:?}", self.client.stats());
}
return Ok(self.handler.take_token());
return match reason {
ConnectionError::Transport(TransportError::NoError)
| ConnectionError::Application(0) => Ok(self.handler.take_token()),
_ => Err(reason.into()),
};
}

match ready(self.socket, self.timeout.as_mut()).await? {
Expand Down
4 changes: 2 additions & 2 deletions neqo-transport/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ const ERROR_AEAD_LIMIT_REACHED: TransportError = 15;
#[derive(Clone, Debug, PartialEq, PartialOrd, Ord, Eq)]
pub enum Error {
NoError,
// Each time tihe error is return a different parameter is supply.
// This will be use to distinguish each occurance of this error.
// Each time this error is returned a different parameter is supplied.
// This will be used to distinguish each occurance of this error.
InternalError,
ConnectionRefused,
FlowControlError,
Expand Down

0 comments on commit 32a2a59

Please sign in to comment.