Add NewConnectedRoundTripper for race-aware callers#6
Merged
Conversation
Client.RoundTripper() returns a lazy http.RoundTripper — it doesn't touch the network until RoundTrip is called. That works for standalone callers but defeats race-based transports (e.g. getlantern/kindling's race transport) that expect their transports to block until a connection is actually ready: a lazy wrapper "connects" instantly and always wins the race, hiding failures and ordering requests by insertion rather than by real readiness. Add NewConnectedRoundTripper(ctx, addr) which blocks until a front is taken from the pool AND the TLS handshake to that front completes. Returns a one-shot RoundTripper bound to that specific connection; if the request fails, the front is marked failed; on success it's requeued. addr is accepted for API parity with other "connect to addr" transports but is ignored — domainfront always dials the front chosen by the pool. Integration test: fetches /ping on borda.lantern.io through the returned RT. Passes against real CloudFront infrastructure (status=202).
myleshorton
added a commit
to getlantern/kindling
that referenced
this pull request
Apr 19, 2026
Two review comments on #31: 1. (Copilot, kindling.go:176) WithDomainFronting returned a cached http.RoundTripper immediately without doing any connection work, which defeats raceTransport's "connect in parallel, send serially by real readiness" design — the fronted transport would always "win" the race before anyone else had done real network work. Switch to the new domainfront.Client.NewConnectedRoundTripper (getlantern/domainfront#6), which blocks until a front is taken from the pool and the TLS handshake completes. Each race attempt now gets a one-shot RT bound to a real pre-dialed connection. 2. (Copilot, kindling_test.go:210) TestLantern_DomainFronting asserted a non-empty response body, but a successful fronted request can legitimately have an empty body (204/304/405-with-no-body). Drop the body-length assertion — status-code assertions are the actual signal that domain fronting worked. Verified against real CDN infrastructure: KINDLING_INTEGRATION=1 go test -run TestLantern_DomainFronting → PASS (405 from config.getiantem.org through the pre-connected RT).
3 tasks
There was a problem hiding this comment.
Pull request overview
This PR introduces a new API, Client.NewConnectedRoundTripper(ctx, addr), to support race-based callers by returning a RoundTripper that only becomes available after a real TLS connection to a selected front has been established.
Changes:
- Adds
Client.NewConnectedRoundTripperplus aconnectedRoundTripperimplementation that reuses existing request rewrite/body buffering/doRequest logic. - Adds an integration test validating that
NewConnectedRoundTripperblocks until a TLS-connected front is available and can complete a real/pingrequest via CloudFront.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| connected.go | Adds the new connected, one-shot RoundTripper API and implementation. |
| integration_test.go | Adds an integration test exercising the new API against a real endpoint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Four Copilot comments on #6: 1. (connected.go:72) connectedRoundTripper was documented as one-shot but nothing enforced it. A second RoundTrip call would reuse an already- closed conn and double-report success/failure to the pool, enqueueing the same front twice. Added an atomic.Bool `used` guard; the second call returns the new exported sentinel ErrRoundTripperUsed. 2. (connected.go:50) If the caller never called RoundTrip (e.g. a losing racer), the taken front and pre-dialed conn leaked until GC. Added io.Closer: Close returns the front to the pool (non-success) and closes the conn. Uses the same `used` guard so Close after RoundTrip is a no-op and double-Close is safe. 3. (connected.go:53) On retry exhaustion the last dial error was discarded and the caller got a generic "could not connect to any front". Now wraps the last error and reports the attempt count: "could not connect to any front after N attempts: <last err>". 4. (integration_test.go:195) io.ReadAll error was ignored, so a truncated body would silently look like success. Asserts require.NoError. Unit tests in connected_test.go cover: second RoundTrip returns ErrRoundTripperUsed; Close returns front + closes conn + is idempotent; RoundTrip with an unmapped host closes the conn exactly once and a subsequent Close is a no-op. Integration test still passes against real CloudFront.
myleshorton
added a commit
to getlantern/kindling
that referenced
this pull request
Apr 19, 2026
Moves from the adam/connected-roundtripper branch tag to the released main-branch SHA containing NewConnectedRoundTripper, Close, and the single-use guard (getlantern/domainfront#6).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds `Client.NewConnectedRoundTripper(ctx, addr)` that blocks until a front is actually dialed (TLS handshake complete) and returns a one-shot RoundTripper bound to that connection.
Motivation: `Client.RoundTripper()` returns a lazy wrapper — it doesn't touch the network until `RoundTrip` is called. That breaks race-based callers like getlantern/kindling's `raceTransport`, which expect transports to block on real readiness so the race reflects actual wire-level latency instead of insertion order. A lazy wrapper "connects" instantly and always wins the race, hiding failures.
API
```go
rt, err := client.NewConnectedRoundTripper(ctx, "")
// rt is now bound to a TLS-connected front; use it for exactly one request
resp, err := rt.RoundTrip(req)
```
Changes
Test plan
🤖 Generated with Claude Code