fix(sports): say when the schema cannot be read, instead of failing silently - #520
Merged
Conversation
…ilently _schema_font_size swallowed every exception and cached an empty dict. That is not cosmetic. With no schema, a configured font size can no longer be compared against the schema default, so every size is treated as a deliberate user choice and skips the snap to the font's pixel grid -- which renders 4x6-font.ttf at 6 instead of 7: a 3px-wide glyph instead of 4px. That shipped. On a 256x64 panel it made the odds, the team records and the date row hard to read, and it was found by a user counting pixels on a photo of the panel rather than by anything here. The cause (_plugin_dir returning None under the real plugin loader) is fixed in #519; this makes the same class of failure audible next time: Orphan: could not read config_schema.json (FileNotFoundError: ...); every font size will be treated as user-chosen and will skip its pixel grid snap. Font sizes may render a pixel narrow. The message names the consequence, not just the error, because the error alone does not suggest "your fonts are a pixel narrow". Logged rather than raised: an unreadable schema must not stop a plugin rendering. The cache is built once per class (per schema path in sports_card), so this cannot repeat per frame. Scope deliberately small. An audit of the three shared modules found 23 handlers that swallow and return a default, but all 23 catch specific types -- TypeError, ValueError, ImportError -- turning bad config values into defaults, which is what they are for. Of 77 broad handlers across the font and odds paths, 74 already log. Only these two were both broad and silent.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
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.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds warning logs for schema read failures in both font-size handling paths. The logs include exception details and state that font sizes will skip pixel-grid snapping. ChangesSchema Logging
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to Schema-read failures now produce warnings while preserving rendering fallback behavior. No current merge-blocking risk remains. 🚥 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 |
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
1 participant
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.
_schema_font_sizeswallowed every exception and cached an empty dict.That isn't cosmetic. With no schema, a configured font size can't be compared against the schema default — so every size is treated as a deliberate user choice and skips the snap to the font's pixel grid.
4x6-font.ttfthen renders at 6 instead of 7: a 3px-wide glyph instead of 4px.That shipped. On a 256×64 panel it made the odds, team records and date row hard to read, and it was found by a user counting pixels on a photo of the panel — not by anything here. The cause (
_plugin_dirreturningNoneunder the real plugin loader) is fixed in #519. This makes the same class of failure audible next time:The message names the consequence, not just the error —
FileNotFoundErroralone doesn't suggest "your fonts are a pixel narrow".Logged rather than raised: an unreadable schema must not stop a plugin rendering. The cache is built once per class (per schema path in
sports_card), so this can't repeat per frame. Verified the warning actually fires in both modules.Scope is deliberately small
I went in expecting to fix ~23 handlers and found that was the wrong target:
TypeError,ValueError,ImportError), turning bad config values into defaults, which is what they're forChanging the typed handlers would have added noise and risk for no benefit.
Core suite: 3881 passed, 0 failed.
Summary by CodeRabbit