Skip to content

fix(openai): drain error response body for HTTP connection reuse#2450

Merged
esengine merged 1 commit into
esengine:main-v2from
HUQIANTAO:fix/openai-drain-body
May 31, 2026
Merged

fix(openai): drain error response body for HTTP connection reuse#2450
esengine merged 1 commit into
esengine:main-v2from
HUQIANTAO:fix/openai-drain-body

Conversation

@HUQIANTAO
Copy link
Copy Markdown
Contributor

Summary

After reading the limited (4KB) error body from a non-200 response, drain any remaining body to io.Discard before closing. This ensures the HTTP/2 connection can be reused by Go's transport pool.

Before

msg, _ := io.ReadAll(io.LimitReader(resp.Body, 4096))
resp.Body.Close()

After

msg, _ := io.ReadAll(io.LimitReader(resp.Body, 4096))
_, _ = io.Copy(io.Discard, resp.Body)
resp.Body.Close()

Motivation

Go's http.Transport reuses connections only when the response body is fully drained. Without draining, each retry in sendWithRetry opens a new TCP connection instead of reusing the existing one. This is especially impactful for the retry loop (up to 3 attempts on transient errors).

@github-actions github-actions Bot added the v2 Go rewrite (1.x) — main-v2 branch, active development label May 31, 2026
@esengine esengine merged commit d84fbab into esengine:main-v2 May 31, 2026
3 checks passed
esengine pushed a commit that referenced this pull request May 31, 2026
Surface previously-swallowed errors via slog across checkpoint dir/persist, controller snapshot (compact + rewind), model-switch snapshot, config load-for-edit, and MCP initialize parsing — silent failures (e.g. a checkpoint that never persisted) are now visible in logs without disrupting flow. Rebased over #2450/#2459: dropped the now-redundant serve.writeJSON change (already landed in #2459) and merged the openai readErr fallback with #2450's body drain. Thanks @HUQIANTAO.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Go rewrite (1.x) — main-v2 branch, active development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants