Cancel HTTP transport races when their timeout ends

[hawkff]

Summary

• bind direct transport attempts to the request timeout context
• cancel and join losing workers before returning
• cover winner, timeout, cancellation, and worker-exit paths

Validation

• Namespace CI run 29212350369 passed on rerun after a transient dependency download failure
• local read-only review reported no correctness findings

Greptile Summary

This PR updates the direct HTTP transport race cleanup. The main changes are:

• Adds a shared race helper for direct HTTP/ECH and H3 attempts.
• Gives each worker its own cancellation context.
• Cancels and waits for losing workers before returning a winner.
• Keeps the winning response context alive until the body closes.
• Adds tests for timeout, cancellation, winner, loser, panic, and failure paths.

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
libcore/http.go	Refactors direct transport racing so losing workers are cancelled and the winning response stays readable until its body closes.
libcore/http_test.go	Adds tests for worker cancellation, winner readability, timeout behavior, panic handling, joined failures, and non-OK responses.

_{Reviews (4): Last reviewed commit: "fix(http): cancel direct request race on..." | Re-trigger Greptile}

[coderabbitai]

📝 Walkthrough

Walkthrough

The change replaces direct H3 racing with a shared concurrent request helper that manages cancellation, response-body cleanup, first-success selection, and aggregated errors. doH3Direct uses the helper for ECH-over-HTTP(s) and H3 paths, with expanded concurrency tests.

Changes

HTTP Request Racing

Layer / File(s)	Summary
Shared request racing and lifecycle handling libcore/http.go, libcore/http_test.go	Concurrent labeled requests return the first 200 OK response, close failed or losing bodies, cancel workers when appropriate, preserve joined errors, and handle empty responses and panics. Tests cover these behaviors.
H3 direct strategy integration libcore/http.go	doH3Direct races ECH-over-HTTP(s) and H3 HTTPS requests with a ten-second timeout, using only the ECH path for http URLs.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant httpRequest
  participant raceHTTPRequests
  participant RequestWorkers
  participant ResponseBodies
  httpRequest->>raceHTTPRequests: start labeled request functions
  raceHTTPRequests->>RequestWorkers: run requests with racing context
  RequestWorkers-->>raceHTTPRequests: responses or errors
  raceHTTPRequests->>ResponseBodies: close failed and losing bodies
  raceHTTPRequests-->>httpRequest: first 200 OK response or joined error

Loading

Poem

I’m a rabbit with requests in flight,
Racing through the HTTP night.
Winners keep their bodies bright,
Losers close theirs out of sight.
Errors join and timeouts bite—
Hop, hop, tests confirm it’s right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: canceling HTTP transport races when the timeout ends.
Description check	✅ Passed	The description accurately covers the refactor, cleanup, and tests added in the pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

libcore/http_test.go (1)

137-184: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Test doesn't exercise real connection/context-cancellation semantics.

winnerBody/lateBody are trackingReadCloser mocks whose Close() unconditionally returns nil, regardless of the passed context's state. This test validates the scheduling contract of raceHTTPRequests (winner returned, late body eventually closed) but can't detect the failure mode where a real net/http/http3.Transport response body becomes unreadable once its request's context is canceled — which is exactly what happens in doH3Direct today (see the critical comment on libcore/http.go). Consider adding a test using httptest.NewServer and a real http.Client bound to the race context to confirm the winning body stays readable after raceHTTPRequests/doH3Direct return.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@libcore/http_test.go` around lines 137 - 184, Extend
TestRaceHTTPRequestsFirstSuccessClosesLateBody with an httptest server and real
http.Client requests bound to the race context, rather than relying only on
trackingReadCloser mocks. Verify the winning response body remains readable
after raceHTTPRequests returns and cancellation of the losing request, covering
the real net/http context-cancellation behavior relevant to doH3Direct.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@libcore/http_test.go`:
- Around line 137-184: Extend TestRaceHTTPRequestsFirstSuccessClosesLateBody
with an httptest server and real http.Client requests bound to the race context,
rather than relying only on trackingReadCloser mocks. Verify the winning
response body remains readable after raceHTTPRequests returns and cancellation
of the losing request, covering the real net/http context-cancellation behavior
relevant to doH3Direct.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f0cf2744-b432-4f84-ab3a-3307b9ae276e

📥 Commits

Reviewing files that changed from the base of the PR and between 6541ec1 and eef7bad.

📒 Files selected for processing (2)

• libcore/http.go
• libcore/http_test.go

[hawkff]

Added a real HTTP server regression test that delays the winning body, confirms the losing request is canceled, and reads the winner after parent-context cancellation.

[hawkff]

Exact head passed Namespace CI run 29213880616, including the production-tag libcore tests.