fix(mcp): a held-open session is not a request, so it carries no deadline

coilyco-ops commented

2026-08-13 15:53:41 +00:00

Member

The 180.000s was ours.

defaultRequestTimeout = 3 * time.Minute          // tuning.go

httpClient := &http.Client{ Timeout: cfg.RequestTimeout, ... }
tools := &MCPProvider{ HTTPClient: httpClient }   // the same client

http.Client.Timeout is a whole-request deadline that includes reading the body, and one client served both the model calls and the MCP transport. Every symptom on the issue follows from that and needs no server-side cause:

http.response.status_code: 200 beside an error — the headers genuinely arrived, then the client cut the body.
use of closed network connection — the client closing its own connection.
Only streamable sessions hit it — a session held open to receive server messages never finishes a "request" by the client's definition.
Exactly 180.000s, to the microsecond — it is a constant, not a coincidence.

The fix

The MCP transport gets its own client: same otelhttp instrumentation so tracing is unchanged, and no whole-request timeout. Per-call deadlines are untouched — callTimeout already bounds every tool call, which is where a bound belongs.

The model client is not modified. A unary completion that never returns must still not hold a turn open forever. This is additive.

What stays as it is, deliberately

The spans remain parentless. The provider dials on context.WithCancel(context.Background()) so a connection outlives the turn that opened it — that is the property connection reuse exists for, and attaching the session to a turn context would break it. Kai's first recommendation would have cost more than it fixed; the second one, giving the session its own span name so it stops polluting HTTP POST percentiles, is right and is filed separately rather than folded in here.

Tests

Four. Two assert the two clients differ in the one way that matters. The other two reproduce the failure at millisecond scale against httptest: a client with a whole-request timeout returns a real 200 and then fails the body read — the exact signature on the issue — while the session client leaves the body open past the same deadline.

Mutating sessionHTTPClient to carry defaultRequestTimeout turns the guard red.

The held-open test first asserted what the body finally yielded, which races with the server's close and failed once. That assertion is gone: staying open past a deadline is the property, and what arrives afterwards is the server's business. Verified green across forced uncached reruns rather than cache hits.

closes #160

The 180.000s was ours. ```go defaultRequestTimeout = 3 * time.Minute // tuning.go httpClient := &http.Client{ Timeout: cfg.RequestTimeout, ... } tools := &MCPProvider{ HTTPClient: httpClient } // the same client ``` `http.Client.Timeout` is a whole-request deadline that includes reading the body, and one client served both the model calls and the MCP transport. Every symptom on the issue follows from that and needs no server-side cause: - **`http.response.status_code: 200` beside an error** — the headers genuinely arrived, then the client cut the body. - **`use of closed network connection`** — the client closing its own connection. - **Only streamable sessions hit it** — a session held open to receive server messages never finishes a "request" by the client's definition. - **Exactly 180.000s, to the microsecond** — it is a constant, not a coincidence. ## The fix The MCP transport gets its own client: same `otelhttp` instrumentation so tracing is unchanged, and no whole-request timeout. Per-call deadlines are untouched — `callTimeout` already bounds every tool call, which is where a bound belongs. **The model client is not modified.** A unary completion that never returns must still not hold a turn open forever. This is additive. ## What stays as it is, deliberately The spans remain parentless. The provider dials on `context.WithCancel(context.Background())` so a connection outlives the turn that opened it — that is the property connection reuse exists for, and attaching the session to a turn context would break it. Kai's first recommendation would have cost more than it fixed; the second one, giving the session its own span name so it stops polluting `HTTP POST` percentiles, is right and is filed separately rather than folded in here. ## Tests Four. Two assert the two clients differ in the one way that matters. The other two reproduce the failure at millisecond scale against `httptest`: a client with a whole-request timeout returns a real 200 and then fails the body read — the exact signature on the issue — while the session client leaves the body open past the same deadline. Mutating `sessionHTTPClient` to carry `defaultRequestTimeout` turns the guard red. The held-open test first asserted what the body finally yielded, which races with the server's close and failed once. That assertion is gone: staying open past a deadline is the property, and what arrives afterwards is the server's business. Verified green across forced uncached reruns rather than cache hits. closes #160

coilyco-ops added 1 commit

2026-08-13 15:53:42 +00:00

fix(mcp): a held-open session is not a request, so it carries no deadline

ci / image-build (pull_request) Successful in 22s

Details

ci / test (pull_request) Successful in 35s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

d49a6b4f42

Two spans a day died at exactly 180.000s against the Forgejo MCP,
carrying http 200 alongside an error and use of closed network
connection. That is not the server failing. It is our own
http.Client.Timeout, defaultRequestTimeout, three minutes to the
microsecond, applied to a streamable session that never finishes a
request by the client's definition.

One client served the model calls and the MCP transport. The MCP
transport now has its own, with the same otelhttp instrumentation and no
whole-request timeout. Per-call deadlines are unchanged: callTimeout
already bounds every tool call, which is where a bound belongs.

The model client keeps its deadline. A unary completion that never
returns must not hold a turn open forever.

The parentless spans stay parentless and that is correct: the provider
dials on a root context so a connection outlives the turn that opened
it, which is the property connection reuse exists for.

closes #160

Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-13 15:53:42 +00:00

coilyco-ops referenced this pull request

2026-08-13 15:54:30 +00:00

A long-lived MCP session is counted in HTTP POST percentiles, so a connection's lifetime reads as request latency #560

coilyco-ops referenced this pull request

2026-08-13 15:55:55 +00:00

A long-lived MCP session is counted in HTTP POST percentiles, so a connection's lifetime reads as request latency #560

coilyco-ops referenced this pull request

2026-08-13 15:56:16 +00:00

Steam MCP 400s and cancelled tool-discovery calls recurred #161

coilyco-ops merged commit e0ab0253a0 into main