fix: Concentrate adapter timeout/retries/error taxonomy, stream diagnostics, Gemini request-ID - #110
Merged
Merged
Conversation
…ards
The Concentrate adapter lagged behind every other provider client in
three ways (audit E1-E3):
- E1: it built a private &http.Client{Timeout: 120s} instead of using
the shared pooled transport. The hard-coded whole-response timeout
killed any stream longer than two minutes and bypassed connection
reuse. It now uses core.NewPooledHTTPClient(core.DefaultTimeout).
- E2: SetRetry was a no-op claiming retries were handled at the HTTP
client level (http.Client never retries). Both request paths now go
through core.DoWithRetry with GetBody set, mirroring openai.go and
anthropic.go, and SetRetry/Retry store and return the config.
- E3: non-200 responses were read with an unbounded io.ReadAll and
returned as fmt.Errorf strings, dropping status/request-ID structure
so IsRetriable()/IsAuthError() and engine classify() could not work.
Errors now come from core.ParseProviderError + core.FormatAPIError
(*core.EyrieError with provider/op/status/request-ID), and the
captured X-Request-Id is propagated to stream results.
- normalizeToolParams mutated the caller's tool schema map in place;
it now returns a shallow copy with additionalProperties injected.
Tests cover retry-on-500-then-success, structured auth/rate-limit
errors on chat and stream paths, streaming through a slow server with
the default pooled client, and the no-mutation contract.
client/core's ProcessOpenAIStream emits end-of-stream health diagnostics
(reasoning-only or empty responses, via DetectResponseHealth) as an
error-type event immediately followed by the terminal done event — the
comment says non-fatal. But engine/stream.go mapped every error event
to &Error{Code: ErrorProviderUnavailable} and forward() returned, so
the terminal done/usage event never reached the host and Err() was set
even though content had been delivered. Reasoning-only responses
routinely trigger this diagnostic, failing otherwise-successful
streams (audit E4).
Event-flow change:
- client/core/stream.go finish() now marks the diagnostic error event
with the existing EyrieStreamEvent.Warning field (additive; .Error is
still populated for consumers that only read it).
- engine/stream.go normalizeEvent() forwards error events carrying
Warning as Event{Type: EventWarning, Warning: ...} without an error,
so forward() emits it and keeps consuming; the final done/usage is
delivered and Err() stays nil. Error events without Warning remain
terminal provider_unavailable failures, identical to before.
- client/continuation.go (deprecated helper) and client/tracing.go
apply the same distinction: diagnostics are forwarded without ending
the stream / failing the span.
Tests: core asserts the diagnostic carries Warning and precedes done;
engine adds regression tests for diagnostic-then-done (content + done
delivered, Err() nil) and fatal error (terminal, provider_unavailable).
Gemini StreamChat captured the X-Goog-Request-Id response header (used for error correlation) but passed "" to llm.NewStreamResult on both the shared-parser and legacy-parser paths, so successful streams lost the provider correlation ID. Pass the captured value (audit E5).
doRequestWithMimoAuthRetry was duplicated verbatim between the OpenAI and Anthropic adapters; extract it into a package-level doWithMimoAuthRetry (next to mimoAuthHeaders) that takes a setRetryHeaders callback for the provider-specific Bearer headers. Both methods keep their signatures as thin wrappers — no behavior change.
…o 1.26.6 - concentrate_responses.go: resp.Body was read but never closed when the stream request returned a non-200 status (bodyclose) - go.mod + CI: Go 1.26.6 — 1.26.5 stdlib has reachable vulns (GO-2026-6090, GO-2026-6089) that fail govulncheck
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
http.Client.Timeoutwith shared pooled client; addedDoWithRetry(matching openai/anthropic); replacedfmt.ErrorfwithParseProviderError/FormatAPIErrorfor structured error taxonomy; fixednormalizeToolParamsmutating caller's map.Err()— the terminaldone/usageevent reaches the host correctly.X-Goog-Request-Idnow propagated toNewStreamResultin both parser paths.doRequestWithMimoAuthRetryfrom openai/anthropic into sharedclient/adapters/mimo.go.Test plan
GOWORK=off go build ./...client/adapters(334),client/core(90),engine(90)./config(TestHasAnyConfiguredDeployment_RejectsPlaceholder) — reproduced on base commit; unrelated.