-
Notifications
You must be signed in to change notification settings - Fork 745
Improve error message clarity by specifying to check supervisor logs #6250
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
base: main
Are you sure you want to change the base?
Improve error message clarity by specifying to check supervisor logs #6250
Conversation
…with 'ha supervisor logs'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves error message clarity by providing specific instructions for users to check supervisor logs. Instead of the vague "Unknown error, see supervisor" message, users now receive actionable guidance with the specific command to run.
Key changes:
- Enhanced default error messages to include the CLI command
'ha supervisor logs' - Updated error handling across API utilities and job management system
- Ensured consistent messaging for unknown errors throughout the codebase
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| supervisor/api/utils.py | Updated default error message in API error handling to include specific CLI command |
| supervisor/jobs/init.py | Enhanced SupervisorJobError default message with actionable instruction |
| tests/jobs/test_job_manager.py | Updated test assertions to match new error message format |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I've fixed the linter error and changed it to "Supervisor" for consistency with other texts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Only thought, do we want to suggest they use the CLI here or include a my link to supervisor logs like https://my.home-assistant.io/redirect/logs/?provider=supervisor?
Yeah I was going to say, the more user friendly version are the logs available through the frontend, so we probably should suggest those primarily 🤔? it seems that nowadays they are shown even without Advanced mode, so everyone should be able to navigate there. The problem is that navigation changed at one point, but I think it did not the last 2 years so at least right now it should be the same for all users of the latest Supervisor version (Settings > System > Logs > Select "Supervisor" in the top right menu). But maybe instead just get rid of the unspecific error in more/all cases? Do you happen to have a concrete case in which people run into commonly we could improve? |
|
@agners That's why I suggested a my link. It doesn't matter what the navigation is now, it'll just open the Supervisor logs in the UI wherever we're putting them now. |
Yeah, but those exceptions get raised and pop up as a message on the bottom left. We can't use my links straight in the exception message. We'd have to catch the exception and present it somehow differently in the frontend I guess 🤔 |
|
So leave it as a CLI direction then? If it just pops up as a toast they won't be able to copy and paste that. But there's a better chance they'll remember |
Improve error message clarity by specifying to check supervisor logs with 'ha supervisor logs'
Proposed change
Type of change
This addresses a common user experience issue where users don't understand what to do when they see "Unknown error, see supervisor" messages.
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed: