Skip to content

Extract TcpConnector and Connector trait from hyper #2078

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

Open
MOZGIII opened this issue Dec 13, 2019 · 11 comments
Open

Extract TcpConnector and Connector trait from hyper #2078

MOZGIII opened this issue Dec 13, 2019 · 11 comments

Comments

@MOZGIII
Copy link

MOZGIII commented Dec 13, 2019

I feel like we need some crate (in-between tokio and hyper) to provide a layer of connectors.
In hyper everything's about http, but we do need a TcpConnector and a Connector trait to be available for better composability in our networking libraries.

The rationale is the "connecting" isn't just about HTTP, it's about all the connection-based protocols. It abstracts really nicely and allows us to really use the transport protocols interchangeably.

There's an emerging anti-pattern that I observed is a couple of codebases - to use HttpConnector to establish non-http TCP stream. I don't think it's right, and that's why I classify it as an anti-pattern. Unfortunately, even the authors do recommend this: #1445.

I propose we extract TcpConnector from HttpConnector, and keep the HttpConnector as a thin layer atop of TcpConnector, providing functionality like using URI as an argument (and enforce_http and so on). We can put the happy eyeballs and DNS resolution support in a layer between TcpConnector and HttpConnector (HttpConnector<HappyEyeballsConnector<TcpConnector, DomainResolver>>).

What do you think?

P.S. Maybe this is not the right place for this issue, but can start here and move somewhere else if needed.

@MOZGIII MOZGIII changed the title Extract TcpConnector and Connector trait from hyper. Extract TcpConnector and Connector trait from hyper Dec 13, 2019
@djc
Copy link
Contributor

djc commented Dec 14, 2019

Note that in the near future, we might want to support QUIC as an alternative to TCP. So any forward-looking attempt at this should take that into consideration.

FWIW I think tower's Service trait could be used for this use case given sensible arguments.

@MOZGIII
Copy link
Author

MOZGIII commented Dec 14, 2019

I'd say tower services is an abstraction that can be applied atop of the connectors to make them play nice with the tower infrastructure. I'm afraid not every connector can be easily described as one request-response interaction. The readiness can be a non-trivial thing, so we might it might not naturally fil into the service trait too. TCP, HTTP fit nicely though, but I'm not sure the same applies to TLS - I'd try to avoid sacrificing configurability to fit the service trait.

@seanmonstar
Copy link
Member

So the thinking we've had for a while is that the Service trait fits the pattern well (there's a MakeConnection trait).

I'm not sure the same applies to TLS - I'd try to avoid sacrificing configurability to fit the service trait.

What do you think doesn't fit into the pattern of async connect(Dst) -> Result<impl AsyncRead + AsyncWrite, Error>?

@MOZGIII
Copy link
Author

MOZGIII commented Dec 16, 2019

An example comes to mind: SOCKS5 authorization with interactive user credentials input. Here we need to split the connection into two phases: first the handshake, and then auth negotiation.
And, in general, I'm worried about things like this - where you need multiple phases to complete the connection.

@djc
Copy link
Contributor

djc commented Dec 16, 2019

What do you think doesn't fit into the pattern of async connect(Dst) -> Result<impl AsyncRead + AsyncWrite, Error>?

For one thing, that doesn't support QUIC.

@LucioFranco
Copy link
Member

@djc I feel like quic is a bit unique here compared to TCP in general. Not sure, how quic fits into the AsyncRead + AsyncWrite interface?

@MOZGIII
Copy link
Author

MOZGIII commented Dec 16, 2019

Don't anyone else think impl AsyncRead + AsyncWrite is a bit too strict? We'll probably make it a trait type, so why add more restrictions. Websocket for example also doesn't fit, but it's a connection-based protocol.

@MOZGIII
Copy link
Author

MOZGIII commented Dec 16, 2019

I figured the protocols that require connection and protocols that provide streaming read/write interface are separate sets, and they partially overlap.

@djc
Copy link
Contributor

djc commented Dec 17, 2019

@djc I feel like quic is a bit unique here compared to TCP in general. Not sure, how quic fits into the AsyncRead + AsyncWrite interface?

QUIC is different from TCP in this regard, yes. (I'm not sure if the word unique is useful here, since QUIC is intended to be a general purpose alternative to TCP.) QUIC connections indeed don't fit in with the AsyncRead + AsyncWrite world, since they conceptually provide a bundle of byte streams rather than a single byte stream (and QUIC byte streams can be unidirectional or bidirectional, but that's probably less important).

@alikor
Copy link

alikor commented Mar 17, 2023

So any news on QUIC support?

@djc
Copy link
Contributor

djc commented Mar 17, 2023

@alikor for HTTP/3 support in Hyper, #1818 is probably a better place to follow.

(Not sure what the role of this issue should be in the context of the impending 1.0 release?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants