Decompose Client into utils

hyperium / hyper

An HTTP library for Rust

https://hyper.rs

MIT License

14.57k stars 1.6k forks source link

Decompose Client into utils #3080

Open seanmonstar opened 1 year ago

seanmonstar commented 1 year ago

Besides a connector and pool, the Client from 0.14.x did many small things that should be decomposed into pieces in hyper-util, so users can choose to compose the parts they want.

[ ] Create a SetHost service layer (could also fit in tower-http) (0.14.x code)
[ ] Create a Http1RequestTarget service layer (0.14.x code)
[ ] Create a DelayedRelease service layer (0.14.x code)
[ ] Create an AutoConnection which can wrap HTTP/1 and HTTP/2 (and eventually 3) connections

seanmonstar commented 1 year ago

@davidpdrsn hey! any thoughts about some of these things moving to tower-http? Or should they stay as hyper-util? Or a 3rd option?

tomkarw commented 1 year ago

Is this issue up to date? If yes, I'd be happy to pick it up.

As for the tower-http part, I don't think it's a blocker for now, we can always move that part of code to a different crate once it's extracted.

seanmonstar commented 1 year ago

I believe so, yea!

tomkarw commented 1 year ago

@seanmonstar How should we deal with information that was available in the broad context of send_request function, but won't be available now in the narrower context of the middleware?

Both Http1RequestTarget and DelayedResponse requires pooled (some context about the already established connection) which is not trivially available inService middleware.

seanmonstar commented 1 year ago

@tomkarw Do you mind expanding on what you think is needed? Such as the is_proxied part? I've been thinking that could go somewhere else and not be part of the connector anymore. Possibly a request extension...

tomkarw commented 9 months ago

Current v0.14.x Client had following logic: if the connection info indicated that the request is being proxied, and the schema was exactly HTTPS, we normalize the URI to the origin form (stripping the host). When making Http1RequestTarget a Service, we need to work with an interface that only exposes Request. We loose this tight coupling with the connection pool and thus connection info.

We need to decide whether we:

remove the proxy case, potentially breaking proxied requests
somehow sneak the connection info as part of the Request (like in the extentions?)
do what's currently in the PR, which is to set is_proxied when instanciating the Service, but that doesn't work, as the is_proxied per-connection (and thus per-request) information and should be determined separetly for each request.