Commit Graph
22 Commits
Author SHA1 Message Date
David Gageot 634dfc81fe refactor(modelerrors): simplify provider error parsing
Same behaviour, less code, easier to read:

  * StatusError.Error()    14 -> 9 lines (request-id concat moved into

                           parseProviderError, no second branch here)

  * extractFirstJSONObject  35 -> 9 lines, renamed firstJSONObject:

                           replace the hand-rolled brace/string/escape

                           state machine with json.Decoder, which

                           handles escapes correctly by construction

  * parseProviderErrorDetails split into two single-purpose helpers:

      - parseProviderError    finds the JSON body, parses it, appends

                              the request id to the rendered details

      - formatProviderError   renders the parsed struct as a one-liner

  * unquoteJSONScalar      renamed scalarString and switched to plain

                           any/float64 instead of json.RawMessage +

                           per-scalar json.Unmarshal calls

All existing tests still pass unchanged; lint clean.

Assisted-By: docker-agent
2026-04-27 18:05:42 +02:00
David Gageot a3d0cd9728 feat(modelerrors): surface structured provider error details
Parse the JSON body that providers attach to non-2xx responses and lift

the structured fields (error.type, error.message, error.code, error.param,

status, request id) into StatusError.Error(), stripping the URL noise.

Recognised shapes:

  Anthropic   {"type":"error","error":{"type":"...","message":"..."}}

  OpenAI      {"error":{"message":"...","type":"...","code":"...","param":"..."}}

  Gemini      {"error":{"code":N,"message":"...","status":"..."}}

  Proxies     {"message":"Bad Request"}

When the body is opaque (Docker AI gateway returning only

{"message":"Bad Request"}) we still strip the URL so the user gets

"HTTP 400: Bad Request" instead of a 200-character POST/URL/JSON wall.

Falls back to the original err.Error() text whenever no JSON object is

found or the body has no recognised fields, so non-SDK errors are

unchanged (e.g. "HTTP 429: rate limit exceeded").

Assisted-By: docker-agent
2026-04-27 18:02:33 +02:00
David Gageot 20400137a0 modelerrors: remove unused MaxRetryAfterWait re-export
No callers reference modelerrors.MaxRetryAfterWait — all usage goes
through backoff.MaxRetryAfterWait directly. Remove the constant and
the now-unused backoff import.

Assisted-By: docker-agent
2026-03-19 19:48:39 +01:00
David Gageot dcfb3c76d5 modelerrors: unexport IsRetryableStatusCode and ExtractHTTPStatusCode
Both are only used within the package. Rename to isRetryableStatusCode
and extractHTTPStatusCode to reduce the public API surface.

Assisted-By: docker-agent
2026-03-19 19:47:36 +01:00
David Gageot f699806729 modelerrors: unexport ParseRetryAfterHeader
Only called within the package (by WrapHTTPError and tests).
Rename to parseRetryAfterHeader to reduce the public API surface.

Assisted-By: docker-agent
2026-03-19 19:46:59 +01:00
David Gageot e76494ef0a modelerrors: remove unused deprecated wrappers
CalculateBackoff and SleepWithContext were deprecated in favor of
backoff.Calculate and backoff.SleepWithContext. No callers use the
modelerrors versions — all code imports pkg/backoff directly.

Remove the deprecated wrappers to reduce API surface.

Assisted-By: docker-agent
2026-03-19 19:44:15 +01:00
David Gageot 6512b13dd4 modelerrors: add NewContextOverflowError constructor
Callers were constructing &ContextOverflowError{Underlying: err}
directly. Adding a constructor improves encapsulation and allows
future field additions without breaking callers. Update
runtime/fallback.go to use the constructor.

Assisted-By: docker-agent
2026-03-19 19:33:48 +01:00
David Gageot 744db2501d modelerrors: include HTTP status code in StatusError.Error()
StatusError.Error() previously delegated directly to the wrapped error,
discarding the status code. This made debugging harder since log lines
wouldn't reveal the HTTP status. Now Error() returns a message like
"HTTP 429: rate limit exceeded" for better observability.

Assisted-By: docker-agent
2026-03-19 19:33:01 +01:00
David Gageot 4fd44ed62d modelerrors: move backoff and sleep utilities to pkg/backoff
SleepWithContext and CalculateBackoff are general-purpose utilities not
specific to model errors. Move them to a new pkg/backoff package for
clarity and reusability. MaxRetryAfterWait moves there too.

Deprecated wrapper variables are left in modelerrors to maintain
backward compatibility. The runtime/fallback package now imports
pkg/backoff directly.

Assisted-By: docker-agent
2026-03-19 19:31:49 +01:00
David Gageot d13541e364 modelerrors: promote retryable/non-retryable patterns to package-level vars
The retryablePatterns and nonRetryablePatterns slices were local
variables inside isRetryableModelError, causing a new allocation on
every call. Promote them to package-level vars (like
contextOverflowPatterns) so they are allocated once.

Assisted-By: docker-agent
2026-03-19 19:27:56 +01:00
David Gageot f182f8de43 modelerrors: switch from math/rand to math/rand/v2
math/rand/v2 is the preferred package since Go 1.22. The API is
compatible (rand.Float64 exists in both).

Assisted-By: docker-agent
2026-03-19 19:27:09 +01:00
David Gageot c303c34dfa modelerrors: use strconv.Atoi instead of fmt.Sscanf in ExtractHTTPStatusCode
The regex match is a simple integer string. strconv.Atoi is simpler,
faster, and already imported. This also removes the fmt import.

Assisted-By: docker-agent
2026-03-19 19:26:41 +01:00
David Gageot 747c989df6 modelerrors: remove provider SDK imports from ExtractHTTPStatusCode
Now that all providers (Anthropic, OpenAI, Gemini) wrap their errors in
*StatusError via WrapHTTPError, the provider-specific type assertions
(anthropic.Error, genai.APIError) in ExtractHTTPStatusCode are redundant.
ClassifyModelError already checks *StatusError first.

Replace the SDK-specific checks with a *StatusError check, keeping the
regex fallback for unwrapped errors. This removes the coupling between
the error classification package and provider-specific SDKs.

Assisted-By: docker-agent
2026-03-19 19:26:01 +01:00
David Gageot f18f5b5546 modelerrors: unexport IsRetryableModelError
The only caller outside the package is ClassifyModelError itself
(as a fallback path). ClassifyModelError is the intended public API
and provides strictly more information (retryable + rateLimited +
retryAfter). Rename to isRetryableModelError to reduce API surface.

Assisted-By: docker-agent
2026-03-19 19:25:00 +01:00
David Gageot 8485131914 modelerrors: remove numeric status-code patterns from string matching
ExtractHTTPStatusCode + IsRetryableStatusCode already handle numeric
status codes (500, 502, 429, 401, etc.) via regex with word boundaries.
Having the same codes as bare substrings in the retryable/non-retryable
pattern lists caused false positives (e.g., "processed 500 items"
would match). Remove the numeric patterns and keep only the descriptive
text patterns.

Assisted-By: docker-agent
2026-03-19 19:24:09 +01:00
Go Expert Coding agent 3d44b7cd40 refactor: use typed StatusError for retry metadata, providers wrap errors at Recv()
Addresses review feedback from rumpl and simonferquel on PR #2096.

The previous approach extracted Retry-After metadata in a dispatcher function
that type-switched on provider SDK error types (`*anthropic.Error`,
`*openai.Error`). This kept all providers coupled to the modelerrors package
and duplicated parsing logic.

New design:

1. `modelerrors.StatusError` — a thin typed wrapper carrying:
   - StatusCode int
   - RetryAfter time.Duration  (parsed from Retry-After header)
   - Err error                 (original SDK error, accessible via Unwrap)

2. `modelerrors.WrapHTTPError(statusCode, resp, err)` — wraps any provider
   error in StatusError; parses Retry-After from resp.Header if non-nil.

3. `modelerrors.ParseRetryAfterHeader(value)` — shared RFC 7231 §7.1.3
   parser (integer seconds + HTTP-date). No SDK imports.

4. `modelerrors.ClassifyModelError(err)` — single-arg; uses errors.As to
   find *StatusError in the chain; falls back to message/pattern matching for
   providers not yet wrapped (e.g. Bedrock).

5. Three new wrap.go files (one per provider family), each a single function:
   - pkg/model/provider/anthropic/wrap.go  — wrapAnthropicError(*anthropic.Error)
   - pkg/model/provider/oaistream/wrap.go  — WrapOpenAIError(*openai.Error) (exported)
   - pkg/model/provider/gemini/wrap.go     — wrapGeminiError(*genai.APIError)

6. Wrap sites — one-line change at Recv() in each adapter:
   - anthropic/adapter.go, beta_adapter.go
   - oaistream/adapter.go
   - openai/response_stream.go (reuses oaistream.WrapOpenAIError)
   - gemini/adapter.go

No changes to fallback.go logic — ClassifyModelError signature stays
`(err error)` and retry/rate-limit decisions are unchanged.

Assisted-By: docker-agent
2026-03-13 11:07:50 +00:00
Go Expert Coding agent 7c22ff16f9 fix: make HTTP 429 retryable when no fallback model, respect Retry-After header
When a model returns HTTP 429 (Too Many Requests):
- If fallback models are configured: skip to next fallback immediately (existing behavior preserved)
- If no fallbacks are configured: retry the same model using the Retry-After header duration (capped at 60s), falling back to exponential backoff if the header is absent

HTTP 500 was already handled as retryable via IsRetryableStatusCode — no change needed.

New additions to pkg/modelerrors:
- ExtractRetryAfter(err): extracts Retry-After duration from *anthropic.Error and *openai.Error responses
- parseRetryAfterHeader(value): parses integer-seconds or HTTP-date Retry-After values
- ClassifyModelError(err): unified classifier returning (retryable, rateLimited bool, retryAfter time.Duration)
- MaxRetryAfterWait constant (60s cap)

pkg/runtime/fallback.go changes:
- Replace both IsRetryableModelError call sites with ClassifyModelError
- Add handleModelError helper to avoid duplicating classification logic
- 429 with fallbacks → break to next model (unchanged behavior)
- 429 without fallbacks → sleep(min(retryAfter, 60s) or backoff), then continue

Fixes #2095

Assisted-By: docker-agent
2026-03-13 10:17:35 +00:00
David Gageot 943d3b0606 This can be retried
Signed-off-by: David Gageot <david.gageot@docker.com>
2026-03-12 17:17:54 +01:00
David Gageot d02c3f0ceb Merge pull request #2027 from dgageot/board/look-at-https-github-com-golangci-golang-ecda3267
Enable modernize and perfsprint linters, fix all findings
2026-03-10 12:45:24 +01:00
David Gageot 8b86ec6e5b refactor: use errors.AsType consistently instead of errors.As with pre-declared variables
Assisted-By: docker-agent
2026-03-10 12:01:39 +01:00
David Gageot d7e406c9bf Enable modernize and perfsprint linters, fix all findings
Add modernize and perfsprint to .golangci.yml and resolve all lint
issues across 97 files:

- Replace fmt.Errorf with errors.New where no format verbs are used
- Replace fmt.Sprintf(%d) with strconv.Itoa for int conversions
- Replace redundant fmt.Sprintf(%s) with direct string values
- Use slices.Contains instead of manual loops (SkillsConfig.HasLocal)
- Use omitzero instead of omitempty on struct fields (time.Time, SkillsConfig)
- Inline boolPtr helpers to new(bool) and remove unused functions
- Replace string concatenation in loops with strings.Builder
- Use fmt.Fprintf instead of WriteString(fmt.Sprintf(...))

Assisted-By: docker-agent
2026-03-10 11:57:30 +01:00
David Gageot 6068592017 refactor: split runtime.go and extract pkg/modelerrors
Break up the oversized runtime.go (2510 lines) by extracting cohesive
groups of functions into dedicated files, and move pure error
classification utilities into a new reusable package.

New files in pkg/runtime:
- model_picker.go: model change/revert tool handlers
- agent_delegation.go: sub-agent dispatch, task transfer, handoffs

New package pkg/modelerrors:
- ContextOverflowError type and detection
- HTTP status code extraction (Anthropic, Gemini, OpenAI)
- Retryable error classification
- Exponential backoff with jitter
- User-friendly error formatting

Assisted-By: docker-agent
2026-03-09 17:32:50 +01:00