diff --git a/apps/console/src/components/chat/ChatPage.tsx b/apps/console/src/components/chat/ChatPage.tsx index 70ce6050..50c1dc9f 100644 --- a/apps/console/src/components/chat/ChatPage.tsx +++ b/apps/console/src/components/chat/ChatPage.tsx @@ -49,7 +49,7 @@ 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 }; +type ChatFailure = { code: string; door?: string; status?: number; reason?: string }; /** * Classify a rejected chat request. @@ -62,14 +62,23 @@ type ChatFailure = { code: string; door?: string; status?: number }; */ function chatFailure(error: unknown, door: string): ChatFailure { if (error instanceof ChatWireError) { + const code = error.wireCode ?? 'console_chat_wire_failed'; return { - code: error.wireCode ?? 'console_chat_wire_failed', + code, door: error.door, status: error.status ?? undefined, + // These routes wrap an inner failure and put its reason in `message`. + // That reason is the only part naming what actually broke, so keep it + // unless it just repeats the code the sentence already covers. + reason: error.message && error.message !== code ? error.message : undefined, }; } // fetch itself rejected, so there is no status: the request never landed. - return { code: 'console_chat_wire_failed', door }; + return { + code: 'console_chat_wire_failed', + door, + reason: error instanceof Error ? error.message : undefined, + }; } function connectionFor(status: number | null, error?: string | null): ConnectionState { @@ -459,6 +468,7 @@ export function ChatPage({ ? degradationFor(loadError.code, { door: loadError.door, status: loadError.status, + reason: loadError.reason, }) : connection === 'disconnected' ? degradationFor('console_data_api_unreachable', transportOrigin) diff --git a/apps/console/src/lib/degradation.test.ts b/apps/console/src/lib/degradation.test.ts index cc8bb76c..726064e0 100644 --- a/apps/console/src/lib/degradation.test.ts +++ b/apps/console/src/lib/degradation.test.ts @@ -140,6 +140,59 @@ describe('degradationFor origin evidence', () => { expect(chat.detail).toContain('502'); }); + // The failing route knows why it failed and says so in its body. Dropping + // that left the reader with a category ("answered 502") and no cause, which + // is what turned a real 502 into another round trip. + it('surfaces the upstream reason alongside the door and status', () => { + const result = degradationFor('project_catalog_failed', { + door: '/api/chat/projects', + status: 502, + reason: 'objects/query failed: 502', + }); + expect(result.cause).toBe('The chat project list could not be read.'); + expect(result.detail).toContain('/api/chat/projects'); + expect(result.detail).toContain('502'); + expect(result.detail).toContain('objects/query failed'); + }); + + // CS15 still holds: a bare wire code is not prose, so it renders as its + // sentence rather than as the identifier. + it('translates a wire-code reason into its sentence', () => { + const result = degradationFor('project_catalog_failed', { + door: '/api/chat/projects', + status: 502, + reason: 'console_data_api_unreachable', + }); + expect(result.detail).toContain('The data API is unreachable.'); + expect(result.detail).not.toContain('console_data_api_unreachable'); + }); + + it('bounds a runaway reason so a stack trace cannot become the banner', () => { + const result = degradationFor('project_catalog_failed', { + door: '/api/chat/projects', + status: 502, + reason: 'x'.repeat(500), + }); + expect(result.detail!.length).toBeLessThan(320); + expect(result.detail).toContain('...'); + }); + + it('every chat route error code has a sentence of its own', () => { + const emitted = [ + 'project_catalog_failed', 'project_write_failed', 'project_select_failed', + 'thread_catalog_failed', 'thread_read_failed', 'thread_create_failed', + 'thread_update_failed', 'thread_not_found', 'attachment_upload_failed', + 'file_required', 'invalid_body', 'tenant_connector_unavailable', + 'web_search_requires_principal', 'console_chat_wire_failed', + 'web_search_unavailable', + ]; + const generic = degradationFor('a_code_that_is_not_mapped_at_all').cause; + for (const code of emitted) { + expect(degradationFor(code).cause, `${code} fell through to the generic sentence`) + .not.toBe(generic); + } + }); + 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 8a4dad19..7217ea45 100644 --- a/apps/console/src/lib/degradation.ts +++ b/apps/console/src/lib/degradation.ts @@ -29,6 +29,14 @@ export type DegradationOrigin = { host?: string; /** HTTP status, when the request was answered at all. Omit for no answer. */ status?: number; + /** + * What the failing route said went wrong, when it said anything. + * + * A route that wraps an inner failure knows the reason and puts it in its + * body; dropping it leaves the reader with a category ("answered 502") and + * no cause. Rendered through `readableReason`, which keeps CS15 intact. + */ + reason?: string; }; type DegradationTemplate = @@ -104,6 +112,86 @@ const WIRE_MAP: Record = { actionLabel: 'Retry', door: 'The chat wire', }, + // Every code the /api/chat/* routes can emit. Before these, all of them fell + // through to GENERIC_UNAVAILABLE, so a failed project read, a failed thread + // write and a missing attachment all said "This surface cannot render right + // now." An unmapped code is not a neutral default: it is a sentence that + // cannot be acted on. + project_catalog_failed: { + level: 'unavailable', + cause: 'The chat project list could not be read.', + actionLabel: 'Retry', + door: 'The chat project catalog', + }, + project_write_failed: { + level: 'unavailable', + cause: 'That chat project could not be saved.', + actionLabel: 'Retry', + door: 'The chat project catalog', + }, + project_select_failed: { + level: 'unavailable', + cause: 'That project could not be made active.', + actionLabel: 'Retry', + door: 'The chat project catalog', + }, + thread_catalog_failed: { + level: 'unavailable', + cause: 'The thread list could not be read.', + actionLabel: 'Retry', + door: 'The chat thread catalog', + }, + thread_read_failed: { + level: 'unavailable', + cause: 'That thread could not be read.', + actionLabel: 'Retry', + door: 'The chat thread catalog', + }, + thread_create_failed: { + level: 'unavailable', + cause: 'That thread could not be created.', + actionLabel: 'Retry', + door: 'The chat thread catalog', + }, + thread_update_failed: { + level: 'unavailable', + cause: 'That thread could not be saved.', + actionLabel: 'Retry', + door: 'The chat thread catalog', + }, + thread_not_found: { + level: 'unavailable', + cause: 'That thread no longer exists.', + door: 'The chat thread catalog', + }, + attachment_upload_failed: { + level: 'unavailable', + cause: 'That attachment could not be uploaded.', + actionLabel: 'Retry', + door: 'The chat attachment upload', + }, + file_required: { + level: 'unavailable', + cause: 'That upload arrived without a file.', + door: 'The chat attachment upload', + }, + invalid_body: { + level: 'unavailable', + cause: 'The console sent a request this route could not read.', + door: 'The chat wire', + }, + tenant_connector_unavailable: { + level: 'unavailable', + cause: 'No connector is available for this tenant.', + actionLabel: 'Open Account', + door: 'The chat wire', + }, + web_search_requires_principal: { + level: 'unavailable', + cause: 'Web search needs a signed-in principal.', + actionLabel: 'Open Account', + door: 'Web search', + }, web_search_unavailable: { level: 'reduced', cause: 'Web search is unavailable.', @@ -202,18 +290,47 @@ export function degradationFor( * credential problem rather than an outage. Returns undefined when there is * nothing concrete to say, so no caller is forced to show an empty line. */ +/** + * Render an upstream reason without leaking a wire code to the reader. + * + * Inner failures often surface as a bare code (`console_data_api_unreachable`) + * rather than prose. CS15 keeps codes off the screen, so a code is translated + * to its sentence; anything else is real prose already and passes through, + * bounded so a stack trace cannot become the banner. + */ +function readableReason(reason: string | undefined): string | undefined { + const trimmed = reason?.trim(); + if (!trimmed) return undefined; + const rendered = /^[a-z][a-z0-9_]*$/.test(trimmed) + // An unmapped bare code has no sentence to show, and inventing one would + // hide the only identifier the reader could search for. + ? (WIRE_MAP[trimmed]?.cause ?? trimmed) + : trimmed; + // Bound at the exit, not inside one branch. The code-shaped test matches any + // run of lowercase, so a long token took the other path and escaped the cap. + return rendered.length > 160 ? `${rendered.slice(0, 157)}...` : rendered; +} + export function describeOrigin(origin: DegradationOrigin | undefined): string | undefined { if (!origin) return undefined; const { door, host, status } = origin; - if (!door && !host && status === undefined) return undefined; + const reason = readableReason(origin.reason); + if (!door && !host && status === undefined) return reason; const subject = [door ?? 'The request', host ? `at ${host}` : null].filter(Boolean).join(' '); + // The reason is the innermost thing known about the failure, so it goes last + // and never replaces the door and status a reader needs to locate it. + const withReason = (sentence: string) => (reason ? `${sentence} ${reason}` : sentence); if (status === undefined) { - return `${subject} did not answer. That is DNS, the network, or a blocked origin, not a status code.`; + return withReason( + `${subject} did not answer. That is DNS, the network, or a blocked origin, not a status code.`, + ); } if (status === 401) { - return `${subject} answered 401. The request was not authenticated, which is a credential problem rather than an outage.`; + return withReason( + `${subject} answered 401. The request was not authenticated, which is a credential problem rather than an outage.`, + ); } if (status === 403) { // Not the same failure as 401, and saying so matters. connectionFor maps @@ -221,12 +338,16 @@ export function describeOrigin(origin: DegradationOrigin | undefined): string | // active_workspace_claim_required and active_workspace_membership_refused. // In those cases the credential is fine and simply does not reach this // workspace, so "fix your credential" would send the reader the wrong way. - return `${subject} answered 403. The credential was accepted but refused for this workspace, which is not an outage.`; + return withReason( + `${subject} answered 403. The credential was accepted but refused for this workspace, which is not an outage.`, + ); } if (status === 404) { - return `${subject} answered 404. Check the configured URL before assuming the service is down.`; + return withReason( + `${subject} answered 404. Check the configured URL before assuming the service is down.`, + ); } - return `${subject} answered ${status}.`; + return withReason(`${subject} answered ${status}.`); } /** Collapse a list of missing capability codes into one reduced marker. */