Skip to content

fix: delegate CloseIdleConnections through userAgentRoundTripper#256

Merged
infiniteregrets merged 1 commit intomainfrom
fix/user-agent-close-idle-connections
Mar 28, 2026
Merged

fix: delegate CloseIdleConnections through userAgentRoundTripper#256
infiniteregrets merged 1 commit intomainfrom
fix/user-agent-close-idle-connections

Conversation

@infiniteregrets
Copy link
Copy Markdown
Member

@infiniteregrets infiniteregrets commented Mar 28, 2026

closes #248

Summary

  • userAgentRoundTripper wraps the user-provided http.Client.Transport to inject User-Agent headers, but did not implement CloseIdleConnections().
  • This caused http.Client.CloseIdleConnections() to silently become a no-op after s2.New() wraps the transport, since the Go standard library only calls CloseIdleConnections() if the transport implements it.
  • Added CloseIdleConnections() to userAgentRoundTripper that delegates to the underlying transport when supported.

Test plan

  • Verify go build ./... passes
  • Confirm CloseIdleConnections() propagates through to the underlying transport after wrapping

🤖 Generated with Claude Code

userAgentRoundTripper wraps the user-provided transport but did not
implement CloseIdleConnections(), causing http.Client.CloseIdleConnections()
to silently become a no-op after the SDK wraps the transport.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@infiniteregrets infiniteregrets requested a review from a team as a code owner March 28, 2026 16:06
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 28, 2026

Greptile Summary

This PR fixes a subtle but real bug where http.Client.CloseIdleConnections() silently became a no-op after s2.New() replaced the HTTP transport with a userAgentRoundTripper wrapper. The fix adds CloseIdleConnections() to userAgentRoundTripper using an idiomatic Go interface-check delegation pattern — the same approach the standard library itself uses internally.

Confidence Score: 5/5

Safe to merge — the change is minimal, correct, and follows idiomatic Go patterns for delegating optional transport methods through wrappers.

Single method addition with no logic changes, no new state, and no risk of breakage. Remaining observations are P2 and do not affect correctness.

No files require special attention.

Important Files Changed

Filename Overview
s2/client.go Adds CloseIdleConnections() to userAgentRoundTripper to delegate to the underlying transport, fixing a silent no-op when callers invoke http.Client.CloseIdleConnections() after the SDK wraps the transport.

Reviews (1): Last reviewed commit: "fix: delegate CloseIdleConnections throu..." | Re-trigger Greptile

@infiniteregrets infiniteregrets merged commit d8aff5b into main Mar 28, 2026
10 checks passed
@infiniteregrets infiniteregrets deleted the fix/user-agent-close-idle-connections branch March 28, 2026 16:37
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.

[Detail Bug] SDK: http.Client.CloseIdleConnections stops working after passing a custom client to s2.New()

1 participant