Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 2 additions & 0 deletions quinn-proto/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2605,6 +2605,7 @@ impl Connection {
code: TransportErrorCode::crypto(0x6d),
frame: None,
reason: "transport parameters missing".into(),
crypto: None,
})?;

if self.has_0rtt() {
Expand Down Expand Up @@ -2678,6 +2679,7 @@ impl Connection {
code: TransportErrorCode::crypto(0x6d),
frame: None,
reason: "transport parameters missing".into(),
crypto: None,
})?;
self.handle_peer_params(params)?;
self.issue_first_cids(now);
Expand Down
1 change: 1 addition & 0 deletions quinn-proto/src/crypto/rustls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ impl crypto::Session for TlsSession {
code: TransportErrorCode::crypto(alert.into()),
frame: None,
reason: e.to_string(),
crypto: Some(Arc::new(e)),
}
} else {
TransportError::PROTOCOL_VIOLATION(format!("TLS error: {e}"))
Expand Down
29 changes: 27 additions & 2 deletions quinn-proto/src/transport_error.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::fmt;
use std::{fmt, sync::Arc};

use bytes::{Buf, BufMut};

Expand All @@ -8,16 +8,27 @@ use crate::{
};

/// Transport-level errors occur when a peer violates the protocol specification
#[derive(Debug, Clone, Eq, PartialEq)]
#[derive(Debug, Clone)]
#[non_exhaustive]
pub struct Error {
/// Type of error
pub code: Code,
/// Frame type that triggered the error
pub frame: Option<frame::FrameType>,
/// Human-readable explanation of the reason
pub reason: String,
/// An underlying crypto (e.g. TLS) layer error
pub crypto: Option<Arc<dyn std::error::Error + Send + Sync>>,
}

impl PartialEq for Error {
fn eq(&self, other: &Self) -> bool {
self.code == other.code
}
}

impl Eq for Error {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should ditch this impl outright? Users should probably only be performing comparisons on code anyway.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would mean dropping PartialEq, Eq from the ConnectionError here https://github.com/quinn-rs/quinn/blob/main/quinn-proto/src/connection/mod.rs#L3917, or converting it to an ugly manual impl. Perhaps we should keep the comparison here, but only for the code field?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment seems to be referencing Eq but not PartialEq. I think it'd be okay to drop Eq from ConnectionError, but we should probably try to preserve PartialEq.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I assumed differently, as in that case, there's little difference between Eq and PartialEq. @Ralith can you clarify what you meant?

Users should probably only be performing comparisons on code anyway.

Regarding this, I rewrote the PartialEq comparison to include only code. Is that OK?


impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.code.fmt(f)?;
Expand All @@ -39,6 +50,19 @@ impl From<Code> for Error {
code: x,
frame: None,
reason: "".to_string(),
crypto: None,
}
}
}

impl Error {
/// Construct an error with a code and a reason
pub fn new(code: Code, reason: String) -> Self {
Self {
code,
frame: None,
reason,
crypto: None,
}
}
}
Expand Down Expand Up @@ -79,6 +103,7 @@ macro_rules! errors {
code: Code::$name,
frame: None,
reason: reason.into(),
crypto: None,
}
}
)*
Expand Down
9 changes: 4 additions & 5 deletions quinn/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1074,11 +1074,10 @@ impl State {
self.close(error_code, reason, shared);
}
Poll::Ready(None) => {
return Err(ConnectionError::TransportError(proto::TransportError {
code: proto::TransportErrorCode::INTERNAL_ERROR,
frame: None,
reason: "endpoint driver future was dropped".to_string(),
}));
return Err(ConnectionError::TransportError(proto::TransportError::new(
proto::TransportErrorCode::INTERNAL_ERROR,
"endpoint driver future was dropped".to_string(),
)));
}
Poll::Pending => {
return Ok(());
Expand Down