fix(music): stop an unreachable YTM Companion flooding the log - #266
Merged
Conversation
Found on a live device: 13,298 ERROR lines in 12 hours from this plugin alone -- 99.8% of everything the device logged. Finding the two real problems underneath it meant filtering this out first, which is exactly how a regression goes unnoticed. The cause was three faults stacked. The poll loop asked for a reconnect every cycle (~2s) and the client always tried, with no backoff, so an unreachable companion was retried forever at full rate. Each attempt logged two ERROR records, though "YouTube Music is not open" is an ordinary state rather than a fault. And they went out through bare logging.error(), landing on the root logger -- which is why they read as "root" in the journal, outside the plugin's logger and so beyond any log level a user could lower. Reconnects now back off from 5s to a 5-minute cap. The first failure gets a single WARNING naming the endpoint and what to check; repeats are debug. The client takes the plugin's logger. Over the same 12 hours: one warning instead of 13,298 errors, and 149 attempts instead of 21,600. A companion that comes back is still picked up within five minutes, and recovery is logged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Contributor
|
Warning Review limit reached
Next review available in: 52 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe Music Player plugin adds YTM Companion reconnect backoff, client-scoped logging, manager integration, regression coverage, and version 1.3.0 release metadata. ChangesYTM reconnect behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Manager
participant YTMClient
participant SocketIO
participant Logger
Manager->>YTMClient: Poll for YTM Companion connection
YTMClient->>YTMClient: Check scheduled retry time
YTMClient->>SocketIO: Attempt connection
SocketIO-->>YTMClient: Return failure or success
YTMClient->>Logger: Write scoped reconnect status
YTMClient-->>Manager: Continue polling or synchronize after recovery
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 37 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/ledmatrix-music/manifest.json`:
- Line 4: Use PATCH version 1.2.1 for the reconnect and logging fixes: update
the top-level version and first release record in
plugins/ledmatrix-music/manifest.json. Regenerate plugins.json with the
repository’s update_registry.py/pre-commit hook so latest_version becomes 1.2.1;
do not hand-edit the generated file.
- Line 72: Update the manifest release notes to distinguish the observed 13,298
log-line measurement from the theoretical 43,200 records produced by 21,600
attempts at two records each, and change the plugin version from 1.3.0 to 1.2.1.
In `@plugins/ledmatrix-music/ytm_client.py`:
- Around line 196-200: Update YTMClient.connect_client so the missing-token
warning is emitted only once per client instance, logging subsequent attempts at
debug level instead; add and maintain a per-instance warning state, and reset it
in load_config when a token is successfully loaded.
- Around line 103-113: Update the socket event handling around the diagnostic
extraction before the external_update_callback submission: initialize title and
author before the try block, validate the nested video value before calling
.get(), and keep malformed-payload logging at debug level. Ensure extraction
failures leave safe diagnostic defaults and never prevent
self.external_update_callback from being submitted with data.
- Around line 206-215: Disable Socket.IO’s automatic reconnection in the client
initialization by setting its reconnection option to false, leaving
connect_client() and _next_retry_at responsible for all retry scheduling. Add a
regression test covering a connection that succeeds and then disconnects,
verifying no independent reconnect loop starts.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a33a018a-4907-47f8-b0bc-5871773eaa4d
📒 Files selected for processing (5)
plugins.jsonplugins/ledmatrix-music/manager.pyplugins/ledmatrix-music/manifest.jsonplugins/ledmatrix-music/test_ytm_reconnect_backoff.pyplugins/ledmatrix-music/ytm_client.py
Two imports left over from an earlier draft, and a literal assigned to `ytm_token`, which reads as a hardcoded credential to a scanner. It is neither a credential nor real, so name it rather than suppress the warning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
ChuckBuilds
force-pushed
the
fix/ytm-reconnect-backoff
branch
from
August 11, 2026 12:30
4fd962c to
291a0a6
Compare
…g a track Five review findings. The missing-token branch sits ahead of the backoff gate, so it warned on every poll cycle -- the same flood this change removes, wearing "not authenticated" instead of "not reachable". It now warns once per client, debug thereafter, and rearms if a token appears and later goes away. socketio.Client was constructed with reconnection=True and infinite attempts, so after an established connection dropped it ran its own 1-10s retry loop that knew nothing about the backoff. Disabled; connect_client() owns retry timing, and the poll loop already drives it. An event carrying "video": null made the nested .get() raise inside the diagnostic logging. The bare except swallowed it, `title` was left unbound, and the line below raised UnboundLocalError -- so the update callback was never submitted and the track change was silently lost. Diagnostics must not gate delivery: the fields are assigned before the try and nested values validated. Pre-existing, but in the path this change is about. Version corrected to a PATCH, 1.2.1: these are fixes, with no new feature, option or display mode. The release note no longer mixes a measurement with a model -- 13,298 error lines is what the device logged; 21,600 was poll cycles, not attempts, and implied a figure that was never observed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
update_registry.py only moves versions forward, so it skipped the 1.3.0 -> 1.2.1 correction and left the registry disagreeing with the manifest. Reset the file to main's state and regenerated, which takes it 1.2.0 -> 1.2.1 as a normal bump rather than hand-editing generated output. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
A string literal assigned to something ending in _token reads as a hardcoded credential to a scanner. The constant already exists for exactly this; use it rather than adding a second literal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found while sweeping a live device for errors. This plugin alone produced 13,298 ERROR lines in 12 hours — 99.8% of everything the device logged. Finding the two real problems underneath meant filtering it out first, which is exactly how a regression goes unnoticed.
Roughly 24 lines a minute, ~34,000 a day, for a companion app that simply wasn't running.
Three faults stacked
No backoff. The poll loop asks for a reconnect every cycle (~2s) and the client always tried. An unreachable server was retried forever at full rate. (
socketio.Clientwas configured with its own backoff, but the explicitconnect_client()call each cycle bypasses it entirely.)Wrong severity. "YouTube Music isn't open" is an ordinary state, not a fault — and it emitted two ERROR records per attempt, one from
connect_errorand one from the exception handler.Root logger.
ytm_client.pycalled barelogging.error(...), so records landed on the root logger. That's why they read as- root -in the journal rather thanplugin.ledmatrix-music. The practical cost is that they sit outside the plugin's logger, so a user cannot lower the level to quiet them —CLAUDE.mdcalls this out ("BasePlugin usesget_logger()… not standardlogging.getLogger()").The fix
Reconnects back off 5s → 5-minute cap. The first failure is a single WARNING naming the endpoint and what to check; repeats are debug. The client takes the plugin's logger.
Measured over the same 12-hour window, at the same 2s poll interval:
Reconnection behaviour is otherwise unchanged: a companion that comes back is picked up within five minutes, the backoff resets, and recovery is logged (
YTM Companion reachable again after N failed attempt(s)).Verification
test_ytm_reconnect_backoff.py: 14 checks against a fake clock — attempts stay far below poll cycles, zero ERROR records, exactly one WARNING that names the endpoint and mentions the companion, delays increase and stay capped, the cap is finite so it always retries eventually, success clears the backoff, and an AST-ish source check that no barelogging.<level>(calls remain.🤖 Generated with Claude Code
https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Summary by CodeRabbit
New Features
Bug Fixes
Tests