-
Notifications
You must be signed in to change notification settings - Fork 685
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
Capture warnings and report to sentry #5697
Conversation
📝 WalkthroughWalkthroughThis change introduces a new function, Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant WH as warning_handler
participant Logger as Logging Module
participant Sentry as sentry_sdk
App->>WH: Issue warning (message, category, filename, lineno, file, line)
alt Message is Exception
WH->>Sentry: capture_exception(message)
end
WH->>Logger: Log warning message
📜 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 (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
supervisor/api/supervisor.py (1)
50-50
: Remove unused import.The
capture_exception
function is imported but not used directly in this file. According to static analysis, this should be removed to avoid confusion and maintain clean code.-from ..utils.sentry import capture_exception, close_sentry, init_sentry +from ..utils.sentry import close_sentry, init_sentry🧰 Tools
🪛 Ruff (0.8.2)
50-50:
..utils.sentry.capture_exception
imported but unusedRemove unused import:
..utils.sentry.capture_exception
(F401)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
supervisor/api/supervisor.py
(1 hunks)supervisor/bootstrap.py
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
`*/**(html|markdown|md)`: - For instructional content in doc...
*/**(html|markdown|md)
: - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
supervisor/bootstrap.py
`*/**(html|markdown|md)`: - Use bold to mark UI strings. - I...
*/**(html|markdown|md)
: - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
supervisor/bootstrap.py
`*/**(html|markdown|md)`: - Be brief in your replies and don...
*/**(html|markdown|md)
: - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
supervisor/bootstrap.py
`*/**(html|markdown|md)`: - Use sentence-style capitalizatio...
*/**(html|markdown|md)
: - Use sentence-style capitalization also in headings.
supervisor/bootstrap.py
`*/**(html|markdown|md)`: do not comment on HTML used for ic...
*/**(html|markdown|md)
: do not comment on HTML used for icons
supervisor/bootstrap.py
`*/**(html|markdown|md)`: Avoid flagging inline HTML for emb...
*/**(html|markdown|md)
: Avoid flagging inline HTML for embedding videos in future reviews for this repository.
supervisor/bootstrap.py
🪛 Ruff (0.8.2)
supervisor/bootstrap.py
227-227: First line of docstring should be in imperative mood: "Custom warning handler that logs warnings using the logging module."
(D401)
228-228: Logging statement uses f-string
(G004)
231-231: Prefer TypeError
exception for invalid type
(TRY004)
supervisor/api/supervisor.py
50-50: ..utils.sentry.capture_exception
imported but unused
Remove unused import: ..utils.sentry.capture_exception
(F401)
🪛 GitHub Check: Check ruff
supervisor/bootstrap.py
[failure] 227-227: Ruff (D401)
supervisor/bootstrap.py:227:5: D401 First line of docstring should be in imperative mood: "Custom warning handler that logs warnings using the logging module."
[failure] 228-228: Ruff (G004)
supervisor/bootstrap.py:228:21: G004 Logging statement uses f-string
[failure] 231-231: Ruff (TRY004)
supervisor/bootstrap.py:231:9: TRY004 Prefer TypeError
exception for invalid type
🪛 GitHub Check: Check pylint
supervisor/bootstrap.py
[warning] 231-231:
W0719: Raising too general exception: Exception (broad-exception-raised)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build armv7 supervisor
- GitHub Check: Build armhf supervisor
- GitHub Check: Build aarch64 supervisor
- GitHub Check: Run tests Python 3.13.2
🔇 Additional comments (1)
supervisor/bootstrap.py (1)
260-260
: LGTM! Overriding the default warning handler.Overriding the default warning handler with a custom implementation is a good approach to capture warnings and report them to Sentry.
f4a6bf0
to
887490c
Compare
By default, warnings are simply printed to stderr. This makes them easy to miss in the log. Capture warnings and user Python logger to log them with warning level. Also, if the message is an instance of Exception (which it typically is), report the warning to Sentry. This is e.g. useful for asyncio RuntimeWarning warnings "coroutine was never awaited".
887490c
to
b91f7d9
Compare
Proposed change
By default, warnings are simply printed to stderr. This makes them easy to miss in the log. Capture warnings and user Python logger to log them with warning level.
Also, if the message is an instance of Exception (which it typically is), report the warning to Sentry. This is e.g. useful for asyncio RuntimeWarning warnings "coroutine was never awaited".
Type of change
Additional information
Checklist
ruff format supervisor tests
)If API endpoints or add-on configuration are added/changed:
Summary by CodeRabbit