Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a clearer error message when settings are missing midsession #6158

Merged

Conversation

raymyers
Copy link
Contributor

@raymyers raymyers commented Jan 8, 2025

End-user friendly description of the problem this fixes or functionality that this introduces

  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

Give a summary of what the PR does, explaining any non-trivial design decisions

The changes in #6101 made ConnectionRefusedError thrown in listen_socket.py display to the client. One of those scenarios is "Settings not found". This change gives those an optional "msg_id" for the translation and adds a user facing message that makes it more clear what action to take.

Another place this could be addressed is in frontend/src/hooks/query/use-settings.ts, but I'm not sure how to approach that and making these Websocket errors more first-class seemed like a good direction.


Link of any specific issues this addresses
ALL-959

@raymyers
Copy link
Contributor Author

raymyers commented Jan 8, 2025

New error

Screenshot from 2025-01-08 16-56-44

err.txt Outdated Show resolved Hide resolved
@raymyers raymyers force-pushed the ray/all-956-user-story-2-nobad-llm-key-set branch from f4aa5f2 to 276d96a Compare January 8, 2025 23:31
Copy link
Member

@amanape amanape left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

frontend/src/context/ws-client-provider.tsx Show resolved Hide resolved
frontend/src/i18n/translation.json Show resolved Hide resolved
frontend/src/i18n/translation.json Show resolved Hide resolved
frontend/src/context/ws-client-provider.tsx Show resolved Hide resolved
@raymyers raymyers force-pushed the ray/all-956-user-story-2-nobad-llm-key-set branch from 705d678 to 9da0b4b Compare January 9, 2025 17:14
@raymyers raymyers force-pushed the ray/all-956-user-story-2-nobad-llm-key-set branch from 9da0b4b to bca9b38 Compare January 9, 2025 18:39
@amanape amanape enabled auto-merge (squash) January 9, 2025 18:52
@amanape amanape merged commit 8907fed into All-Hands-AI:main Jan 9, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants