feat: handle missing QR code #64
Conversation
If the QR code is not successfully generated, send the user a notification. A notification is chosen for persistence, otherwise text on the screen may be lost on boot. Closes #49
|
Warning Rate limit exceeded@mikejgray has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 26 minutes and 33 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces enhancements to the Home Assistant integration plugin, focusing on WebSocket connection improvements and OAuth error handling. The changes include updating the WebSocket connection to support larger message sizes, adding error handling for QR code retrieval during OAuth, and modifying dependency requirements. A test script is also included to demonstrate the plugin's instantiation with a specific configuration. Changes
Sequence DiagramsequenceDiagram
participant Plugin as HomeAssistantPlugin
participant WebSocket as WebSocket Connection
participant OAuth as OAuth Service
Plugin->>OAuth: Initiate OAuth Process
OAuth-->>Plugin: Request QR Code
alt QR Code Retrieved
Plugin->>Plugin: Process QR Code URL
else QR Code Not Retrieved
Plugin->>Plugin: Display Error Notification
end
Plugin->>WebSocket: Establish Connection (5 MB Max)
WebSocket-->>Plugin: Connection Established
Possibly related PRs
Suggested reviewers
Poem
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)
ovos_PHAL_plugin_homeassistant/logic/socketclient.py (1)
49-49: Consider making the WebSocket max_size configurable.The WebSocket message size limit is hardcoded to 5MB. Consider making this value configurable through the plugin's configuration to allow users to adjust it based on their needs.
- self.websocket = await websockets.connect(uri=uri, close_timeout=5, open_timeout=5, max_size=5242880) + max_size = self.config.get("websocket_max_size", 5242880) # Default to 5MB + self.websocket = await websockets.connect(uri=uri, close_timeout=5, open_timeout=5, max_size=max_size)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
DO_NOT_COMMIT.py(1 hunks)ovos_PHAL_plugin_homeassistant/__init__.py(1 hunks)ovos_PHAL_plugin_homeassistant/logic/socketclient.py(1 hunks)requirements.txt(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.21.2)
DO_NOT_COMMIT.py
6-6: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🔇 Additional comments (3)
DO_NOT_COMMIT.py (1)
1-11:⚠️ Potential issueRemove this file or move it to a test directory.
This appears to be a test file that should not be committed to the repository. If this is test code, consider:
- Moving it to a proper test directory.
- Using environment variables or configuration files for sensitive data.
- Adding it to
.gitignore.Additionally:
- The JWT token is exposed, which is a security risk.
- The WebSocket URL is hardcoded with an IP address, making it environment-specific.
Let's verify if this file is already in
.gitignore:🧰 Tools
🪛 Gitleaks (8.21.2)
6-6: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
ovos_PHAL_plugin_homeassistant/__init__.py (1)
886-888: LGTM! Good error handling improvement.The added check for missing QR code and user notification improves the error handling, providing better feedback to users when QR code generation fails.
requirements.txt (1)
8-8: LGTM! Good version constraints with clear comments.The version constraints are well-documented with comments explaining the reasons for the version limits.
Let's verify the latest versions and any known vulnerabilities:
Also applies to: 11-11
✅ Verification successful
Version constraints are secure and well-justified ✅
The specified version range (>=0.54.0,<14.0) is secure and avoids all known vulnerabilities. The upper bound restriction is well-documented, preventing exposure to breaking changes in 14.0+.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check latest versions and vulnerabilities for websockets packages # Check PyPI for latest versions echo "Latest versions from PyPI:" curl -s https://pypi.org/pypi/websockets/json | jq -r '.info.version' curl -s https://pypi.org/pypi/websocket-client/json | jq -r '.info.version' # Check for security advisories echo -e "\nSecurity advisories from GitHub:" gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: PIP, package: "websockets") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }' 'Length of output: 1213
mikejgray
force-pushed
the
FEAT_Multiple
branch
from
d17f1ae to
c8018d7
Compare
mikejgray
force-pushed
the
FEAT_Multiple
branch
from
c8018d7 to
81a08a2
Compare
chore: allow newer versions of websockets
fix: increase websocket max size to 5MB from 1MB
Closes a number of issues ahead of the merge of this plugin with its associated skill.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Dependencies
websocketsand addedwebsocket-clientpackage