mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
514b4994c696ffd86b03878dee014c0964e191e5
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Ethan
|
2295e9d5be |
feat: surface upstream provider error details in chat callout (#24546)
Anthropic HTTP 400 responses (e.g. "image exceeds 5 MB maximum") were collapsed in the chat UI to the generic headline "Anthropic returned an unexpected error (HTTP 400)." with no actionable detail — the upstream message survived to the processor log but was dropped before reaching the client. Add a new optional `Detail` field on `codersdk.ChatStreamError` that carries the upstream provider message alongside the existing normalized headline. The backend extracts `error.message` from `fantasy.ProviderError.ResponseBody` (the JSON envelope shared by Anthropic and OpenAI), falls back to the trimmed provider message when the body is absent or unparseable, and caps the result at 500 runes. The frontend threads `Detail` through `useChatStore`, `liveStatusModel`, and `ChatStatusCallout`, rendering it as a muted secondary line inside the existing `AlertDescription`. Before: <img width="1552" height="185" alt="image" src="https://github.com/user-attachments/assets/524b588e-3cee-4fad-bc15-6bf3aec0899d" /> After: <img width="814" height="173" alt="image" src="https://github.com/user-attachments/assets/eae82a89-3ac1-4a33-8d18-ef9f77263d89" /> ## Persistence `Detail` is **not** persisted — it disappears on refresh. Persisting it would require a DB change (today `chats.last_error` is a single nullable `TEXT` column), and the shape of persisted chat errors is worth a more deliberate rethink — e.g. promoting `last_error` to `JSONB` so we can also retain structured fields like `kind`, `statusCode`, `provider`, and `retryable` instead of only the normalized headline string. That's a bigger design discussion than this PR should carry. In the meantime, seeing the upstream error reason *immediately on failure* is already a large UX improvement over the status quo, and this PR gets us there without prejudicing the eventual persistence design. Tracking persistence in CODAGT-239. Closes CODAGT-235 |
||
|
Ethan
|
4d74603045 |
fix(coderd/x/chatd): respect provider Retry-After headers in chat retry loop (#23351)
> **PR Stack** > 1. **#23351** ← `#23282` *(you are here)* > 2. #23282 ← `#23275` > 3. #23275 ← `#23349` > 4. #23349 ← `main` --- ## Summary `chatretry.Retry()` used pure exponential backoff (1 s, 2 s, 4 s, …) and never consulted provider `Retry-After` headers. Fantasy's `ProviderError` carries `ResponseHeaders` including `Retry-After`, but `chaterror.Classify()` only parsed error text and silently dropped the structured transport metadata. This makes `Retry-After` a first-class signal in the classification → retry pipeline. <img width="853" height="346" alt="image" src="https://github.com/user-attachments/assets/65f012b6-8173-43d2-957e-ab9faddea525" /> ## Changes ### `coderd/chatd/chaterror/classify.go` - Added `RetryAfter time.Duration` field to `ClassifiedError` — a normalized minimum retry delay derived from provider response metadata. - `Classify()` now calls `extractProviderErrorDetails()` before falling back to text heuristics. Structured `ProviderError.StatusCode` takes priority over regex extraction. - `normalizeClassification()` preserves and clamps `RetryAfter`. ### `coderd/chatd/chaterror/provider_error.go` (new) Provider-specific extraction, isolated from the text-based classification logic: - `extractProviderErrorDetails()` unwraps `*fantasy.ProviderError` from the error chain via `errors.As`. - `retryAfterFromHeaders()` parses headers in priority order: 1. `retry-after-ms` (OpenAI-specific, millisecond precision) 2. `retry-after` (standard HTTP — integer seconds or HTTP-date) - Case-insensitive header key lookup. ### `coderd/chatd/chatretry/chatretry.go` - `effectiveDelay(attempt, classified)` computes `max(Delay(attempt), classified.RetryAfter)` — the provider hint acts as a floor without weakening the local exponential backoff. - `Retry()` now uses `effectiveDelay` and passes the effective delay to both `onRetry(...)` and the sleep timer, so downstream payloads, logs, and the frontend countdown stay aligned automatically. ### Tests - `classify_test.go`: Structured provider status + `Retry-After` extraction, `retry-after-ms` priority, HTTP-date parsing, invalid header fallback, `WithProvider` preservation. - `chatretry_test.go`: Retry-after-as-floor semantics — longer hint wins, shorter hint keeps base delay. ## Design notes - **No SDK/API/frontend changes needed.** `codersdk.ChatStreamRetry` already carries `DelayMs` and `RetryingAt`, and the frontend already consumes them. The fix is purely in the server-side delay computation. - **Existing retryability rules unchanged.** This fixes *when* we sleep, not *whether* an error is retryable. - **Provider hint is a floor:** `max(baseDelay, RetryAfter)` ensures we never retry earlier than the provider asks, and never weaken our own backoff curve. |
||
|
Ethan
|
21c2acbad5 |
fix: refine chat retry status UX (#23651)
Follow-up to #23282. The retry and terminal error callouts had a few UX oddities: - Auto-retrying states reused backend error text that said "Please try again" even while the UI was already retrying on behalf of the user. - Terminal error states also said "Please try again" with no action the user could take. - `startup_timeout` had no specific title or retry copy — it fell through to the generic "Retrying request" heading. - The kind pill showed raw enum values like `startup_timeout` and `rate_limit`. - Terminal error metadata showed a "Retryable" / "Not retryable" label that does not help users. - A separate "Provider anthropic" metadata row duplicated information already present in the message body. - The `usage-limit` error kind used a hyphen while every backend kind uses underscores. Changes: **Backend (`chaterror/message.go`)** - Split message generation into `terminalMessage()` and `retryMessage()`, replacing the old `userFacingMessage()`. - Terminal messages include HTTP status codes and actionable guidance (e.g. "Check the API key, permissions, and billing settings."). - Retry messages are clean factual statements without status codes or remediation, suitable for the retry countdown UI (e.g. "Anthropic is temporarily overloaded."). - Removed "Please try again" / "Please try again later" from all paths. - `StreamRetryPayload` calls `retryMessage()` instead of forwarding `classified.Message`. **Frontend** - Removed the parallel frontend message-generation system: `getRetryMessage()`, `getProviderDisplayName()`, `getRetryProviderSubject()`, and the `PROVIDER_DISPLAY_NAMES` map are all deleted from `chatStatusHelpers.ts`. - `liveStatusModel.ts` passes `retryState.error` through directly — the backend owns the copy. - Added specific title and retry copy for `startup_timeout`, and extended the title mapping to cover `auth` and `config`. - Kind pills now show humanized labels ("Startup timeout", "Rate limit", etc.) instead of raw enum strings. - Removed the redundant "Provider anthropic" metadata row. - Removed the terminal "Retryable" / "Not retryable" badge. - Normalized `"usage-limit"` → `"usage_limit"` and added it to `ChatProviderFailureKind` so all error kinds follow the same underscore convention and live in one enum. Refs #23282. |
||
|
Ethan
|
70f031d793 |
feat(coderd/chatd): structured chat error classification and retry hardening (#23275)
> **PR Stack** > 1. #23351 ← `#23282` > 2. #23282 ← `#23275` > 3. **#23275** ← `#23349` *(you are here)* > 4. #23349 ← `main` --- ## Summary Extracts a structured error classification subsystem for agent chat (`chatd`) so that retry and error payloads carry machine-readable metadata — error kind, provider name, HTTP status code, and retryability — instead of raw error strings. This is the **backend half** of the error-handling work. The frontend counterpart is in #23282. ## Changes ### New package: `coderd/chatd/chaterror/` Canonical error classification — extracts error kind, provider, status code, and user-facing message from raw provider errors. One source of truth that drives both retry policy and stream payloads. - **`kind.go`**: Error kind enum (`rate_limit`, `timeout`, `auth`, `config`, `overloaded`, `unknown`). - **`signals.go`**: Signal extraction — parses provider name, HTTP status code, and retryability from error strings and wrapped types. - **`classify.go`**: Classification logic — maps extracted signals to an error kind. - **`message.go`**: User-facing message templates keyed by kind + signals. - **`payload.go`**: Projectors that build `ChatStreamError` and `ChatStreamRetry` payloads from a classified error. ### Modified - **`codersdk/chats.go`**: Added `Kind`, `Provider`, `Retryable`, `StatusCode` fields to `ChatStreamError` and `ChatStreamRetry`. - **`coderd/chatd/chatretry/`**: Thinned to retry-policy only; classification logic moved to `chaterror`. - **`coderd/chatd/chatloop/`**: Added per-attempt first-chunk timeout (60 s) via `guardedStream` wrapper — produces retryable `startup_timeout` errors instead of hanging forever. - **`coderd/chatd/chatd.go`**: Publishes normalized retry/error payloads via `chaterror` projectors. |