Repository navigation
fix(web): keep a failed connect's reason on the server card (#2783) - #2801
Merged
Merged
Conversation
The connect-failure toast auto-closes after a few seconds, after which the server card showed only a red "Failed" with no reason. The reason was already held in connectErrorMessage but only exposed as a data attribute for automation. Thread it to the errored card and render it under the header, in compact mode too, so a failure like the legacy SSE endpoint origin mismatch stays readable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>
There was a problem hiding this comment.
🟢 Approval recommended
The scoped diagnostic display correctly reuses existing error state, has focused test coverage, and has no identified blocking issues.
No open issues 🎉
What changed in this PR
This PR improves web-client diagnostics for #2783 by keeping the failed connection’s reason visible on its server card after the toast disappears. It does not change SSE transport behavior or claim to fix the unreproduced connection failure.
Changes:
- Passes the existing connection error to the failed server’s card, including in compact mode.
- Adds themed error text with wrapping for long messages.
- Adds visibility and server-targeting tests plus a diagnostic Storybook example.
| File | Description |
|---|---|
| clients/web/src/theme/Text.ts | Adds error-text styling and wrapping. |
| clients/web/src/components/views/InspectorView/types.ts | Documents the error message’s card display. |
| clients/web/src/components/views/InspectorView/InspectorView.tsx | Passes the error message to the server list. |
| clients/web/src/components/screens/ServerListScreen/ServerListScreen.tsx | Targets the message to the failed server. |
| clients/web/src/components/screens/ServerListScreen/ServerListScreen.test.tsx | Tests failed-card targeting. |
| clients/web/src/components/groups/ServerCard/ServerCard.tsx | Displays the failure reason in normal and compact modes. |
| clients/web/src/components/groups/ServerCard/ServerCard.test.tsx | Tests error-message visibility conditions. |
| clients/web/src/components/groups/ServerCard/ServerCard.stories.tsx | Adds a failed-card example with a long reason. |
Review effort: Balanced
Member
Author
|
Copilot review closed: round 1 came back clean (no inline comments, no suppressed findings, approval recommended), so no further rounds. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2783
Not reproducible: this PR improves diagnostics only
#2783 could not be reproduced. I tested against a real Python MCP SDK 1.x
FastMCPserver on the legacy SSE transport. It sends CRLF framing and a relative/messages/?session_id=…endpoint, the shape the report describes. Against it, the web client connected and listed tools, both with and without--header "Authorization: Bearer …", and the CLI did too. The reporter's setup has two differences I couldn't test: HTTPS through a cloud load balancer, and Inspector sendingaccept-encoding: gzip, deflateanduser-agent: nodewhere theircurl -Nsent neither.What I did find is why the report contains no error text. One failure path matches all three reported symptoms: only the
GET /sseis logged, the stream closes after one event, and the card showsFailed. That path is the SDK rejecting theendpointevent (for exampleEndpoint origin does not match connection origin) and closing the transport. On that path the backend returns the reason, and the browser shows it, but only in a toast that auto-closes after about 4s. After that the card says onlyFailed, which is all the reporter had. The reason was already stored inconnectErrorMessage, but the only place it was exposed was adata-error-messageattribute for automation.Reporter: once this ships, please retry, and if it still fails, open a new issue with the error text the server card now shows under
Failed. That message should name the cause this issue couldn't pin down.Change
ServerCardtakes anerrorMessageprop and renders it under the header while the card iserrored. This includes compact mode, where everything else below the header is hidden.ServerListScreenpassesconnectErrorMessageonly to theerroredServerIdcard, andInspectorViewthreads it through.connectErrorTexttheme variant: danger-text color, and wrapping, so a long transport error carrying a URL or JSON body stays inside the card.ErroredWithReasonstory.Screenshots
Both were taken 6s after toggling Connect, once the toast had closed, against a stub legacy-SSE server that sends a cross-origin
endpointevent.Before: the card shows
Failedand nothing else.After: the reason stays on the card.
npm run local:gatepassed.🤖 Generated with Claude Code