Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(client): exit with non-zero on error #1786

Merged
merged 3 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
larseggert marked this conversation as resolved.
Show resolved Hide resolved
InternalError,
ConnectionRefused,
FlowControlError,
Expand Down