-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Introduce QUIC traits #3924
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
base: master
Are you sure you want to change the base?
Introduce QUIC traits #3924
Conversation
|
||
use bytes::Buf; | ||
|
||
// TODO: Should this be gated by an `http3` feature? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I have a feeling this should be gated by http3 feature because quic is apart of http3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some thought about things learned in h3 to the traits below.
If I understand the current proposal/implementation correctly it requires quic implementations to implement both traits if they want to be used with hyper and h3 alone right?
Using h3s traits publicly in hyper is not possible because of the stability guaranties right?
Would it be possible to impl rt::quic::Connection for F where F: h3::quic::Connection
or would i limit the possible changes in h3 to much?
/// Bidirectional streams that can be opened or accepted by this connection. | ||
type BidiStream: SendStream<B> + RecvStream; | ||
/// Errors that may occur opening or accepting streams. | ||
type Error; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
H3 needs some some more infomation on the error. Because of this the ConnectionErrorIncoming
type was created.
https://github.com/hyperium/h3/blob/94cb13f74979f3a0b6c4f98c3711abac8eb0435a/h3/src/quic.rs#L20-L35
This can be done with a trait or a specific type like in h3 atm.
The InternalError
variant allows h3 to react to errors happening inside of the trait implementations with a Connection error.
Also h3 reacts to some ApplicationErrors.
// Q: shorten to bidi? | ||
/// Accept a bidirection stream. | ||
fn poll_accept_bidirectional_stream( | ||
self: Pin<&mut Self>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pin
here probably requires Q
in the glue to be Boxed.
https://github.com/hyperium/hyper/pull/3925/files#diff-5d22d97a30a051ea5e00978de444e3df97844c87b4174d6256e601c85216b1b4R5.
It should be possible to change h3 so its traits can be Pin
as well hyperium/h3#298
fn poll_accept_recv_stream( | ||
self: Pin<&mut Self>, | ||
cx: &mut Context<'_>, | ||
) -> Poll<Result<Option<Self::RecvStream>, Self::Error>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Option
here and on poll_accept_bidirectional_stream
is not necessary, as the peer can always create a new stream until the connection is closed. Then Err
is returned.
In RecvStream::poll_data
this is dfferent because streams can either be reset or EOS.
This adds initial support for QUIC to hyper, a prerequisite for HTTP/3 support.
This adds a new module,
hyper::rt::quic
, and new traits which largely match what was originally proposed for the h3 crate. The idea is similar to the other transport traits inhyper::rt
, which is that a user can describe to hyper how their QUIC implementation works, and then hyper will be able to use that for HTTP/3.While the
h3
crate has drifted some from the original proposal, I wasn't sure the additional complexity that had been added was necessary to expose at the hyper level. If during the implementation process it proves we do need it, we can make the change.These traits are put behind a
cfg(hyper_unstable_quic)
flag, until they are stabilized.This should likely stay as a PR proposal until a separate PR is ready to merge that integrates these traits with HTTP/3 glue in
hyper::proto::h3
.Please keep comments to implementation details.
cc #1818