hyper icon indicating copy to clipboard operation
hyper copied to clipboard

Extract `TcpConnector` and `Connector` trait from hyper

Open MOZGIII opened this issue 6 years ago • 11 comments

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 avatar Dec 13 '19 20:12 MOZGIII

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.

djc avatar Dec 14 '19 08:12 djc

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.

MOZGIII avatar Dec 14 '19 16:12 MOZGIII

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>?

seanmonstar avatar Dec 16 '19 20:12 seanmonstar

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.

MOZGIII avatar Dec 16 '19 20:12 MOZGIII

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.

djc avatar Dec 16 '19 20:12 djc

@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?

LucioFranco avatar Dec 16 '19 21:12 LucioFranco

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 avatar Dec 16 '19 22:12 MOZGIII

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

MOZGIII avatar Dec 16 '19 22:12 MOZGIII

@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).

djc avatar Dec 17 '19 12:12 djc

So any news on QUIC support?

alikor avatar Mar 17 '23 12:03 alikor

@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?)

djc avatar Mar 17 '23 12:03 djc