From 29fbfa06498fab9b16a8d45a8a861a734c791391 Mon Sep 17 00:00:00 2001 From: Travis Gilbert <1travisgilbert@gmail.com> Date: Sun, 2 Aug 2026 02:37:43 -0400 Subject: [PATCH] fix(console): stop blaming the data API for chat-route failures Travis hit "The data API is unreachable." on /workspace/{id}/chat while the data API was answering normally: commonplace-api returned 200 on /healthz and a fail-closed 401 on /graphql, and the console's own /api/objects/views proxy returned a correct 401. Nothing was unreachable. The banner came from the chat catalog fetch. fetchChatCatalog dials /api/chat/projects, a console route, and ChatPage mapped EVERY rejection from it to `console_data_api_unreachable`: loadError === 'workspace_object_scope_unenforced' ? 'workspace_object_scope_unenforced' : 'console_data_api_unreachable' // everything else So a 500, a timeout, or a parse error on the console's own chat route announced an outage on a different service. This is the same defect the degraded-state work set out to remove, left standing on the path users hit most, and #152/#154 did not touch it. Worse, that change removed the (uncorrelated) origin from this branch, guaranteeing it renders a bare sentence with no evidence at all. That is exactly the screenshot. readJson threw `new Error(message)`, discarding the status and the wire code the route returned, so the caller had nothing to classify with. It now throws a typed ChatWireError carrying door, status and wireCode, and every one of the six call sites passes the door it dialed. ChatPage reports the code the route actually named, falling back to `console_chat_wire_failed` when it named none, with that request's own door and status. `workspace_object_scope_unenforced` needs no special case now: it is a mapped code and resolves on its own. The disconnected branch is unchanged and still carries transportOrigin, because `connection` is derived from onTransport and that origin really is its outcome. vitest run src/lib/degradation.test.ts 14 passed (+2: a chat failure must not say "data api", and names the chat wire when the caller supplies no door) pnpm --filter @commonplace/console run build:railway exit 0 --- apps/console/src/components/chat/ChatPage.tsx | 60 ++++++++++++++----- apps/console/src/lib/chat/catalog-client.ts | 57 +++++++++++++++--- apps/console/src/lib/degradation.test.ts | 21 +++++++ apps/console/src/lib/degradation.ts | 1 + 4 files changed, 114 insertions(+), 25 deletions(-) diff --git a/apps/console/src/components/chat/ChatPage.tsx b/apps/console/src/components/chat/ChatPage.tsx index 962c624a..013e4b9b 100644 --- a/apps/console/src/components/chat/ChatPage.tsx +++ b/apps/console/src/components/chat/ChatPage.tsx @@ -27,6 +27,7 @@ import { ChatDropProvider } from '@/components/chat/ChatDropOverlay'; import { useChatPageRuntime } from '@/components/chat/runtime'; import type { ChatArtifactPayload, ChatCatalog, ChatThreadRecord } from '@/lib/chat/project-types'; import { + ChatWireError, createChatThread, fetchChatCatalog, fetchChatThread, @@ -47,6 +48,30 @@ const emptySubscribe = () => () => {}; const MESSAGE_PERSIST_DEBOUNCE_MS = 500; const EMPTY_CAPABILITIES: readonly CapabilityItem[] = []; +/** A failed chat request, kept with the evidence needed to describe it. */ +type ChatFailure = { code: string; door?: string; status?: number }; + +/** + * Classify a rejected chat request. + * + * `/api/chat/*` are the console's own routes. Before this, every rejection + * here was relabelled `console_data_api_unreachable`, so a 500 from + * /api/chat/projects told the reader the data API was down while it was + * answering normally. Report the code the route actually named, and when it + * named none, say the chat wire failed, because that is what happened. + */ +function chatFailure(error: unknown, door: string): ChatFailure { + if (error instanceof ChatWireError) { + return { + code: error.wireCode ?? 'console_chat_wire_failed', + door: error.door, + status: error.status ?? undefined, + }; + } + // fetch itself rejected, so there is no status: the request never landed. + return { code: 'console_chat_wire_failed', door }; +} + function connectionFor(status: number | null, error?: string | null): ConnectionState { if (status === 401 || error === 'principal_resolution=unauthenticated') return 'unauthenticated'; if ( @@ -235,7 +260,7 @@ export function ChatPage({ const connection = useShellStore((state) => state.connection); const [catalog, setCatalog] = useState(null); const [thread, setThread] = useState(null); - const [loadError, setLoadError] = useState(null); + const [loadError, setLoadError] = useState(null); const [railCollapsed, setRailCollapsed] = useState(false); const [wide, setWide] = useState(true); const [includeOverrides, setIncludeOverrides] = useState>(() => new Map()); @@ -292,7 +317,7 @@ export function ChatPage({ }) .catch((error: unknown) => { if (active) { - setLoadError(error instanceof Error ? error.message : 'catalog_unreachable'); + setLoadError(chatFailure(error, '/api/chat/projects')); } }); return () => { @@ -339,7 +364,7 @@ export function ChatPage({ router.replace(`/chat/${encodeURIComponent(created.id)}`); } catch (error) { if (active) { - setLoadError(error instanceof Error ? error.message : 'thread_unreachable'); + setLoadError(chatFailure(error, '/api/chat/threads')); } } }; @@ -417,20 +442,23 @@ export function ChatPage({ } : undefined; - // Only the disconnected branch may carry that evidence. `connection` is - // derived from onTransport, so the last transport outcome is genuinely its - // outcome. `loadError` is not: it comes from the chat catalog and thread - // fetches, which are different requests. A healthy /api/objects/views probe - // followed by a 502 from /api/chat/projects would otherwise render a banner - // claiming the data API answered 200. Evidence about the wrong request is - // worse than no evidence, which is the failure this whole change exists to - // stop. + // Each branch reports its own request, and only its own. + // + // `loadError` comes from /api/chat/*, the console's own routes. It used to be + // relabelled `console_data_api_unreachable`, so a 500 from + // /api/chat/projects announced that the data API was down while the data API + // was answering normally. It now carries the code the route named, plus the + // door and status of the request that actually failed. + // + // `connection` is derived from onTransport, so the last transport outcome is + // genuinely its outcome and may carry `transportOrigin`. Attaching that + // origin to a chat failure would describe the wrong request, which is worse + // than describing none. const degradation = loadError - ? degradationFor( - loadError === 'workspace_object_scope_unenforced' - ? 'workspace_object_scope_unenforced' - : 'console_data_api_unreachable', - ) + ? degradationFor(loadError.code, { + door: loadError.door, + status: loadError.status, + }) : connection === 'disconnected' ? degradationFor('console_data_api_unreachable', transportOrigin) : null; diff --git a/apps/console/src/lib/chat/catalog-client.ts b/apps/console/src/lib/chat/catalog-client.ts index 8e2b9dbe..62b84200 100644 --- a/apps/console/src/lib/chat/catalog-client.ts +++ b/apps/console/src/lib/chat/catalog-client.ts @@ -7,17 +7,56 @@ import type { ChatThreadRecord, } from '@/lib/chat/project-types'; -async function readJson(response: Response): Promise { +/** + * A chat-route failure that keeps its evidence instead of flattening it into a + * string. + * + * These routes are the console's own (`/api/chat/*`). They are NOT the data + * API. Throwing a bare Error meant the caller had nothing but a message to go + * on, so ChatPage labelled every one of them "The data API is unreachable." and + * pointed the reader at a service that was answering fine. + */ +export class ChatWireError extends Error { + /** The console route dialed, e.g. '/api/chat/projects'. */ + readonly door: string; + /** HTTP status, or null when the request never landed at all. */ + readonly status: number | null; + /** The wire code the route named, when it named one. */ + readonly wireCode: string | null; + + constructor(options: { + message: string; + door: string; + status: number | null; + wireCode?: string | null; + }) { + super(options.message); + this.name = 'ChatWireError'; + this.door = options.door; + this.status = options.status; + this.wireCode = options.wireCode ?? null; + } +} + +async function readJson(response: Response, door: string): Promise { if (!response.ok) { - const body = await response.json().catch(() => ({})) as { message?: string; error?: string }; - throw new Error(body.message ?? body.error ?? `chat catalog failed: ${response.status}`); + const body = (await response.json().catch(() => ({}))) as { + message?: string; + error?: string; + }; + throw new ChatWireError({ + message: body.message ?? body.error ?? `chat request failed: ${response.status}`, + door, + status: response.status, + wireCode: body.error ?? null, + }); } return response.json() as Promise; } export async function fetchChatCatalog(): Promise { const response = await fetch('/api/chat/projects', { cache: 'no-store' }); - return readJson(response); + return readJson(response, '/api/chat/projects'); } export async function saveChatProject( @@ -28,7 +67,7 @@ export async function saveChatProject( headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(project), }); - return readJson(response); + return readJson(response, '/api/chat/projects'); } export async function selectChatProject(projectId: string): Promise { @@ -37,7 +76,7 @@ export async function selectChatProject(projectId: string): Promise headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ activeProjectId: projectId }), }); - return readJson(response); + return readJson(response, '/api/chat/projects'); } export async function createChatThread(input: { @@ -50,14 +89,14 @@ export async function createChatThread(input: { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(input), }); - return readJson(response); + return readJson(response, '/api/chat/threads'); } export async function fetchChatThread(threadId: string): Promise { const response = await fetch(`/api/chat/threads/${encodeURIComponent(threadId)}`, { cache: 'no-store', }); - return readJson(response); + return readJson(response, `/api/chat/threads/${encodeURIComponent(threadId)}`); } export async function persistChatThread( @@ -69,7 +108,7 @@ export async function persistChatThread( headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(patch), }); - return readJson(response); + return readJson(response, `/api/chat/threads/${encodeURIComponent(threadId)}`); } export async function persistChatMessages( diff --git a/apps/console/src/lib/degradation.test.ts b/apps/console/src/lib/degradation.test.ts index 19e7f2d8..cc8bb76c 100644 --- a/apps/console/src/lib/degradation.test.ts +++ b/apps/console/src/lib/degradation.test.ts @@ -119,6 +119,27 @@ describe('degradationFor origin evidence', () => { expect(result.detail).toContain('503'); }); + // The console's /api/chat/* routes are not the data API. ChatPage used to + // relabel every chat rejection as console_data_api_unreachable, so a 500 + // from /api/chat/projects reported an outage on a service that was + // answering normally. A chat failure must describe the chat wire. + it('does not blame the data API for a chat-route failure', () => { + const chat = degradationFor('console_chat_wire_failed', { + door: '/api/chat/projects', + status: 500, + }); + expect(chat.cause.toLowerCase()).not.toContain('data api'); + expect(chat.cause.toLowerCase()).toContain('chat'); + expect(chat.detail).toContain('/api/chat/projects'); + expect(chat.detail).toContain('500'); + }); + + it('names the chat wire when the caller supplies no door', () => { + const chat = degradationFor('console_chat_wire_failed', { status: 502 }); + expect(chat.detail).toMatch(/chat wire/i); + expect(chat.detail).toContain('502'); + }); + it('still keeps the wire code out of what the user sees', () => { const result = degradationFor('console_data_api_unreachable', { door: '/api/objects/views', diff --git a/apps/console/src/lib/degradation.ts b/apps/console/src/lib/degradation.ts index 23a573d6..8a4dad19 100644 --- a/apps/console/src/lib/degradation.ts +++ b/apps/console/src/lib/degradation.ts @@ -102,6 +102,7 @@ const WIRE_MAP: Record = { level: 'unavailable', cause: 'The chat wire could not complete this turn.', actionLabel: 'Retry', + door: 'The chat wire', }, web_search_unavailable: { level: 'reduced',