Repository navigation
Let HttpClient and Http2Connection take a custom DNS resolver - #341
Conversation
The built-in connector resolves host names with hyper-util's GaiResolver: one uncached getaddrinfo call on tokio's blocking pool for every new connection. An HTTP/1.1 pool with no idle connection opens one connection per in-flight request, so a burst of requests becomes a burst of blocking jobs, and the pool starts a thread for each job that finds none idle. The only way to change the resolver was to replace HttpClient with a hand-built hyper client behind ServiceTransport. That is a different type, and it loses HttpClient's defaults. Add a Resolve trait, and dns_resolver(Arc<dyn Resolve>) on HttpClientBuilder and Http2ConnectionBuilder, in the spirit of reqwest's ClientBuilder::dns_resolver. Callers can cache answers, coalesce concurrent lookups, use an async DNS client, or pin names in tests. The resolver is type-erased in a private enum that sits in HttpConnector<R>'s resolver slot, so HttpClient and Http2Connection stay single concrete types and no existing signature changes. With no custom resolver the enum forwards to GaiResolver, at the cost of one boxed future and one Vec per new connection. The lookup stays inside establishment_timeout, IP-literal hosts still skip resolution, and a resolver error surfaces as unavailable like any other connect failure. The builders keep their auto traits. Signed-off-by: Richard Beddington <[email protected]>
Behaviour:
- An empty answer from a custom resolver now fails the lookup
("resolver returned no addresses for <host>") instead of reaching hyper,
which reports it as "tcp connect error" / "Network unreachable".
Docs on Resolve::resolve now state what an implementation can rely on:
- when a non-zero port is dialled (only when the URI has no explicit port);
- how the connector orders addresses (by the first address's family, not
strictly in the returned order) and what local_address does to them;
- that host arrives as written in the URI, that resolve must not block,
when the future is dropped, and that establishment_timeout is the only
bound on the lookup;
- which error each transport returns: SharedHttp2Connection returns a
ConnectError from the resolver with its own code, while HttpClient and the
connect_* terminals always return unavailable;
- that a panic leaves an Http2Connection unable to recover;
- that TLS server name and certificate verification still use the URI host.
The example now uses host, which shows the owned copy that the 'static
future needs. The lists of settings that custom connectors ignore, the
tcp_connect_timeout docs (the division is per address family) and the guide
mention the resolver.
Tests: the two port cases, the empty answer, the source chain, the error
code on each transport, TLS through with_tls and connect_tls against a
certificate that names only the URI host, a hung resolver and a reconnect on
Http2Connection, a custom connector, a resolver panic, and the builders'
Debug output.
Signed-off-by: Iain McGinniss <[email protected]>
|
[claude code] Thanks for this. The design is the right one: with the erasure inside One behaviour changeAn empty What
|
| Topic | What the code does |
|---|---|
| Port | A non-zero port from the resolver is dialled only when the URI has no explicit port; an explicit URI port replaces it. This is hyper-util's set_port, so two tests pin it. |
| Order | "In order of preference" was not accurate for a mixed list: hyper dials the first address's family in order and starts the other family after its Happy Eyeballs delay. With local_address set it dials only that family. |
| Count | tcp_connect_timeout is divided across the addresses of each family, so a long list leaves each address a short timeout. The two tcp_connect_timeout docs said "across the address set" and now say "each family". |
host |
It arrives as written in the URI, so API.example.com and api.example.com are different keys unless the implementation normalises them. |
| Blocking and drop | resolve and its future run on the task that opens the connection, so they must not block. The future can be dropped at any .await, and establishment_timeout is the only bound on the lookup. |
| Errors | HttpClient and the connect_* terminals always return Unavailable. SharedHttp2Connection returns a ConnectError from the resolver as is: a resolver that returned permission_denied surfaced as PermissionDenied there. The resolver's error is in the source() chain and not in the message. |
| Panics | See the follow-up below. |
| TLS | The resolver chooses only the address to dial. The server name and certificate verification still use the URI host. |
The example now uses host. With _host it avoided the error that a real implementation meets first: the future is 'static, so using host inside async move fails with lifetime may not live long enough.
Smaller edits
- The lists of settings that the custom-connector and Unix-socket terminals ignore now include
dns_resolver, anddocs/guide.mdhas a paragraph on it. - The
UnwindSafecomment and test sayHttpClientBuilder.Http2ConnectionBuilderwas already!UnwindSafeonmain, through hyper'sArc<dyn Timer>. Your claim that no auto trait is lost holds for all five client types. - The changelog fragment names
client::Resolveandclient::Resolving, and its timestamp moved from 22 October to the commit's date. - We removed the reqwest comparison, because reqwest's trait has a different signature and a reqwest resolver does not compile here.
Tests added
The two port cases, the empty answer, the source() chain, the error code on each transport, a hung resolver and a second attempt on Http2Connection, a custom connector ignoring the resolver, a resolver panic, and the builders' Debug output. Two TLS tests (with_tls and connect_tls) pin the name to loopback against a certificate that names only resolver-test.invalid; both fail when the certificate names another host. TestResolver now takes a capturing closure and is shared with the http2 tests.
Locally on d164584: the workspace tests pass with --all-features (803 in the connectrpc lib), as do the client tests with only client (230) and only client-tls (247), --no-default-features, cargo doc with -D warnings, the pinned fmt, and cargo +1.88 check. Clippy passes on nightly-2026-02-27; the pinned 1.95 run is left to CI.
Left for follow-ups
- A resolver panic leaves an
Http2Connectionunable to recover.Reconnect::poll_readypolls the stored connect future in place, so after a panic every later attempt polls the same future again. With a resolver that panics once, four calls on aSharedHttp2Connectionreached the resolver once, and calls two to four failed withinternal: h2 connect task failed: ... `async fn` resumed after panicking. This predates your PR, and a custom connector can reach it too. We will fix it separately. For now the# Panicssection states it andconnection_does_not_recover_from_a_dns_resolver_panicpins it; the fix will update both. - The error code differs by transport. This PR documents and tests the difference. Making
HttpClientandconnect_*also return the resolver'sConnectErroris a separate decision. - Names.
ResolveandResolvingare bare names inconnectrpc::client. Aclient::dnsmodule is worth a thought before release, as is a closure adapter, which can be added later without a break.
The "Semantics" section of the PR description now differs from the code on ports and on the error code, so it needs an update before the squash.
What
Adds a way to replace the system DNS resolver in the built-in client transports:
connectrpc::client::Resolve:fn resolve(&self, host: &str) -> Resolving, whereResolvingis a boxed future ofVec<SocketAddr>.HttpClientBuilder::dns_resolver(Arc<dyn Resolve>), for all three flavours (plaintext,plaintext_http2_only,with_tls).Http2ConnectionBuilder::dns_resolver(Arc<dyn Resolve>), for the built-in connector. The custom-connector and Unix-socket terminals ignore it, as they ignoretcp_connect_timeout.Why
The built-in connector uses hyper-util's
GaiResolver: one uncachedgetaddrinfoon tokio's blocking pool for every new connection. An HTTP/1.1 pool with no idle connection opens one connection per in-flight request, so a burst of requests becomes a burst of blocking jobs, and the pool starts a thread for each job that finds none idle. A service that talks to a handful of stable names pays for the same lookup again and again, at the moment when it is busiest.Today the only way out is to build a hyper client by hand and wrap it in
ServiceTransport. That works, but it is a different type fromHttpClient, which hurts libraries that exposeT: ClientTransport = HttpClient. It also means a copy ofHttpClient's defaults (nodelay, the connect and establishment timeouts, the scheme checks, the error mapping) that has to be kept in step.With the hook a caller can cache answers, coalesce concurrent lookups, use an async DNS client, or pin names in tests. reqwest has the same thing as
ClientBuilder::dns_resolver.How
hyper-util already has the seam:
HttpConnector<R>::new_with_resolver. A private enum sits in that slot:Because the erasure is inside the connector,
HttpClientandHttp2Connectionstay single concrete types, and no existing signature changes. The public trait takes&strand returnsVec<SocketAddr>, so no hyper-util type enters the public API.For callers who don't set a resolver
GaiResolver), the same address order, and the same error source (io::Error).Vecper new connection (not per request).GaiFuture.dyn Resolvewould have costHttpClientBuilderitsUnwindSafeandRefUnwindSafewithout a word, so the private enum implements both, and a test pins it.Semantics
0, and the connector fills in the port from the URI or the scheme's default.establishment_timeout.ErrorCode::Unavailable, like any other connect failure.Tests
.invalidhost, which the system resolver can never resolve: once forHttpClient, once forHttp2Connection.Unavailable.establishment_timeoutbounds a resolver that never answers.HttpClientBuilderkeeps its auto traits.Resolve.Tested against the 0.9.1 release:
cargo test --lib client::with--features client,client-tls, the new tests again with--no-default-features --features client, the doc-test,cargo clippy --all-targets --all-features -- -D warningsandcargo doc --no-deps --all-featureswithRUSTDOCFLAGS="-D warnings". The same change passes the client tests on 0.8.2. It has not been run on main; this PR's CI is its first run there.Alternative considered
A general connector hook on
HttpClientBuilder, likeHttp2Connection::lazy_with_connector. It is more powerful, but the caller then has to rebuild nodelay and the TCP connect timeout, and for TLS the wholeHttpsConnector. The resolver is the part that callers most often want to change, and the two hooks don't exclude each other. I'm happy to switch to that shape if you prefer it.Compatibility
Additive only. The same change applies to the 0.8 and 0.9 release lines, and I can open back-port PRs if that helps.