fix(console): stop blaming the data API for chat-route failures - #155
Conversation
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
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
Pull request overview
This PR fixes mislabelled degraded-state messaging on the Console chat route (/workspace/{id}/chat) by preserving and reporting evidence (door + status + route wire code) from failures of the console’s own /api/chat/* routes, instead of incorrectly mapping them to “The data API is unreachable.”
Changes:
- Introduces a typed
ChatWireErrorthat preserves route door, HTTP status, and wire code when/api/chat/*requests fail. - Updates
ChatPageto classify chat failures using the route’s named wire code (orconsole_chat_wire_failed) and attach the failing request’s door/status to the degradation banner. - Adds degradation mapping/test coverage so chat-route failures describe the chat wire (and no longer mention “data api”).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/console/src/lib/degradation.ts | Adds a door default for console_chat_wire_failed so evidence can fall back to “The chat wire” when no door is provided. |
| apps/console/src/lib/degradation.test.ts | Adds tests pinning that chat-route failures do not blame the data API and that the chat wire is named when no door is supplied. |
| apps/console/src/lib/chat/catalog-client.ts | Adds ChatWireError and updates JSON reading to throw typed errors with door/status/wireCode instead of flattening to Error(message). |
| apps/console/src/components/chat/ChatPage.tsx | Switches loadError to structured ChatFailure and uses it to drive correct degradation codes and evidence for chat-route failures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch (error) { | ||
| if (active) { | ||
| setLoadError(error instanceof Error ? error.message : 'thread_unreachable'); | ||
| setLoadError(chatFailure(error, '/api/chat/threads')); | ||
| } | ||
| } |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29fbfa0649
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| message: body.message ?? body.error ?? `chat request failed: ${response.status}`, | ||
| door, | ||
| status: response.status, | ||
| wireCode: body.error ?? null, |
There was a problem hiding this comment.
When the object seam returns a known code without a message, responseError turns that code into the thrown error message, and /api/chat/projects wraps it as { error: 'project_catalog_failed', message: '<inner code>' }. Selecting only body.error here therefore discards codes such as workspace_object_scope_unenforced; ChatPage passes the unmapped wrapper to degradationFor and shows the generic unavailable sentence instead of the existing scope-specific guidance. The previous path deliberately recognized that inner code, so preserve recognized message codes or return the underlying code as a separate structured field.
Useful? React with 👍 / 👎.
| // fetch itself rejected, so there is no status: the request never landed. | ||
| return { code: 'console_chat_wire_failed', door }; |
There was a problem hiding this comment.
This fallback handles every non-ChatWireError, not only a rejected fetch. If /api/chat/projects or a thread route answers successfully but its JSON is malformed or truncated, response.json() throws a SyntaxError and this branch records no status; degradationFor then tells the user that the route did not answer and attributes it to DNS, the network, or a blocked origin even though the route did answer. Wrap successful-response decoding failures with the response status, or otherwise classify them separately from transport rejection.
Useful? React with 👍 / 👎.
Travis hit "The data API is unreachable." on
/workspace/{id}/chatwhile the data API was answering normally.Measured at the time:
commonplace-api…/healthzcommonplace-api…/graphql/api/objects/viewsprincipal_resolution=unauthenticatedCONSOLE_DATA_API_URLhttp://commonplace-api.railway.internal:8080, correctNothing was unreachable. The config was right. The banner was wrong.
Where it came from
fetchChatCatalogdials/api/chat/projects— a console route, not the data API. ChatPage mapped every rejection from it toconsole_data_api_unreachable:A 500, a timeout, or a JSON parse error on the console's own chat route announced an outage on a different service entirely.
This is the same defect the degraded-state work existed to remove, left standing on the path users hit most. #152/#154 did not fix it — and worse, that change removed the (uncorrelated) origin from this branch, so it now renders a bare sentence with no evidence at all. That is precisely the reported screenshot: the sentence, twice, and nothing else.
The fix
readJsonthrewnew Error(message), discardingresponse.statusand the wire code the route returned, so the caller had nothing to classify with. It now throws a typed error:All six call sites in
catalog-client.tspass the door they dialed. ChatPage reports the code the route actually named, falling back toconsole_chat_wire_failedwhen it named none, with that request's own door and status.workspace_object_scope_unenforcedneeds no special case any more: it is a mapped code and resolves on its own.The
disconnectedbranch is untouched and still carriestransportOrigin, becauseconnectionis derived fromonTransportand that origin genuinely is its outcome.What the banner says now
/api/chat/projects/api/chat/projectsanswered 500.Verification
vitest run src/lib/degradation.test.tspnpm --filter @commonplace/console run build:railwayThe two new tests pin the thing that went wrong: a chat failure must not contain "data api", and it names the chat wire when the caller supplies no door.
Note
/api/versionnow serves onv2.theoremharness.comreportingsha 267600cc, so #154 is deployed and the version endpoint works. This PR fixes the labelling that #154 left behind.