opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix!: persist structured chat errors (#24919)

Browse Source 
**Breaking change for changelog:**

> `codersdk.Chat.last_error` now returns a structured `ChatError` object
(`{message, kind, provider, retryable, status_code, detail}`) instead of
a plain string. The chats API is experimental
(`/api/experimental/chats`), so this ships without a deprecation cycle;
consumers reading `chat.last_error` as a string must update to read
`chat.last_error.message`. SDK/generated TypeScript terminal error
payloads now use the single `ChatError` type; the live stream error
payload type is renamed from `ChatStreamError` to `ChatError`.

Persisted chat errors now carry the same provider-specific detail (kind,
provider, retryable, HTTP status, optional detail) as the live stream,
so refreshing a failed chat rehydrates with the full structured error
instead of a one-line headline.

Existing rows are migrated in place: legacy text errors are wrapped into
`{message, kind: "generic"}` so already-errored chats still render, and
rows with `last_error IS NULL` stay NULL. Internally, persisted fallback
decoding now reuses the existing `chaterror.KindGeneric` constant, with
no JSON value change.

Closes CODAGT-239
This commit is contained in:
Ethan
2026-05-05 12:56:06 +10:00
committed by GitHub
parent 7e01edeb8e
commit 4751416b29
47 changed files with 599 additions and 256 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/database/db2sdk/db2sdk.go
+31 -3
View File
@@ -28,6 +28,7 @@ import (
"github.com/coder/coder/v2/coderd/util/ptr"
"github.com/coder/coder/v2/coderd/util/slice"
"github.com/coder/coder/v2/coderd/workspaceapps/appurl"
"github.com/coder/coder/v2/coderd/x/chatd/chaterror"
"github.com/coder/coder/v2/coderd/x/chatd/chatprompt"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/provisionersdk/proto"
@@ -1607,6 +1608,34 @@ func nullTimePtr(v sql.NullTime) *time.Time {
return &value
}
const fallbackChatLastErrorMessage = "The chat request failed unexpectedly."
func decodeChatLastError(raw pqtype.NullRawMessage) *codersdk.ChatError {
if !raw.Valid {
return nil
}
var payload codersdk.ChatError
if err := json.Unmarshal(raw.RawMessage, &payload); err != nil {
return &codersdk.ChatError{
Message: fallbackChatLastErrorMessage,
Kind: chaterror.KindGeneric,
}
}
payload.Message = strings.TrimSpace(payload.Message)
payload.Detail = strings.TrimSpace(payload.Detail)
payload.Kind = strings.TrimSpace(payload.Kind)
payload.Provider = strings.TrimSpace(payload.Provider)
if payload.Kind == "" {
payload.Kind = chaterror.KindGeneric
}
if payload.Message == "" {
payload.Message = fallbackChatLastErrorMessage
}
return &payload
}
// Chat converts a database.Chat to a codersdk.Chat. It coalesces
// nil slices and maps to empty values for JSON serialization and
// derives RootChatID from the parent chain when not explicitly set.
@@ -1622,6 +1651,7 @@ func Chat(c database.Chat, diffStatus *database.ChatDiffStatus, files []database
if labels == nil {
labels = map[string]string{}
}
lastError := decodeChatLastError(c.LastError)
chat := codersdk.Chat{
ID: c.ID,
OrganizationID: c.OrganizationID,
@@ -1636,9 +1666,7 @@ func Chat(c database.Chat, diffStatus *database.ChatDiffStatus, files []database
MCPServerIDs: mcpServerIDs,
Labels: labels,
ClientType: codersdk.ChatClientType(c.ClientType),
}
if c.LastError.Valid {
chat.LastError = &c.LastError.String
LastError: lastError,
}
if c.PlanMode.Valid {
chat.PlanMode = codersdk.ChatPlanMode(c.PlanMode.ChatPlanMode)
coderd/database/db2sdk/db2sdk_test.go
+93 -1
View File
@@ -19,6 +19,7 @@ import (
"github.com/coder/coder/v2/coderd/database/dbgen"
"github.com/coder/coder/v2/coderd/database/dbtestutil"
"github.com/coder/coder/v2/coderd/database/dbtime"
"github.com/coder/coder/v2/coderd/x/chatd/chaterror"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/provisionersdk/proto"
)
@@ -916,6 +917,17 @@ func TestChat_AllFieldsPopulated(t *testing.T) {
// field to codersdk.Chat, this test will fail until the
// converter is updated.
now := dbtime.Now()
lastErrorPayload := codersdk.ChatError{
Message: "boom",
Detail: "provider detail",
Kind: chaterror.KindGeneric,
Provider: "openai",
Retryable: true,
StatusCode: 503,
}
lastErrorRaw, err := json.Marshal(lastErrorPayload)
require.NoError(t, err)
input := database.Chat{
ID: uuid.New(),
OwnerID: uuid.New(),
@@ -929,7 +941,7 @@ func TestChat_AllFieldsPopulated(t *testing.T) {
Title: "all-fields-test",
Status: database.ChatStatusRunning,
ClientType: database.ChatClientTypeUi,
LastError: sql.NullString{String: "boom", Valid: true},
LastError: pqtype.NullRawMessage{RawMessage: lastErrorRaw, Valid: true},
CreatedAt: now,
UpdatedAt: now,
Archived: true,
@@ -970,6 +982,8 @@ func TestChat_AllFieldsPopulated(t *testing.T) {
got := db2sdk.Chat(input, diffStatus, fileRows)
require.Equal(t, &lastErrorPayload, got.LastError)
v := reflect.ValueOf(got)
typ := v.Type()
// HasUnread is populated by ChatRowsWithChildren (which joins the
@@ -1053,6 +1067,84 @@ func TestChat_NilFilesOmitted(t *testing.T) {
require.Empty(t, result.Files)
}
func TestChat_LastErrorFallback(t *testing.T) {
t.Parallel()
const fallbackMessage = "The chat request failed unexpectedly."
tests := []struct {
name string
raw json.RawMessage
expectPayload *codersdk.ChatError
}{
{
name: "MalformedJSON",
raw: json.RawMessage(`{`),
expectPayload: &codersdk.ChatError{
Message: fallbackMessage,
Kind: chaterror.KindGeneric,
Retryable: false,
},
},
{
name: "MessageMissingPreservesMetadata",
raw: json.RawMessage(`{"kind":"timeout","provider":"openai","status_code":504}`),
expectPayload: &codersdk.ChatError{
Message: fallbackMessage,
Kind: "timeout",
Provider: "openai",
Retryable: false,
StatusCode: 504,
},
},
{
name: "WhitespaceMessageDefaultsKind",
raw: json.RawMessage(`{"message":" ","provider":"openai"}`),
expectPayload: &codersdk.ChatError{
Message: fallbackMessage,
Kind: chaterror.KindGeneric,
Provider: "openai",
Retryable: false,
},
},
{
name: "KindMissingDefaultsGeneric",
raw: json.RawMessage(`{"message":"OpenAI returned an unexpected error.","provider":"openai","status_code":502}`),
expectPayload: &codersdk.ChatError{
Message: "OpenAI returned an unexpected error.",
Kind: chaterror.KindGeneric,
Provider: "openai",
Retryable: false,
StatusCode: 502,
},
},
}
for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
chat := database.Chat{
ID: uuid.New(),
OwnerID: uuid.New(),
LastModelConfigID: uuid.New(),
Title: "fallback payload",
Status: database.ChatStatusError,
CreatedAt: dbtime.Now(),
UpdatedAt: dbtime.Now(),
LastError: pqtype.NullRawMessage{
RawMessage: tc.raw,
Valid: true,
},
}
result := db2sdk.Chat(chat, nil, nil)
require.Equal(t, tc.expectPayload, result.LastError)
})
}
}
func TestChat_MultipleFiles(t *testing.T) {
t.Parallel()
coderd/database/dump.sql
Generated
+1 -1
View File
@@ -1438,7 +1438,7 @@ CREATE TABLE chats (
root_chat_id uuid,
last_model_config_id uuid NOT NULL,
archived boolean DEFAULT false NOT NULL,
last_error text,
last_error jsonb,
mode chat_mode,
mcp_server_ids uuid[] DEFAULT '{}'::uuid[] NOT NULL,
labels jsonb DEFAULT '{}'::jsonb NOT NULL,
coderd/database/migrations/000485_chat_last_error_jsonb.down.sql
+3
View File
@@ -0,0 +1,3 @@
ALTER TABLE chats
ALTER COLUMN last_error TYPE text
USING last_error ->> 'message';
coderd/database/migrations/000485_chat_last_error_jsonb.up.sql
+9
View File
@@ -0,0 +1,9 @@
ALTER TABLE chats
ALTER COLUMN last_error TYPE jsonb
USING CASE
WHEN last_error IS NULL THEN NULL
ELSE jsonb_build_object(
'message', last_error,
'kind', 'generic'
)
END;
coderd/database/migrations/testdata/fixtures/000424_chat_last_error.up.sql
Vendored
+27
View File
@@ -0,0 +1,27 @@
-- Migration 424 adds chats.last_error as text. Seed one existing fixture
-- chat with a legacy plain-text error so migration 485 has a non-null row
-- to backfill, and add a second chat that leaves last_error NULL so the
-- migration fixture can assert both branches of the CASE expression.
UPDATE chats
SET last_error = 'Legacy provider failure'
WHERE id = '72c0438a-18eb-4688-ab80-e4c6a126ef96';
INSERT INTO chats (
id,
owner_id,
last_model_config_id,
title,
status,
created_at,
updated_at
)
SELECT
'5a4ac6a3-9dc5-440f-ae6b-5805e477bc59',
owner_id,
last_model_config_id,
'Fixture Chat With Null Error',
'waiting',
'2024-01-01 00:00:00+00',
'2024-01-01 00:00:00+00'
FROM chats
WHERE id = '72c0438a-18eb-4688-ab80-e4c6a126ef96';
coderd/database/migrations/testdata/fixtures/000485_chat_last_error_jsonb.up.sql
Vendored
+28
View File
@@ -0,0 +1,28 @@
-- Migration 485 retypes chats.last_error to jsonb and backfills legacy
-- text rows into the structured persisted payload shape.
DO $$
DECLARE
payload jsonb;
BEGIN
SELECT last_error INTO STRICT payload
FROM chats
WHERE id = '72c0438a-18eb-4688-ab80-e4c6a126ef96';
IF payload ->> 'message' <> 'Legacy provider failure' THEN
RAISE EXCEPTION 'expected migrated last_error message, got %',
payload ->> 'message';
END IF;
IF payload ->> 'kind' <> 'generic' THEN
RAISE EXCEPTION 'expected migrated last_error kind, got %',
payload ->> 'kind';
END IF;
PERFORM 1
FROM chats
WHERE id = '5a4ac6a3-9dc5-440f-ae6b-5805e477bc59'
AND last_error IS NULL;
IF NOT FOUND THEN
RAISE EXCEPTION 'expected null last_error row to remain NULL after migration';
END IF;
END $$;
coderd/database/models.go
+1 -1
View File
@@ -4367,7 +4367,7 @@ type Chat struct {
RootChatID uuid.NullUUID `db:"root_chat_id" json:"root_chat_id"`
LastModelConfigID uuid.UUID `db:"last_model_config_id" json:"last_model_config_id"`
Archived bool `db:"archived" json:"archived"`
LastError sql.NullString `db:"last_error" json:"last_error"`
LastError pqtype.NullRawMessage `db:"last_error" json:"last_error"`
Mode NullChatMode `db:"mode" json:"mode"`
MCPServerIDs []uuid.UUID `db:"mcp_server_ids" json:"mcp_server_ids"`
Labels StringMap `db:"labels" json:"labels"`
coderd/database/queries.sql.go
+16 -16
View File
@@ -5400,7 +5400,7 @@ type AutoArchiveInactiveChatsRow struct {
RootChatID uuid.NullUUID `db:"root_chat_id" json:"root_chat_id"`
LastModelConfigID uuid.UUID `db:"last_model_config_id" json:"last_model_config_id"`
Archived bool `db:"archived" json:"archived"`
LastError sql.NullString `db:"last_error" json:"last_error"`
LastError pqtype.NullRawMessage `db:"last_error" json:"last_error"`
Mode NullChatMode `db:"mode" json:"mode"`
MCPServerIDs []uuid.UUID `db:"mcp_server_ids" json:"mcp_server_ids"`
Labels json.RawMessage `db:"labels" json:"labels"`
@@ -8701,7 +8701,7 @@ SET
worker_id = $2::uuid,
started_at = $3::timestamptz,
heartbeat_at = $4::timestamptz,
last_error = $5::text,
last_error = $5::jsonb,
updated_at = NOW()
WHERE
id = $6::uuid
@@ -8710,12 +8710,12 @@ RETURNING
`
type UpdateChatStatusParams struct {
Status ChatStatus `db:"status" json:"status"`
WorkerID uuid.NullUUID `db:"worker_id" json:"worker_id"`
StartedAt sql.NullTime `db:"started_at" json:"started_at"`
HeartbeatAt sql.NullTime `db:"heartbeat_at" json:"heartbeat_at"`
LastError sql.NullString `db:"last_error" json:"last_error"`
ID uuid.UUID `db:"id" json:"id"`
Status ChatStatus `db:"status" json:"status"`
WorkerID uuid.NullUUID `db:"worker_id" json:"worker_id"`
StartedAt sql.NullTime `db:"started_at" json:"started_at"`
HeartbeatAt sql.NullTime `db:"heartbeat_at" json:"heartbeat_at"`
LastError pqtype.NullRawMessage `db:"last_error" json:"last_error"`
ID uuid.UUID `db:"id" json:"id"`
}
func (q *sqlQuerier) UpdateChatStatus(ctx context.Context, arg UpdateChatStatusParams) (Chat, error) {
@@ -8768,7 +8768,7 @@ SET
worker_id = $2::uuid,
started_at = $3::timestamptz,
heartbeat_at = $4::timestamptz,
last_error = $5::text,
last_error = $5::jsonb,
updated_at = $6::timestamptz
WHERE
id = $7::uuid
@@ -8777,13 +8777,13 @@ RETURNING
`
type UpdateChatStatusPreserveUpdatedAtParams struct {
Status ChatStatus `db:"status" json:"status"`
WorkerID uuid.NullUUID `db:"worker_id" json:"worker_id"`
StartedAt sql.NullTime `db:"started_at" json:"started_at"`
HeartbeatAt sql.NullTime `db:"heartbeat_at" json:"heartbeat_at"`
LastError sql.NullString `db:"last_error" json:"last_error"`
UpdatedAt time.Time `db:"updated_at" json:"updated_at"`
ID uuid.UUID `db:"id" json:"id"`
Status ChatStatus `db:"status" json:"status"`
WorkerID uuid.NullUUID `db:"worker_id" json:"worker_id"`
StartedAt sql.NullTime `db:"started_at" json:"started_at"`
HeartbeatAt sql.NullTime `db:"heartbeat_at" json:"heartbeat_at"`
LastError pqtype.NullRawMessage `db:"last_error" json:"last_error"`
UpdatedAt time.Time `db:"updated_at" json:"updated_at"`
ID uuid.UUID `db:"id" json:"id"`
}
func (q *sqlQuerier) UpdateChatStatusPreserveUpdatedAt(ctx context.Context, arg UpdateChatStatusPreserveUpdatedAtParams) (Chat, error) {
coderd/database/queries/chats.sql
+2 -2
View File
@@ -718,7 +718,7 @@ SET
worker_id = sqlc.narg('worker_id')::uuid,
started_at = sqlc.narg('started_at')::timestamptz,
heartbeat_at = sqlc.narg('heartbeat_at')::timestamptz,
last_error = sqlc.narg('last_error')::text,
last_error = sqlc.narg('last_error')::jsonb,
updated_at = NOW()
WHERE
id = @id::uuid
@@ -733,7 +733,7 @@ SET
worker_id = sqlc.narg('worker_id')::uuid,
started_at = sqlc.narg('started_at')::timestamptz,
heartbeat_at = sqlc.narg('heartbeat_at')::timestamptz,
last_error = sqlc.narg('last_error')::text,
last_error = sqlc.narg('last_error')::jsonb,
updated_at = @updated_at::timestamptz
WHERE
id = @id::uuid