skip to content

Should a retry loop live in a custom http.RoundTripper or in a wrapper around Client.Do?

level: seniorimportance: should knowfreq 33%

answer

  1. two layers, one call
  2. who follows redirects, and who is below that
  3. the Client's single budget covers every attempt
  4. clone the request, close what you discard
  5. invisible is convenient and also the risk

basics

~20 s

A RoundTripper retries invisibly under the Client, so redirects, cookies and the single Client.Timeout all sit above every attempt. A wrapper around Do sits above them and can rebuild requests freely, but only helps callers who use it.

solid answer

~50 s

The two places see different things. A retrying `http.RoundTripper` is installed as `Client.Transport`, so it runs *below* the `Client`: redirect following and cookie handling happen above it and see only the final attempt's response, and `Client.Timeout` covers every attempt and every wait between them, not each one. It also has to honour the `RoundTripper` contract — do not mutate the caller's request, so `Clone` it and refresh `Body` from `GetBody`; close the body of any response you discard before retrying. Its advantage is that it is invisible: existing call sites get retries with no code change, which is also its risk. A wrapper around `Client.Do` sits above redirects, can build a fresh request per attempt, and makes the retry policy explicit at the call site — but it only applies where callers actually use it. I default to the wrapper for application code and reserve the transport for a client the whole team already shares.

code

go · 30 lines
go
type retryTransport struct {
	base        http.RoundTripper
	maxAttempts int
	delay       time.Duration
}

func (t *retryTransport) RoundTrip(req *http.Request) (*http.Response, error) {
	var lastErr error
	for attempt := 0; attempt < t.maxAttempts; attempt++ {
		try := req.Clone(req.Context())
		if req.GetBody != nil {
			body, err := req.GetBody()
			if err != nil {
				return nil, err
			}
			try.Body = body
		} else if req.Body != nil && attempt > 0 {
			return nil, lastErr // not replayable
		}
		resp, err := t.base.RoundTrip(try)
		if err == nil {
			return resp, nil
		}
		lastErr = err
		if werr := wait(req.Context(), t.delay); werr != nil {
			return nil, werr
		}
	}
	return nil, lastErr
}

go deeper

for a junior

Know that retries can sit either inside a custom Transport or in a function that calls Client.Do in a loop, and that the loop around Do is the easier one to read and reason about.

for a middle

Explain the ordering: the Client handles cookies and redirects above the Transport, so a retry inside RoundTrip repeats one hop and is invisible to everything above it.

for a senior

Show the contract details you would enforce in review — cloning the request, closing discarded responses, replayability guarded on GetBody, and the single Client.Timeout shared across attempts.

for a principal

Own the placement as a policy decision: transparent retries change the behaviour of every existing call site at once, so decide who is allowed to turn them on and for which endpoints.

## The two layers A call to `client.Do(req)` runs the `Client`'s own logic — applying the cookie jar, sending the request, then examining the response for a redirect and possibly issuing another request — and each individual send goes through `client.Transport.RoundTrip(req)`. So there are two places retry logic can go, and they are not equivalent. **Below**, in a custom `http.RoundTripper` wrapping `http.DefaultTransport`. **Above**, in a function of your own that calls `client.Do` in a loop. ## What the transport position gets you and costs you A retrying transport is transparent. Every existing `client.Do` call, and every library that was handed this client, gains retries without touching a line of code. That is genuinely useful for a shared client that many packages already use, and genuinely dangerous for the same reason: a caller who wrote one POST now has a client that may write it more than once, and nothing at the call site says so. The contract is stricter than people expect. The `http.RoundTripper` documentation says an implementation should not modify the request — the request is the caller's value, and other code may still hold it — so a retrying transport must `req.Clone(req.Context())` per attempt and refresh `Body` from `GetBody` rather than reaching into the original. It also says `RoundTrip` must close the request body, and that a response it returns is the caller's to close; the mirror of that is that any response *you* obtained and then decided to discard in order to retry is yours to close. Skipping that close is the classic leak: the connection is never returned for reuse and the retry loop quietly exhausts the connection budget. The timeout consequence is the one people trip on in production. `Client.Timeout` spans the whole `Do` call. If the transport does three attempts with waits between them, all of that happens inside that single budget, and the timeout fires part-way through attempt three with no indication that two earlier attempts were consumed. Redirects are the other surprise: because the `Client` handles them above the transport, a retry inside `RoundTrip` re-sends the same hop, not the redirect chain. ## What the wrapper position gets you and costs you A function like `doWithRetries(ctx, client, endpoint, payload)` runs above everything. It can construct a completely fresh `*http.Request` per attempt, so the drained-body problem disappears by construction rather than by remembering to call `GetBody`. It sees the final response after redirects were followed, so its view matches the caller's mental model. It can give each attempt its own derived context with its own deadline while keeping an overall bound, which the transport position cannot do cleanly because it is handed a context it does not own. The cost is discipline. It only applies where it is used, so a package that reaches for `http.DefaultClient` directly gets nothing, and "why did this path not retry" becomes a code-reading exercise. ## Retrying on a status code If your policy retries on 5xx rather than only on transport errors, the wrapper position is clearly better. A `RoundTripper` that inspects status codes is stretching its contract — the documentation is explicit that `RoundTrip` should not interpret the response — and it must then read and close every discarded response body itself, in the layer least equipped to know whether the caller wanted that body. ## What to check in review either way - The request is replayable at all: a nil `GetBody` on a non-nil body means one attempt only, and the code should say so rather than send a truncated payload. - Each attempt gets a request the previous attempt did not mutate. - Every discarded response body is closed. - The wait between attempts observes the context, so shutdown wins. - The overall bound is expressed somewhere — if the only bound is `Client.Timeout`, understand that it is shared across all attempts. ## Testing the choice An `httptest.NewServer` whose handler fails the first two calls and succeeds on the third pins the behaviour of either design, and it makes the differences visible: record the bodies the handler received and assert they are identical, count the attempts, and assert the connection was not left dangling. Run the same test against both placements and the redirect and timeout differences stop being theoretical. ## The default I would defend Application code: the explicit wrapper, because a retry is a decision about someone else's data and it should be visible at the call site. Shared infrastructure client: a transport, but with retries off by default and enabled per route, so the decision is still made by whoever knows the endpoint.

  • Why must a retrying RoundTripper clone the request instead of resetting fields on the one it was given?
    The http.RoundTripper contract says an implementation should not modify the request it receives, because the caller still owns that value and may inspect or reuse it. req.Clone(ctx) produces an independent copy with its own header map, so an attempt cannot leak state into the next one or back into the caller's request.
  • What breaks if the transport retries on a 503 and forgets to close the discarded response?
    The connection that carried the discarded response is never released, so it is not returned for reuse and stays accounted against the host's connection budget. Under a burst of retried 503s the client starves itself of connections and starts failing on dials, which looks like a network problem rather than a leak in your own retry code.
  • How does Client.Timeout behave when the retries live inside the Transport?
    It covers the entire Do call, so all attempts plus the waits between them share one budget. Three attempts under a five-second Timeout do not get five seconds each; the timeout can fire mid-attempt with no signal that earlier attempts consumed most of the window. If you want per-attempt bounds, derive them from the request context instead.
  • When is the transparent transport actually the right choice?
    When one client object is already shared across a codebase you cannot edit call site by call site, and the retry policy is genuinely uniform for the endpoints it talks to. Even then, keep it off by default and enable it per route or per host, so the decision stays with whoever knows whether that endpoint tolerates a repeated write.

saying these in an interview costs you the question

  • Mutates the caller's request inside RoundTrip instead of cloning it
  • Discards a response to retry without closing its body
  • Expects Client.Timeout to restart for each attempt
  • Assumes a transport retry re-runs the redirect chain
  • Turns on transparent retries for POSTs across a whole codebase