diff --git a/coderd/x/chatd/chaterror/classify_test.go b/coderd/x/chatd/chaterror/classify_test.go index 577e120e72..0f1438decc 100644 --- a/coderd/x/chatd/chaterror/classify_test.go +++ b/coderd/x/chatd/chaterror/classify_test.go @@ -221,6 +221,10 @@ func TestClassify_PatternCoverage(t *testing.T) { {name: "BrokenPipeLiteral", err: "broken pipe", wantKind: chaterror.KindTimeout, wantRetry: true}, {name: "BadGatewayLiteral", err: "bad gateway", wantKind: chaterror.KindTimeout, wantRetry: true}, {name: "GatewayTimeoutLiteral", err: "gateway timeout", wantKind: chaterror.KindTimeout, wantRetry: true}, + {name: "ClientConnLiteral", err: "client conn", wantKind: chaterror.KindTimeout, wantRetry: true}, + {name: "GOAWAYLiteral", err: "goaway", wantKind: chaterror.KindTimeout, wantRetry: true}, + {name: "HTTP2StreamClosedLiteral", err: "http2: stream closed", wantKind: chaterror.KindTimeout, wantRetry: true}, + {name: "UseOfClosedNetworkConnectionLiteral", err: "use of closed network connection", wantKind: chaterror.KindTimeout, wantRetry: true}, {name: "AuthenticationLiteral", err: "authentication", wantKind: chaterror.KindAuth, wantRetry: false}, {name: "UnauthorizedLiteral", err: "unauthorized", wantKind: chaterror.KindAuth, wantRetry: false}, {name: "InvalidAPIKeyLiteral", err: "invalid api key", wantKind: chaterror.KindAuth, wantRetry: false}, @@ -289,6 +293,168 @@ func TestClassify_TransportFailuresUseBroaderRetryMessage(t *testing.T) { } } +// TestClassify_HTTP2TransportErrors checks HTTP/2 transport errors +// classify as retryable KindTimeout. Split into two sub-tables so a +// bug in transport matching cannot be masked by provider detection +// (and vice versa). +func TestClassify_HTTP2TransportErrors(t *testing.T) { + t.Parallel() + + // Transport patterns, no provider hint. Provider stays empty and + // Message uses the generic subject. + transportOnly := []struct { + name string + err string + }{ + { + name: "HTTP2ClientConnForceClosed", + err: "http2: client connection force closed via ClientConn.Close", + }, + { + name: "HTTP2TransportGOAWAY", + err: "http2: Transport received Server's graceful shutdown GOAWAY", + }, + { + name: "HTTP2ServerGOAWAY", + err: "http2: server sent GOAWAY and closed the connection", + }, + { + name: "HTTP2StreamClosed", + err: "http2: stream closed", + }, + { + name: "UseOfClosedNetworkConnectionOnPOST", + err: `Post "https://example.com/v1/messages": use of closed network connection`, + }, + { + name: "HTTP2ClientConnIsClosed", + err: "http2: client conn is closed", + }, + { + name: "HTTP2ClientConnNotUsable", + err: "http2: client conn not usable", + }, + { + name: "HTTP2ClientConnNotEstablished", + err: "http2: client conn could not be established", + }, + { + name: "HTTP2ClientConnectionLost", + err: "http2: client connection lost", + }, + } + + for _, tt := range transportOnly { + t.Run("TransportOnly/"+tt.name, func(t *testing.T) { + t.Parallel() + + classified := chaterror.Classify(xerrors.New(tt.err)) + require.Equal(t, chaterror.KindTimeout, classified.Kind, "Kind") + require.True(t, classified.Retryable, "Retryable") + require.Equal(t, "", classified.Provider, "Provider") + require.Equal(t, + "The AI provider is temporarily unavailable.", + classified.Message, + "Message", + ) + }) + } + + // Same transport signature with a provider host in the URL so + // detectProvider can stamp Provider. + providerDetection := []struct { + name string + err string + provider string + wantMessage string + }{ + { + name: "CustomerRegressionAnthropic", + err: `stream response: Post "https://api.anthropic.com/v1/messages": http2: client connection force closed via ClientConn.Close`, + provider: "anthropic", + wantMessage: "Anthropic is temporarily unavailable.", + }, + { + name: "OpenAIForceClosed", + err: `stream response: Post "https://api.openai.com/v1/chat/completions": http2: client connection force closed via ClientConn.Close`, + provider: "openai", + wantMessage: "OpenAI is temporarily unavailable.", + }, + { + name: "GoogleGOAWAY", + err: `stream response: Post "https://generativelanguage.googleapis.com/v1beta/models/gemini-pro:streamGenerateContent": http2: server sent GOAWAY and closed the connection`, + provider: "google", + wantMessage: "Google is temporarily unavailable.", + }, + } + + for _, tt := range providerDetection { + t.Run("ProviderDetection/"+tt.name, func(t *testing.T) { + t.Parallel() + + classified := chaterror.Classify(xerrors.New(tt.err)) + require.Equal(t, chaterror.KindTimeout, classified.Kind, "Kind") + require.True(t, classified.Retryable, "Retryable") + require.Equal(t, tt.provider, classified.Provider, "Provider") + require.Equal(t, tt.wantMessage, classified.Message, "Message") + }) + } +} + +// TestClassify_StatusCodeBeatsHTTP2Transport ensures explicit status +// codes still win over the new HTTP/2 patterns. +func TestClassify_StatusCodeBeatsHTTP2Transport(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + err string + wantKind string + wantRetryable bool + wantStatus int + }{ + { + name: "HTTP2With429", + err: "http2: server error 429 Too Many Requests", + wantKind: chaterror.KindRateLimit, + wantRetryable: true, + wantStatus: 429, + }, + { + name: "HTTP2With401", + err: "http2: 401 unauthorized", + wantKind: chaterror.KindAuth, + wantRetryable: false, + wantStatus: 401, + }, + { + name: "ClientConnWith429RateLimitWins", + err: "http2: client conn is closed: status 429 Too Many Requests", + wantKind: chaterror.KindRateLimit, + wantRetryable: true, + wantStatus: 429, + }, + { + name: "GOAWAYWith401AuthWins", + err: "http2: server sent GOAWAY: status 401 unauthorized", + wantKind: chaterror.KindAuth, + wantRetryable: false, + wantStatus: 401, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + classified := chaterror.Classify(xerrors.New(tt.err)) + require.Equal(t, tt.wantKind, classified.Kind, "Kind") + require.Equal(t, tt.wantRetryable, classified.Retryable, "Retryable") + require.Equal(t, tt.wantStatus, classified.StatusCode, "StatusCode") + }) + } +} + func TestClassify_StartupTimeoutWrappedClassificationWins(t *testing.T) { t.Parallel() diff --git a/coderd/x/chatd/chaterror/message_test.go b/coderd/x/chatd/chaterror/message_test.go new file mode 100644 index 0000000000..07551d5d4c --- /dev/null +++ b/coderd/x/chatd/chaterror/message_test.go @@ -0,0 +1,99 @@ +package chaterror_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/x/chatd/chaterror" +) + +// TestTerminalMessage covers the per-provider "temporarily +// unavailable" copy, the startup-timeout copy, and the generic +// fallback string for its intended (unclassified, non-retryable) +// path. +func TestTerminalMessage(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + kind string + provider string + retryable bool + statusCode int + want string + }{ + { + name: "Timeout_Retryable_Anthropic", + kind: chaterror.KindTimeout, + provider: "anthropic", + retryable: true, + want: "Anthropic is temporarily unavailable.", + }, + { + name: "Timeout_Retryable_OpenAI", + kind: chaterror.KindTimeout, + provider: "openai", + retryable: true, + want: "OpenAI is temporarily unavailable.", + }, + { + name: "Timeout_Retryable_UnknownProvider", + kind: chaterror.KindTimeout, + provider: "", + retryable: true, + want: "The AI provider is temporarily unavailable.", + }, + { + name: "Timeout_NotRetryable_NoStatus", + kind: chaterror.KindTimeout, + provider: "", + retryable: false, + want: "The request timed out before it completed.", + }, + { + name: "StartupTimeout_Anthropic", + kind: chaterror.KindStartupTimeout, + provider: "anthropic", + retryable: true, + want: "Anthropic did not start responding in time.", + }, + { + name: "StartupTimeout_OpenAI", + kind: chaterror.KindStartupTimeout, + provider: "openai", + retryable: true, + want: "OpenAI did not start responding in time.", + }, + { + // Generic fallback reserved for genuinely + // unclassified non-retryable failures. + name: "Generic_NotRetryable_NoStatus", + kind: chaterror.KindGeneric, + provider: "", + retryable: false, + want: "The chat request failed unexpectedly.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + classified := chaterror.ClassifiedError{ + Kind: tt.kind, + Provider: tt.provider, + Retryable: tt.retryable, + StatusCode: tt.statusCode, + } + // terminalMessage is unexported; round-trip through + // WithClassification + Classify to exercise it. + wrapped := chaterror.WithClassification( + xerrors.New(tt.name), + classified, + ) + require.Equal(t, tt.want, chaterror.Classify(wrapped).Message) + }) + } +} diff --git a/coderd/x/chatd/chaterror/signals.go b/coderd/x/chatd/chaterror/signals.go index eb09a6d620..9f91e97e06 100644 --- a/coderd/x/chatd/chaterror/signals.go +++ b/coderd/x/chatd/chaterror/signals.go @@ -37,6 +37,17 @@ var ( "broken pipe", "bad gateway", "gateway timeout", + // "client conn" covers all of the stdlib http2 ClientConn errors: + // "client conn is closed", "client conn not usable", + // "client conn could not be established", + // "client connection force closed via ClientConn.Close", + // and "client connection lost". + "client conn", + // Transport-layer failures (HTTP/2 force-closed streams, + // GOAWAY, closed network connections) so we retry. + "goaway", + "http2: stream closed", + "use of closed network connection", } authStrongPatterns = []string{ "authentication", diff --git a/coderd/x/chatd/chatloop/chatloop_test.go b/coderd/x/chatd/chatloop/chatloop_test.go index a24bcee6e3..2e7539bdb8 100644 --- a/coderd/x/chatd/chatloop/chatloop_test.go +++ b/coderd/x/chatd/chatloop/chatloop_test.go @@ -491,6 +491,85 @@ func TestRun_RetriesStartupTimeoutWhileOpeningStream(t *testing.T) { } } +// TestRun_HTTP2TransportErrorClassifiedAsRetryableTimeout proves the +// provider comes from Model.Provider() (not from sniffing the error +// text) by using an error string with no provider hint and running +// the same assertion across two providers. +func TestRun_HTTP2TransportErrorClassifiedAsRetryableTimeout(t *testing.T) { + t.Parallel() + + providers := []string{"anthropic", "openai"} + for _, provider := range providers { + t.Run(provider, func(t *testing.T) { + t.Parallel() + + const startupTimeout = 5 * time.Millisecond + + ctx, cancel := context.WithTimeout( + context.Background(), + testutil.WaitShort, + ) + defer cancel() + + mClock := quartz.NewMock(t) + trap := mClock.Trap().AfterFunc("startupGuard") + defer trap.Close() + + attempts := 0 + var retries []chatretry.ClassifiedError + model := &chattest.FakeModel{ + ProviderName: provider, + StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) { + attempts++ + if attempts == 1 { + // Bare transport error; Provider must + // come from Model.Provider(). + return nil, xerrors.New( + "http2: client connection force closed via ClientConn.Close", + ) + } + return streamFromParts([]fantasy.StreamPart{{ + Type: fantasy.StreamPartTypeFinish, + FinishReason: fantasy.FinishReasonStop, + }}), nil + }, + } + + done := make(chan error, 1) + go func() { + done <- Run(context.Background(), RunOptions{ + Model: model, + MaxSteps: 1, + StartupTimeout: startupTimeout, + Clock: mClock, + PersistStep: func(_ context.Context, _ PersistedStep) error { + return nil + }, + OnRetry: func( + _ int, + _ error, + classified chatretry.ClassifiedError, + _ time.Duration, + ) { + retries = append(retries, classified) + }, + }) + }() + + // One guard per attempt. + trap.MustWait(ctx).MustRelease(ctx) + trap.MustWait(ctx).MustRelease(ctx) + + require.NoError(t, awaitRunResult(ctx, t, done)) + require.Equal(t, 2, attempts) + require.Len(t, retries, 1) + require.Equal(t, chaterror.KindTimeout, retries[0].Kind, "Kind") + require.True(t, retries[0].Retryable, "Retryable") + require.Equal(t, provider, retries[0].Provider, "Provider") + }) + } +} + func TestRun_RetriesStartupTimeoutBeforeFirstPart(t *testing.T) { t.Parallel() diff --git a/coderd/x/chatd/chatretry/chatretry_test.go b/coderd/x/chatd/chatretry/chatretry_test.go index 8d5f517339..640ffdf4e0 100644 --- a/coderd/x/chatd/chatretry/chatretry_test.go +++ b/coderd/x/chatd/chatretry/chatretry_test.go @@ -317,3 +317,24 @@ func TestRetry_UsesRetryAfterAsDelayFloor(t *testing.T) { }) } } + +// TestRetry_HTTP2TransportErrorKeepsRetrying proves a bare HTTP/2 +// transport error is treated as retryable, so Retry drives one more +// attempt instead of returning on the first call. +func TestRetry_HTTP2TransportErrorKeepsRetrying(t *testing.T) { + t.Parallel() + + calls := 0 + err := chatretry.Retry(context.Background(), func(_ context.Context) error { + calls++ + if calls == 1 { + return xerrors.New( + "http2: client connection force closed via ClientConn.Close", + ) + } + return nil + }, nil) + + require.NoError(t, err) + require.Equal(t, 2, calls, "expected one retry after an HTTP/2 transport failure") +} diff --git a/site/src/pages/AgentsPage/components/ChatConversation/LiveStreamTail.stories.tsx b/site/src/pages/AgentsPage/components/ChatConversation/LiveStreamTail.stories.tsx index 0907f16ba7..a6737fb61e 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/LiveStreamTail.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/LiveStreamTail.stories.tsx @@ -4,6 +4,7 @@ import { LiveStreamTailContent } from "./LiveStreamTail"; import { buildLiveStatus, buildReconnectState, + buildRetryState, buildStreamRenderState, FIXTURE_NOW, textResponseStreamParts, @@ -103,6 +104,91 @@ export const TerminalOverloadedError: Story = { }, }; +/** + * Transport timeouts render the per-provider "temporarily + * unavailable" copy with a "Request timed out" heading rather than + * the generic "Request failed" fallback. + */ +export const TerminalTimeoutErrorAnthropic: Story = { + args: { + ...defaultArgs, + liveStatus: buildLiveStatus({ + streamError: { + kind: "timeout", + message: "Anthropic is temporarily unavailable.", + provider: "anthropic", + retryable: false, + }, + }), + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect( + canvas.getByRole("heading", { name: /request timed out/i }), + ).toBeVisible(); + expect( + canvas.getByText(/anthropic is temporarily unavailable/i), + ).toBeVisible(); + // Guard against the pre-fix generic fallback. + expect( + canvas.queryByText(/the chat request failed unexpectedly/i), + ).not.toBeInTheDocument(); + }, +}; + +/** Transport timeout with an unknown provider uses the generic subject. */ +export const TerminalTimeoutErrorUnknownProvider: Story = { + args: { + ...defaultArgs, + liveStatus: buildLiveStatus({ + streamError: { + kind: "timeout", + message: "The AI provider is temporarily unavailable.", + retryable: false, + }, + }), + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect( + canvas.getByRole("heading", { name: /request timed out/i }), + ).toBeVisible(); + expect( + canvas.getByText(/the ai provider is temporarily unavailable/i), + ).toBeVisible(); + }, +}; + +/** Retrying a transport timeout shows attempt + countdown. */ +export const RetryingTimeoutAnthropic: Story = { + args: { + ...defaultArgs, + liveStatus: buildLiveStatus({ + retryState: buildRetryState({ + attempt: 2, + kind: "timeout", + error: "Anthropic is temporarily unavailable.", + provider: "anthropic", + }), + }), + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect( + canvas.getByRole("heading", { name: /request timed out/i }), + ).toBeVisible(); + expect( + canvas.getByText(/anthropic is temporarily unavailable/i), + ).toBeVisible(); + expect(canvas.getByText(/attempt 2/i)).toBeVisible(); + // StatusCountdown renders label and seconds as separate text + // nodes, so match against the element's combined textContent. + await waitFor(() => { + expect(canvasElement).toHaveTextContent(/retrying in \d+s/i); + }); + }, +}; + /** Terminal startup timeouts get a specific heading without provider metadata. */ export const TerminalStartupTimeoutError: Story = { args: {