Skip to content

Commit

Permalink
refactor!: Treat unexpected messages as serialization errors
Browse files Browse the repository at this point in the history
The semantics of this are a bit questionable, but it allows us to remove
the `RecvNextError` and `UnexpectedMessageType` types. Essentially, we
treat the problem of receiving an unexpected message as if we've tried
to deserialize the expected message, and found something else. This
doesn't quite match the logic (we in fact *do* deserialize a message
successfully), but is suitable for the API.

BREAKING CHANGE: The `RecvNextError` and `UnexpectedMessageType` types
have been removed. `RecvError` is used where `RecvNextError` was used
previously.
  • Loading branch information
Chris Connelly authored and connec committed Aug 27, 2021
1 parent 5bf79c1 commit e0695c3
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 43 deletions.
6 changes: 3 additions & 3 deletions src/connections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::Endpoint;

use super::{
connection_pool::{ConnId, ConnectionPool, ConnectionRemover},
error::{ConnectionError, Error, RecvError, RecvNextError, Result, SendError},
error::{ConnectionError, Error, RecvError, Result, SendError, SerializationError},
wire_msg::WireMsg,
};
use bytes::Bytes;
Expand Down Expand Up @@ -104,10 +104,10 @@ impl RecvStream {
}

/// Read next user message from the stream
pub async fn next(&mut self) -> Result<Bytes, RecvNextError> {
pub async fn next(&mut self) -> Result<Bytes, RecvError> {
match self.next_wire_msg().await? {
WireMsg::UserMsg(bytes) => Ok(bytes),
msg => Err(RecvNextError::UnexpectedMessageType(msg.into())),
msg => Err(SerializationError::unexpected(msg).into()),
}
}

Expand Down
10 changes: 3 additions & 7 deletions src/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use super::{
listen_for_incoming_connections, listen_for_incoming_messages, Connection,
DisconnectionEvents, RecvStream, SendStream,
},
error::{ClientEndpointError, ConnectionError, RecvNextError, Result, SendError},
error::{ClientEndpointError, ConnectionError, Result, SendError, SerializationError},
};
use bytes::Bytes;
use std::net::SocketAddr;
Expand Down Expand Up @@ -195,9 +195,7 @@ impl<I: ConnId> Endpoint<I> {
"Unexpected message when verifying public endpoint: {}",
other
);
return Err(Error::Recv(RecvNextError::UnexpectedMessageType(
other.into(),
)));
return Err(Error::Recv(SerializationError::unexpected(other).into()));
}
Ok(Err(err)) => {
error!("Error while verifying Public IP Address");
Expand Down Expand Up @@ -527,9 +525,7 @@ impl<I: ConnId> Endpoint<I> {
send_stream.send(WireMsg::EndpointEchoReq).await?;
match WireMsg::read_from_stream(&mut recv_stream.quinn_recv_stream).await {
Ok(WireMsg::EndpointEchoResp(socket_addr)) => Ok(socket_addr),
Ok(msg) => Err(Error::Recv(RecvNextError::UnexpectedMessageType(
msg.into(),
))),
Ok(msg) => Err(Error::Recv(SerializationError::unexpected(msg).into())),
Err(err) => Err(err.into()),
}
});
Expand Down
39 changes: 9 additions & 30 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ pub enum Error {
Send(#[from] SendError),
/// Failed to receive message.
#[error("Failed to receive message")]
Recv(#[from] RecvNextError),
Recv(#[from] RecvError),
}

impl From<quinn::ConnectionError> for Error {
Expand All @@ -85,12 +85,6 @@ impl From<quinn::ConnectionError> for Error {
}
}

impl From<RecvError> for Error {
fn from(error: RecvError) -> Self {
Self::Recv(error.into())
}
}

/// Errors returned by [`Endpoint::new_client`](crate::Endpoint::new_client).
#[derive(Debug, Error)]
pub enum ClientEndpointError {
Expand Down Expand Up @@ -337,18 +331,6 @@ impl From<quinn::WriteError> for SendError {
}
}

/// Errors that can be returned by [`RecvStream::next`](crate::RecvStream::next).
#[derive(Debug, Error)]
pub enum RecvNextError {
/// Failed to receive any message.
#[error(transparent)]
Recv(#[from] RecvError),

/// The type of message received is not the expected one.
#[error(transparent)]
UnexpectedMessageType(UnexpectedMessageType),
}

/// Errors that can occur when receiving messages.
#[derive(Debug, Error)]
pub enum RecvError {
Expand Down Expand Up @@ -407,6 +389,14 @@ impl SerializationError {
pub(crate) fn new(message: impl ToString) -> Self {
Self(bincode::ErrorKind::Custom(message.to_string()).into())
}

/// Construct a `SerializationError` for an unexpected message.
pub(crate) fn unexpected(actual: WireMsg) -> Self {
Self::new(format!(
"The message received was not the expected one: {}",
actual
))
}
}

/// Errors that can occur when interacting with streams.
Expand All @@ -432,14 +422,3 @@ pub enum StreamError {
#[derive(Debug, Error)]
#[error(transparent)]
pub struct UnsupportedStreamOperation(Box<dyn std::error::Error + Send + Sync>);

/// The type of message received is not the expected one.
#[derive(Debug, Error)]
#[error("The of the message received was not the expected one: {0}")]
pub struct UnexpectedMessageType(WireMsg);

impl From<WireMsg> for UnexpectedMessageType {
fn from(msg: WireMsg) -> Self {
UnexpectedMessageType(msg)
}
}
5 changes: 2 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,8 @@ pub use connection_pool::ConnId;
pub use connections::{DisconnectionEvents, RecvStream, SendStream};
pub use endpoint::{Endpoint, IncomingConnections, IncomingMessages};
pub use error::{
ClientEndpointError, Close, ConnectionError, Error, InternalConfigError, RecvError,
RecvNextError, Result, SendError, SerializationError, StreamError, TransportErrorCode,
UnexpectedMessageType, UnsupportedStreamOperation,
ClientEndpointError, Close, ConnectionError, Error, InternalConfigError, RecvError, Result,
SendError, SerializationError, StreamError, TransportErrorCode, UnsupportedStreamOperation,
};

#[cfg(test)]
Expand Down

0 comments on commit e0695c3

Please sign in to comment.