Skip to content

fix: close retried responses in retry middleware - #16

Merged
umputun merged 1 commit into
masterfrom
retry-close-discarded
Oct 6, 2026
Merged

umputun merged 1 commit into
masterfrom
retry-close-discarded

Conversation

@umputun

@umputun umputun commented Oct 6, 2026

Copy link
Copy Markdown
Member

Retry kept a response it decided to retry in lastResponse and looped, and nothing closed its body:

if !r.shouldRetry(resp) {
	return resp, nil
}

lastResponse = resp // next attempt overwrites it, body still open

net/http releases a connection only after the body is read to EOF or closed, so each retried response held one TCP connection and its transport goroutines for the lifetime of the client. The reproducer from #15 (a server answering 502, 502, 200, five requests) ends with 11 connections still open and 35 goroutines.

RoundTrip now closes the body of a response it drops in favor of another attempt, both for a retryable status and for a response returned together with an error. The response of the last attempt is returned to the caller open, as before.

The body is closed without draining, which differs from the fix suggested in the issue. A read in the middleware has no time bound, so a server that sends headers and then stalls the body would block the retry forever on a client without a timeout, where today Retry moves on to the next attempt. Closing alone is enough to release the connection. Same reproducer after the change:

connections opened still open goroutines
before, go1.25.0 and go1.27.1 11 11 35
after, go1.27.1 1 1 5
after, go1.25.0 11 1 5

The go1.27.1 transport drains on close within its own limits and reuses the connection. The go1.25.0 transport does not, so there a retried response costs a new connection, which is closed and no longer leaked.

Related to #15

Retry kept a response it decided to retry in lastResponse and looped, and
nothing closed its body. Each retried response held one TCP connection and
its transport goroutines for the lifetime of the client.

RoundTrip closes the body of a response dropped in favor of another attempt,
for a retryable status and for a response returned together with an error.
The response of the last attempt is returned to the caller open, as before.

The body is closed without draining. A read in the middleware has no time
bound and would block the retry when an upstream sends headers and stalls
the body. The go1.27.1 transport drains on close within its own limits and
reuses the connection. The go1.25.0 transport does not, so there a retried
response costs a new connection, which is closed and no longer leaked.

Related to #15
Copilot AI balanced review requested due to automatic review settings October 6, 2026 16:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@umputun
umputun merged commit 7a78ba7 into master Oct 6, 2026
7 checks passed
@umputun
umputun deleted the retry-close-discarded branch October 6, 2026 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants