-
Notifications
You must be signed in to change notification settings - Fork 0
Development #33
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
Development #33
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
…4b50-93c4-0e11b13594f4 Clarify error handler ordering is correct - no changes needed
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
…47b6-b204-5bf8892748d3 Extract magic numbers to named constants in audio playback code
…4606-8149-97ceaa22873d Use platform.system() instead of sys.platform for OS detection
Extract duplicated hex opacity calculation to utility function
Extract avatar active offset magic number into configurable setting
Fix audio reference cleanup on play() failure in popup mode
Fix race condition in popup avatar lifecycle
✅ Windows Build SuccessfulExecutable: Build Status
Download the artifacts from the workflow run to test before merging. Once merged to |
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 contains multiple improvements and refactoring changes for version 1.3.1, focusing on code organization, error handling, and robustness.
Key changes:
- Refactored queue manager imports from
apptomodules.queue_managermodule across multiple routers for better separation of concerns - Enhanced YouTube authentication error handling with specific
RefreshErrorhandling and improved user-facing error messages - Added test credential checking to conditionally skip Twitch integration tests when credentials are unavailable
- Implemented custom asyncio exception handler to suppress harmless TwitchIO shutdown errors
- Fixed frontend undefined value handling for
parallelMessageLimitsetting - Merged default settings with database settings for backward compatibility with new settings
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/components/settings/GeneralSettings.jsx | Fixed undefined check for parallelMessageLimit to prevent treating undefined as a custom value |
| backend/version.py | Version bump from 1.3.0 to 1.3.1 |
| backend/tests/test_clearchat.py | Added credential checking decorator to skip TwitchBot tests when credentials unavailable |
| backend/routers/system.py | Refactored to import parallel_message_queue from modules.queue_manager |
| backend/routers/config_backup.py | Refactored to import avatar_message_queue from modules.queue_manager |
| backend/routers/avatars.py | Refactored to import avatar_message_queue from modules.queue_manager across all endpoints, removed trailing whitespace |
| backend/modules/youtube_listener.py | Added RefreshError handling, improved error messages, removed exc_info from generic exception handlers |
| backend/modules/persistent_data.py | Added default settings merging logic for backward compatibility |
| backend/app.py | Increased cleanup delay from 0.1s to 0.5s for bot shutdowns, added custom exception handler to suppress TwitchIO shutdown noise |
Comments suppressed due to low confidence (1)
backend/tests/test_clearchat.py:7
- Import of 'os' is not used.
import os
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| return False | ||
| except Exception as e: | ||
| logger.error(f"Error finding active stream: {e}", exc_info=True) | ||
| logger.error(f"Error finding active stream: {e}") |
Copilot
AI
Nov 25, 2025
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.
[nitpick] The removal of exc_info=True from this error log means the full traceback won't be logged. While this may reduce log clutter for expected errors, it could make debugging unexpected exceptions more difficult. Consider keeping exc_info=True for the catch-all Exception handler to help diagnose unforeseen issues, while omitting it for the specific RefreshError and HttpError handlers where the error context is already clear.
| return None | ||
| except Exception as e: | ||
| logger.error(f"Error getting live chat ID: {e}", exc_info=True) | ||
| logger.error(f"Error getting live chat ID: {e}") |
Copilot
AI
Nov 25, 2025
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.
[nitpick] The removal of exc_info=True from this error log means the full traceback won't be logged. While this may reduce log clutter for expected errors, it could make debugging unexpected exceptions more difficult. Consider keeping exc_info=True for the catch-all Exception handler to help diagnose unforeseen issues, while omitting it for the specific RefreshError and HttpError handlers where the error context is already clear.
| logger.error(f"Error getting live chat ID: {e}") | |
| logger.error(f"Error getting live chat ID: {e}", exc_info=True) |
| """ | ||
| import pytest | ||
| import asyncio | ||
| import os |
Copilot
AI
Nov 25, 2025
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.
The os module is imported but doesn't appear to be used anywhere in this test file. Consider removing this unused import to keep the code clean.
| import os |
🐳 Docker Image Built SuccessfullyImage: Test this PR with Docker:docker pull ghcr.io/pladisdev/chat-yapper:pr-858a27b
docker run -d \
--name chat-yapper-pr \
-p 8069:8008 \
-e TWITCH_CLIENT_ID=your_id \
-e TWITCH_CLIENT_SECRET=your_secret \
ghcr.io/pladisdev/chat-yapper:pr-858a27bAccess at: http://localhost:8069 The Docker image will be published to the GitHub Container Registry when merged to |
No description provided.