mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
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.
This commit is contained in:
@@ -732,7 +732,7 @@ func TestSubscribeDeliversRetryEventViaPubsubOnce(t *testing.T) {
|
||||
expected := &codersdk.ChatStreamRetry{
|
||||
Attempt: 1,
|
||||
DelayMs: (1500 * time.Millisecond).Milliseconds(),
|
||||
Error: "OpenAI is rate limiting requests (HTTP 429). Please try again later.",
|
||||
Error: "OpenAI is rate limiting requests (HTTP 429).",
|
||||
Kind: chaterror.KindRateLimit,
|
||||
Provider: "openai",
|
||||
StatusCode: 429,
|
||||
@@ -773,7 +773,7 @@ func TestSubscribePrefersStructuredErrorPayloadViaPubsub(t *testing.T) {
|
||||
defer cancel()
|
||||
|
||||
classified := chaterror.ClassifiedError{
|
||||
Message: "OpenAI is rate limiting requests (HTTP 429). Please try again later.",
|
||||
Message: "OpenAI is rate limiting requests (HTTP 429).",
|
||||
Kind: chaterror.KindRateLimit,
|
||||
Provider: "openai",
|
||||
Retryable: true,
|
||||
|
||||
@@ -178,7 +178,7 @@ func normalizeClassification(classified ClassifiedError) ClassifiedError {
|
||||
classified.Kind = KindGeneric
|
||||
}
|
||||
if classified.Message == "" {
|
||||
classified.Message = userFacingMessage(classified)
|
||||
classified.Message = terminalMessage(classified)
|
||||
}
|
||||
return classified
|
||||
}
|
||||
|
||||
@@ -22,7 +22,7 @@ func TestClassify(t *testing.T) {
|
||||
name: "AmbiguousOverloadKeepsProviderUnknown",
|
||||
err: xerrors.New("status 529 from upstream"),
|
||||
want: chaterror.ClassifiedError{
|
||||
Message: "The AI provider is temporarily overloaded (HTTP 529). Please try again later.",
|
||||
Message: "The AI provider is temporarily overloaded (HTTP 529).",
|
||||
Kind: chaterror.KindOverloaded,
|
||||
Provider: "",
|
||||
Retryable: true,
|
||||
@@ -33,7 +33,7 @@ func TestClassify(t *testing.T) {
|
||||
name: "ExplicitAnthropicOverload",
|
||||
err: xerrors.New("anthropic overloaded_error"),
|
||||
want: chaterror.ClassifiedError{
|
||||
Message: "Anthropic is temporarily overloaded. Please try again later.",
|
||||
Message: "Anthropic is temporarily overloaded.",
|
||||
Kind: chaterror.KindOverloaded,
|
||||
Provider: "anthropic",
|
||||
Retryable: true,
|
||||
@@ -110,7 +110,7 @@ func TestClassify(t *testing.T) {
|
||||
name: "ExplicitStatus429ClassifiesAsRateLimit",
|
||||
err: xerrors.New("status 429 from upstream"),
|
||||
want: chaterror.ClassifiedError{
|
||||
Message: "The AI provider is rate limiting requests (HTTP 429). Please try again later.",
|
||||
Message: "The AI provider is rate limiting requests (HTTP 429).",
|
||||
Kind: chaterror.KindRateLimit,
|
||||
Provider: "",
|
||||
Retryable: true,
|
||||
@@ -132,7 +132,7 @@ func TestClassify(t *testing.T) {
|
||||
name: "ServiceUnavailableClassifiesAsRetryableTimeout",
|
||||
err: xerrors.New("service unavailable"),
|
||||
want: chaterror.ClassifiedError{
|
||||
Message: "The AI provider is temporarily unavailable. Please try again later.",
|
||||
Message: "The AI provider is temporarily unavailable.",
|
||||
Kind: chaterror.KindTimeout,
|
||||
Provider: "",
|
||||
Retryable: true,
|
||||
@@ -176,7 +176,7 @@ func TestClassify(t *testing.T) {
|
||||
name: "DeadlineExceededStaysNonRetryableTimeout",
|
||||
err: context.DeadlineExceeded,
|
||||
want: chaterror.ClassifiedError{
|
||||
Message: "The request timed out before it completed. Please try again.",
|
||||
Message: "The request timed out before it completed.",
|
||||
Kind: chaterror.KindTimeout,
|
||||
Provider: "",
|
||||
Retryable: false,
|
||||
@@ -279,7 +279,7 @@ func TestClassify_TransportFailuresUseBroaderRetryMessage(t *testing.T) {
|
||||
require.True(t, classified.Retryable)
|
||||
require.Equal(
|
||||
t,
|
||||
"The AI provider is temporarily unavailable. Please try again later.",
|
||||
"The AI provider is temporarily unavailable.",
|
||||
classified.Message,
|
||||
)
|
||||
})
|
||||
@@ -299,7 +299,7 @@ func TestClassify_StartupTimeoutWrappedClassificationWins(t *testing.T) {
|
||||
)
|
||||
|
||||
require.Equal(t, chaterror.ClassifiedError{
|
||||
Message: "OpenAI did not start responding in time. Please try again.",
|
||||
Message: "OpenAI did not start responding in time.",
|
||||
Kind: chaterror.KindStartupTimeout,
|
||||
Provider: "openai",
|
||||
Retryable: true,
|
||||
@@ -315,7 +315,7 @@ func TestWithProviderUsesExplicitHint(t *testing.T) {
|
||||
|
||||
enriched := classified.WithProvider("azure openai")
|
||||
require.Equal(t, chaterror.ClassifiedError{
|
||||
Message: "Azure OpenAI is rate limiting requests (HTTP 429). Please try again later.",
|
||||
Message: "Azure OpenAI is rate limiting requests (HTTP 429).",
|
||||
Kind: chaterror.KindRateLimit,
|
||||
Provider: "azure",
|
||||
Retryable: true,
|
||||
@@ -331,7 +331,7 @@ func TestWithProviderAddsProviderWhenUnknown(t *testing.T) {
|
||||
|
||||
enriched := classified.WithProvider("openai")
|
||||
require.Equal(t, chaterror.ClassifiedError{
|
||||
Message: "OpenAI is rate limiting requests (HTTP 429). Please try again later.",
|
||||
Message: "OpenAI is rate limiting requests (HTTP 429).",
|
||||
Kind: chaterror.KindRateLimit,
|
||||
Provider: "openai",
|
||||
Retryable: true,
|
||||
|
||||
@@ -5,78 +5,113 @@ import (
|
||||
"strings"
|
||||
)
|
||||
|
||||
func userFacingMessage(classified ClassifiedError) string {
|
||||
// terminalMessage produces the user-facing error description shown
|
||||
// when retries are exhausted. It includes HTTP status codes and
|
||||
// actionable remediation guidance.
|
||||
func terminalMessage(classified ClassifiedError) string {
|
||||
subject := providerSubject(classified.Provider)
|
||||
switch classified.Kind {
|
||||
case KindOverloaded:
|
||||
return optionalStatusMessage(
|
||||
subject,
|
||||
classified.StatusCode,
|
||||
"%s is temporarily overloaded (HTTP %d). Please try again later.",
|
||||
"%s is temporarily overloaded. Please try again later.",
|
||||
)
|
||||
if classified.StatusCode > 0 {
|
||||
return fmt.Sprintf(
|
||||
"%s is temporarily overloaded (HTTP %d).",
|
||||
subject, classified.StatusCode,
|
||||
)
|
||||
}
|
||||
return fmt.Sprintf("%s is temporarily overloaded.", subject)
|
||||
|
||||
case KindRateLimit:
|
||||
return optionalStatusMessage(
|
||||
subject,
|
||||
classified.StatusCode,
|
||||
"%s is rate limiting requests (HTTP %d). Please try again later.",
|
||||
"%s is rate limiting requests. Please try again later.",
|
||||
)
|
||||
if classified.StatusCode > 0 {
|
||||
return fmt.Sprintf(
|
||||
"%s is rate limiting requests (HTTP %d).",
|
||||
subject, classified.StatusCode,
|
||||
)
|
||||
}
|
||||
return fmt.Sprintf("%s is rate limiting requests.", subject)
|
||||
|
||||
case KindTimeout:
|
||||
if classified.StatusCode > 0 {
|
||||
return fmt.Sprintf(
|
||||
"%s is temporarily unavailable (HTTP %d). Please try again later.",
|
||||
subject,
|
||||
classified.StatusCode,
|
||||
"%s is temporarily unavailable (HTTP %d).",
|
||||
subject, classified.StatusCode,
|
||||
)
|
||||
}
|
||||
if classified.Retryable {
|
||||
return fmt.Sprintf("%s is temporarily unavailable. Please try again later.", subject)
|
||||
if !classified.Retryable {
|
||||
return "The request timed out before it completed."
|
||||
}
|
||||
return "The request timed out before it completed. Please try again."
|
||||
return fmt.Sprintf("%s is temporarily unavailable.", subject)
|
||||
|
||||
case KindStartupTimeout:
|
||||
return fmt.Sprintf("%s did not start responding in time. Please try again.", subject)
|
||||
return fmt.Sprintf(
|
||||
"%s did not start responding in time.", subject,
|
||||
)
|
||||
|
||||
case KindAuth:
|
||||
if displayName := providerDisplayName(classified.Provider); displayName != "" {
|
||||
return fmt.Sprintf(
|
||||
"Authentication with %s failed. Check the API key, permissions, and billing settings.",
|
||||
displayName,
|
||||
)
|
||||
displayName := providerDisplayName(classified.Provider)
|
||||
if displayName == "" {
|
||||
displayName = "the AI provider"
|
||||
}
|
||||
return "Authentication with the AI provider failed. Check the API key, permissions, and billing settings."
|
||||
return fmt.Sprintf(
|
||||
"Authentication with %s failed."+
|
||||
" Check the API key, permissions, and billing settings.",
|
||||
displayName,
|
||||
)
|
||||
|
||||
case KindConfig:
|
||||
return fmt.Sprintf(
|
||||
"%s rejected the model configuration. Check the selected model and provider settings.",
|
||||
"%s rejected the model configuration."+
|
||||
" Check the selected model and provider settings.",
|
||||
subject,
|
||||
)
|
||||
|
||||
default:
|
||||
if classified.StatusCode > 0 {
|
||||
suffix := " Please try again."
|
||||
if classified.Retryable {
|
||||
suffix = " Please try again later."
|
||||
}
|
||||
return fmt.Sprintf(
|
||||
"%s returned an unexpected error (HTTP %d).%s",
|
||||
subject,
|
||||
classified.StatusCode,
|
||||
suffix,
|
||||
"%s returned an unexpected error (HTTP %d).",
|
||||
subject, classified.StatusCode,
|
||||
)
|
||||
}
|
||||
if classified.Retryable {
|
||||
return fmt.Sprintf(
|
||||
"%s returned an unexpected error. Please try again later.",
|
||||
subject,
|
||||
)
|
||||
if !classified.Retryable {
|
||||
return "The chat request failed unexpectedly."
|
||||
}
|
||||
return "The chat request failed unexpectedly. Please try again."
|
||||
return fmt.Sprintf("%s returned an unexpected error.", subject)
|
||||
}
|
||||
}
|
||||
|
||||
func optionalStatusMessage(subject string, statusCode int, withStatus string, withoutStatus string) string {
|
||||
if statusCode > 0 {
|
||||
return fmt.Sprintf(withStatus, subject, statusCode)
|
||||
// retryMessage produces a clean factual description suitable for
|
||||
// display alongside the retry countdown UI. It omits HTTP status
|
||||
// codes (surfaced separately in the payload) and remediation
|
||||
// guidance (not actionable while auto-retrying).
|
||||
func retryMessage(classified ClassifiedError) string {
|
||||
subject := providerSubject(classified.Provider)
|
||||
switch classified.Kind {
|
||||
case KindOverloaded:
|
||||
return fmt.Sprintf("%s is temporarily overloaded.", subject)
|
||||
case KindRateLimit:
|
||||
return fmt.Sprintf("%s is rate limiting requests.", subject)
|
||||
case KindTimeout:
|
||||
return fmt.Sprintf("%s is temporarily unavailable.", subject)
|
||||
case KindStartupTimeout:
|
||||
return fmt.Sprintf(
|
||||
"%s did not start responding in time.", subject,
|
||||
)
|
||||
case KindAuth:
|
||||
displayName := providerDisplayName(classified.Provider)
|
||||
if displayName == "" {
|
||||
displayName = "the AI provider"
|
||||
}
|
||||
return fmt.Sprintf(
|
||||
"Authentication with %s failed.", displayName,
|
||||
)
|
||||
case KindConfig:
|
||||
return fmt.Sprintf(
|
||||
"%s rejected the model configuration.", subject,
|
||||
)
|
||||
default:
|
||||
return fmt.Sprintf(
|
||||
"%s returned an unexpected error.", subject,
|
||||
)
|
||||
}
|
||||
return fmt.Sprintf(withoutStatus, subject)
|
||||
}
|
||||
|
||||
func providerSubject(provider string) string {
|
||||
|
||||
@@ -30,7 +30,7 @@ func StreamRetryPayload(
|
||||
return &codersdk.ChatStreamRetry{
|
||||
Attempt: attempt,
|
||||
DelayMs: delay.Milliseconds(),
|
||||
Error: classified.Message,
|
||||
Error: retryMessage(classified),
|
||||
Kind: classified.Kind,
|
||||
Provider: classified.Provider,
|
||||
StatusCode: classified.StatusCode,
|
||||
|
||||
@@ -20,7 +20,7 @@ func TestStreamErrorPayloadUsesNormalizedClassification(t *testing.T) {
|
||||
payload := chaterror.StreamErrorPayload(classified)
|
||||
|
||||
require.Equal(t, &codersdk.ChatStreamError{
|
||||
Message: "Azure OpenAI is rate limiting requests (HTTP 429). Please try again later.",
|
||||
Message: "Azure OpenAI is rate limiting requests (HTTP 429).",
|
||||
Kind: chaterror.KindRateLimit,
|
||||
Provider: "azure",
|
||||
Retryable: true,
|
||||
@@ -40,7 +40,7 @@ func TestStreamRetryPayloadUsesNormalizedClassification(t *testing.T) {
|
||||
delay := 3 * time.Second
|
||||
startedAt := time.Now()
|
||||
payload := chaterror.StreamRetryPayload(2, delay, chaterror.ClassifiedError{
|
||||
Message: "retry me",
|
||||
Message: "OpenAI returned an unexpected error (HTTP 503).",
|
||||
Kind: chaterror.KindGeneric,
|
||||
Provider: "openai",
|
||||
Retryable: true,
|
||||
@@ -50,7 +50,9 @@ func TestStreamRetryPayloadUsesNormalizedClassification(t *testing.T) {
|
||||
require.NotNil(t, payload)
|
||||
require.Equal(t, 2, payload.Attempt)
|
||||
require.Equal(t, delay.Milliseconds(), payload.DelayMs)
|
||||
require.Equal(t, "retry me", payload.Error)
|
||||
// Retry messages omit the HTTP status code; the status code is
|
||||
// surfaced separately in the payload's StatusCode field.
|
||||
require.Equal(t, "OpenAI returned an unexpected error.", payload.Error)
|
||||
require.Equal(t, chaterror.KindGeneric, payload.Kind)
|
||||
require.Equal(t, "openai", payload.Provider)
|
||||
require.Equal(t, 503, payload.StatusCode)
|
||||
|
||||
@@ -144,7 +144,7 @@ func TestRun_OnRetryEnrichesProvider(t *testing.T) {
|
||||
require.Equal(t, 429, records[0].classified.StatusCode)
|
||||
require.Equal(
|
||||
t,
|
||||
"OpenAI is rate limiting requests (HTTP 429). Please try again later.",
|
||||
"OpenAI is rate limiting requests (HTTP 429).",
|
||||
records[0].classified.Message,
|
||||
)
|
||||
}
|
||||
@@ -254,7 +254,7 @@ func TestRun_RetriesStartupTimeoutWhileOpeningStream(t *testing.T) {
|
||||
require.Equal(t, "openai", retries[0].Provider)
|
||||
require.Equal(
|
||||
t,
|
||||
"OpenAI did not start responding in time. Please try again.",
|
||||
"OpenAI did not start responding in time.",
|
||||
retries[0].Message,
|
||||
)
|
||||
require.ErrorIs(t, <-attemptCause, errStartupTimeout)
|
||||
@@ -313,7 +313,7 @@ func TestRun_RetriesStartupTimeoutBeforeFirstPart(t *testing.T) {
|
||||
require.Equal(t, "openai", retries[0].Provider)
|
||||
require.Equal(
|
||||
t,
|
||||
"OpenAI did not start responding in time. Please try again.",
|
||||
"OpenAI did not start responding in time.",
|
||||
retries[0].Message,
|
||||
)
|
||||
require.ErrorIs(t, <-attemptCause, errStartupTimeout)
|
||||
@@ -475,7 +475,7 @@ func TestRun_RetriesStartupTimeoutWhenStreamClosesSilently(t *testing.T) {
|
||||
require.Equal(t, "openai", retries[0].Provider)
|
||||
require.Equal(
|
||||
t,
|
||||
"OpenAI did not start responding in time. Please try again.",
|
||||
"OpenAI did not start responding in time.",
|
||||
retries[0].Message,
|
||||
)
|
||||
require.ErrorIs(t, <-attemptCause, errStartupTimeout)
|
||||
|
||||
Reference in New Issue
Block a user