Print the exact reason if the WebSocket event to Core fails #5609
Conversation
📝 WalkthroughWalkthroughThe changes update the error handling in the Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
supervisor/homeassistant/module.py (1)
361-364: LGTM! Consider minor formatting improvement.The change improves error reporting by including the specific error details, which aligns with the PR objective. However, the error message could be more concise.
Consider this minor improvement to make the error message more concise:
- f"Preparing backup of Home Assistant Core failed. Failed to inform HA Core: {str(err)}.", + f"Failed to prepare Home Assistant Core backup: {str(err)}",
agners
force-pushed
the
print-error-reason-if-ws-to-core-on-backup-fails
branch
from
1f78b65 to
717e747
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
supervisor/homeassistant/module.py (1)
379-382: Consider using %-formatting for logging.While the error handling improvement is good, the logging statement uses an f-string which is not recommended for logging. Consider using %-formatting instead.
Apply this diff to use the recommended logging format:
_LOGGER.warning( - f"Error resuming normal operations after backup of Home Assistant Core. Failed to inform HA Core: {str(err)}.", + "Error resuming normal operations after backup of Home Assistant Core. Failed to inform HA Core: %s.", + str(err), )🧰 Tools
🪛 Ruff (0.8.2)
381-381: Logging statement uses f-string
(G004)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
supervisor/homeassistant/module.py(2 hunks)tests/homeassistant/test_module.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
supervisor/homeassistant/module.py
381-381: Logging statement uses f-string
(G004)
🔇 Additional comments (4)
tests/homeassistant/test_module.py (3)
93-93: LGTM!The error message is now more specific and descriptive.
99-99: LGTM!The test assertion message correctly reflects the expected error format.
109-110: LGTM!The error messages are consistent with the new format and provide better context for debugging.
Also applies to: 115-115
supervisor/homeassistant/module.py (1)
362-362: LGTM!The error message now includes specific WebSocket error details, improving debuggability.
Proposed change
SSIA
Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed:
Summary by CodeRabbit