Repository navigation
middleware.Retry leaks connections: retried responses are never closed #15
Description
middleware.Retry never closes the body of a response it decides to retry. When shouldRetry(resp) is true, RoundTrip stores the response in lastResponse and loops. The next attempt overwrites it, and nobody reads or closes the body.
https://github.com/go-pkgz/requester/blob/v0.5.0/middleware/retry.go#L129-L141
net/http returns a connection to the idle pool only after the response body is read to EOF or closed, and an unclosed body is never released. So every retried response leaks one TCP connection, plus the transport goroutines serving it, for the lifetime of the client.
Reproducer
A server answers 502, 502, 200, in turn. The client makes 5 requests, reading and closing every body it gets back:
package main
import (
"fmt"
"io"
"net"
"net/http"
"net/http/httptest"
"sync/atomic"
"time"
"github.com/go-pkgz/requester"
"github.com/go-pkgz/requester/middleware"
)
func main() {
var calls, conns atomic.Int32
srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
if calls.Add(1)%3 != 0 {
w.WriteHeader(http.StatusBadGateway)
_, _ = io.WriteString(w, "upstream down")
return
}
_, _ = io.WriteString(w, "ok")
}))
srv.Config.ConnState = func(_ net.Conn, s http.ConnState) {
if s == http.StateNew {
conns.Add(1)
}
}
srv.Start()
defer srv.Close()
rq := requester.New(http.Client{}, middleware.Retry(3, time.Millisecond))
for range 5 {
req, _ := http.NewRequest(http.MethodGet, srv.URL, http.NoBody)
resp, err := rq.Do(req)
if err != nil {
panic(err)
}
_, _ = io.Copy(io.Discard, resp.Body)
_ = resp.Body.Close()
}
fmt.Printf("%d HTTP calls over %d TCP connections\n", calls.Load(), conns.Load())
}Output with v0.5.0 (also current master), go1.27.1:
15 HTTP calls over 11 TCP connections
Expected: 1 connection, reused through keep-alive.
Suggested fix
Drain (bounded) and close the response before the next attempt. The final response is still returned to the caller, who closes it:
if !r.shouldRetry(resp) {
return resp, nil
}
if attempt < attempts-1 { // this response is discarded
_, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 64<<10))
_ = resp.Body.Close()
}
lastResponse = respThe transport-error branch (err != nil) can carry a non-nil resp too, since another middleware may return both. It probably needs the same treatment when that response is discarded.
I'm happy to send a PR with this fix and a test.
Activity
fix: close retried responses in retry middleware (#16)
umputun commentedon Oct 6, 2026
fixed in #16.
Retrynow closes the body of every response it drops before the next attempt, so a retried response no longer holds a connection. Your reproducer ends with 1 open connection and 5 goroutines, down from 11 and 35.I closed the body without the drain you suggested. A read in the middleware has no time limit, so a server that sends 502 headers and then stalls the body would block the retry forever on a client without a timeout, where before
Retrymoved on to the next attempt.the cost is connection reuse on older toolchains. The go1.27.1 transport drains on close within its own limits, and your reproducer gets the expected 1 connection there. The go1.25.0 transport does not, so each retried response opens a new connection, which is closed.