Design portable fetch transport API - #693
martin-kolinek wants to merge 11 commits into
Conversation
| ```rust,ignore | ||
| pub trait Transport: Send + Sync + 'static { | ||
| fn build( | ||
| self: Box<Self>, |
There was a problem hiding this comment.
| self: Box<Self>, | |
| &self, |
why not like this? The transport should still stay boxeable
There was a problem hiding this comment.
🤖 Agreed that the trait should not expose boxing. I changed Transport to Clone + Send + Sync with validate(self, ...), and HttpClient::builder<T: Transport> now performs type erasure internally through a cloneable construction closure. Each builder build consumes a fresh transport-configuration clone; no boxed receiver is required from implementors.
There was a problem hiding this comment.
🤖 Correction after further discussion: Transport should remain dyn-compatible. The design now stores Arc<dyn Transport> internally and uses validate(self: Arc<Self>, ...), which is an object-safe receiver. HttpClient::builder<T> performs the Arc erasure internally, while runtime-selected callers may pass an erased transport directly. This avoids both Box<Self> and clone_box while allowing the factory to retain owned configuration.
There was a problem hiding this comment.
🤖 Final correction: your original &self suggestion is the cleaner contract. Transport::validate(&self, ...) is dyn-compatible and treats the transport as reusable immutable composition configuration. It returns a TransportFactory owning independent validated client state. HttpClient::builder<T> boxes internally, and builder_erased(Box<dyn Transport>) is only the explicit dynamic-construction boundary; neither Box nor Arc appears in the trait receiver.
| pub fn configure(builder: HttpClientBuilder) -> Result<HttpClient> { | ||
| builder | ||
| .connection_lifetime(LIFETIME) | ||
| .client_certificate(ClientCredentialId::new("service-client")) |
There was a problem hiding this comment.
imho, all tls specific configuration should have tls_ prefix to keep clean (reqwest did the same)
There was a problem hiding this comment.
🤖 Applied this to the broad portable builder: the methods are now tls_client_credential(...) and tls_server_name(...). I kept names concise inside TLS-specific composition builders such as fetch_hyper_rustls, where the crate/type scope already establishes that the mechanisms are TLS-specific.
Design a transport-erased HttpClientBuilder around portable library requirements, move backend mechanisms out of the public surface, and document named client credentials and TLS endpoint identity mapping. Add executable WinHTTP probes for independent routing/TLS/authority control and Nagle behavior to ground the design in measured backend capabilities. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c1d47b6-8039-4748-972d-20b238499d82
Require every supported transport to implement the complete portable baseline, add typed pre-build extensions for deliberate backend coupling, and make unsupported certificate policies explicit portable non-goals. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c1d47b6-8039-4748-972d-20b238499d82
Define fetch_hyper as a reusable TLS-neutral HTTP engine and move rustls/native-tls connector construction into dependency-isolated composition crates. Limit fetch stabilization to portable requirements and the typed config registry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c1d47b6-8039-4748-972d-20b238499d82
Rename the proposed TLS-neutral engine to fetch_hyper_common and retain fetch_hyper only when referring to the current crate being split into common and TLS composition crates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c1d47b6-8039-4748-972d-20b238499d82
Give portable HTTP/1.1 and HTTP/2 requirements precedence over WinHTTP's HTTP/3 preference, place response decompression in fetch, and define fallible trailer semantics. Add a direct WinHTTP probe demonstrating known- and unknown-length full-duplex HTTP/2 streaming and revise the implementation design around independent send and receive lanes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c1d47b6-8039-4748-972d-20b238499d82
Reconcile the portable transport design with the merged WinHTTP implementation and current fetch APIs. Model validation plus lazy materialization, clarify migration gaps, retain dependency-isolated probes, and remove accidental ring activation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c1d47b6-8039-4748-972d-20b238499d82
Separate target stabilization behavior from current implementation state, make transport configuration cloneable before validation, define UploadCompletion, and require platform qualification for WinHTTP duplex support. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c1d47b6-8039-4748-972d-20b238499d82
a6310b1 to
2a23226
Compare
Keep Transport generic and cloneable with a by-value validation method. Move boxing and type erasure behind HttpClient::builder so transport implementations do not expose boxed receivers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c1d47b6-8039-4748-972d-20b238499d82
Prefix portable HttpClientBuilder TLS requirements while keeping composition-scoped TLS mechanisms concise. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c1d47b6-8039-4748-972d-20b238499d82
Store unbuilt transports as Arc<dyn Transport> and validate through an owned Arc receiver. This preserves runtime transport selection without exposing Box<Self> or clone_box APIs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c1d47b6-8039-4748-972d-20b238499d82
Keep Transport dyn-compatible without encoding Box or Arc ownership in its receiver. Validation borrows reusable composition configuration and returns an independently owned factory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c1d47b6-8039-4748-972d-20b238499d82
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #693 +/- ##
=======================================
Coverage 100.0% 100.0%
=======================================
Files 640 639 -1
Lines 85528 85484 -44
=======================================
- Hits 85528 85484 -44
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🤖 Defines the proposed stable API boundary for
fetchbefore implementation work begins.Summary
HttpClientBuilderaround portable library requirementsfetch_hyper_commonas a reusable TLS-neutral HTTP enginefetch_hyper_rustlsandfetch_hyper_native_tlscratesBackend evidence
:authorityTCP_NODELAYcontrolTuning policy
TCP_NODELAY