[sushant-me]

Fixes #4584.

Closes: #4585 — the earlier attempt at the same issue, which has been blocked on the Google CLA since 2026-10-01.

roundTripWithOptionalFollowRedirect calls the configured RoundTripper directly so it can inspect a 301/302 instead of following or surfacing it. That is the point of the helper, but it also skips http.Client.Do — and http.Client.Do is where http.Client.Timeout is applied. A caller using WithTimeout therefore got a request the timeout did not bound, while the same request through the client's ordinary path returned context.DeadlineExceeded.

Fix

The deadline is attached once, around the whole helper rather than per hop, so it spans redirects and body reads the way http.Client.Timeout does:

if timeout := c.client.Timeout; timeout > 0 {
    ctx, cancel = context.WithTimeout(ctx, timeout)
    handedOff := false
    defer func() { if !handedOff { cancel() } }()
    resp, err := c.roundTripFollowingRedirect(ctx, u, maxRedirects, opts...)
    if err != nil { return nil, err }
    if resp.Body != nil {
        resp.Body = &cancelOnCloseBody{ReadCloser: resp.Body, cancel: cancel}
        handedOff = true
    }
    return resp, nil
}

The deadline must outlive the call so the caller can read the body, so the cancel func moves to a body wrapper on the success path and is released when the body is closed — which the helper already documents as the caller's responsibility. Every other exit releases it locally. The first draft called cancel() directly on the error path only; go vet (lostcancel) correctly flagged that the timer leaked when a transport returned a nil body, hence the structure above.

Tests

Both fail on master and pass here:

• TestRoundTripWithOptionalFollowRedirect_HonorsClientTimeout — a RoundTripper that waits on req.Context().Done() with a 20 ms client timeout now returns context.DeadlineExceeded. The transport has a 2 s fallback return so a regression fails the assertion instead of hanging the suite.
• TestRoundTripWithOptionalFollowRedirect_TimeoutCoversAllRedirects — a redirect chain asserts every hop sees the same deadline, which is what "spans the whole request" means; a per-hop timeout would give each call a fresh one.

go test ./... and go vet ./github are clean. No cross-host or same-host redirect behaviour changed — the existing tests for those still pass.

[sushant-me]

The tests, linter and workflow-security runs for this branch are in action_required state (they need a maintainer to approve them before they execute), which is why the PR shows no test results yet. cla/google and check-changes have passed. An "Approve and run" from someone with write access will let the suite run; I will address anything it finds.

[codecov]

Codecov Report

❌ Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.63%. Comparing base (c678307) to head (dd08a20).

Files with missing lines	Patch %	Lines
github/github.go	95.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4688      +/-   ##
==========================================
- Coverage   98.64%   98.63%   -0.01%     
==========================================
  Files         202      202              
  Lines       19004    19022      +18     
==========================================
+ Hits        18746    18763      +17     
- Misses        258      259       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sushant-me]

Two commits pushed for the two red checks; both are in github/ only.

• ccea1e9 — golangci-lint / unparam: opts ...RequestOption always received nil — all five call sites pass no options, and after the redirect loop was split out the wrapper only forwarded them. The variadic is removed from both helpers and from the NewRequest call.
• dd08a20 — codecov/patch: the uncovered line was the successful-response path. The deadline has to outlive the call so the caller can still read the body, and Close has to release it; TestRoundTripWithOptionalFollowRedirect_ReleaseTimeoutOnClose asserts both against the request context the transport actually saw (live before Close, context.Canceled after).

Verified locally: go build ./..., go vet ./github/, and all six TestRoundTripWithOptionalFollowRedirect* tests pass. Could the checks be re-run when convenient?

[JamBalaya56562]

Hi @sushant-me, thanks for the fix! #4585 by @sb123sb123 addresses the same issue (#4584), but it has been blocked on the Google CLA since 2026-10-01. Could you add Closes: #4585 to the PR description, so that the maintainers know #4585 can be closed when this one is merged?

[sushant-me]

Added Closes: #4585 to the description. Thanks for flagging it — good to know the earlier attempt was only waiting on the CLA.