Fix #708 – Clear exc_info and stack_info before formatting in InterceptHandler#719
Open
Pranav1921 wants to merge 2 commits into
Open
Fix #708 – Clear exc_info and stack_info before formatting in InterceptHandler#719Pranav1921 wants to merge 2 commits into
Pranav1921 wants to merge 2 commits into
Conversation
Contributor
WalkthroughColorFormatter.format now preserves original Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-10-13T16:40:22.622ZApplied to files:
📚 Learning: 2025-10-15T07:13:34.502ZApplied to files:
🧬 Code graph analysis (1)backend/app/logging/setup_logging.py (1)
🔇 Additional comments (2)
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 |
![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: 1
🧹 Nitpick comments (1)
backend/app/logging/setup_logging.py (1)
63-68: Avoid mutatingLogRecordin-place; save/restoreexc_infoandstack_info.
Clearing these fields on the sharedrecordcan leak into other handlers/formatters if the record is processed elsewhere. Prefer temporarily clearing during formatting and restoring afterwards.def format(self, record: logging.LogRecord) -> str: """Format the log record with colors and component prefix.""" - # Clear exc_info and stack_info before formatting - record.exc_info = None - record.stack_info = None + # Clear exc_info and stack_info before formatting (but don't mutate permanently) + _exc_info, _stack_info = record.exc_info, record.stack_info + record.exc_info = None + record.stack_info = None + try: + # Add component information to the record + component_prefix = self.component_config.get("prefix", "") + record.component = component_prefix + + # Format the message + formatted_message = super().format(record) + finally: + record.exc_info, record.stack_info = _exc_info, _stack_info - - # Add component information to the record - component_prefix = self.component_config.get("prefix", "") - record.component = component_prefix - - # Format the message - formatted_message = super().format(record)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/logging/setup_logging.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Hemil36
Repo: AOSSIE-Org/PictoPy PR: 548
File: backend/app/logging/setup_logging.py:224-243
Timestamp: 2025-10-13T16:40:22.622Z
Learning: In the PictoPy project's centralized logging system (backend/app/logging/setup_logging.py and sync-microservice/app/logging/setup_logging.py), stack traces should not be shown in logs. When rerouting logs through InterceptHandler, exc_info and stack_info should not be preserved.
Learnt from: Hemil36
Repo: AOSSIE-Org/PictoPy PR: 548
File: backend/app/database/images.py:115-115
Timestamp: 2025-10-15T07:13:34.502Z
Learning: In the PictoPy backend and sync-microservice, exception logging using `logger.error(f"...")` without stack traces (i.e., without `exc_info=True` or `logger.exception()`) is acceptable for console logging, as this is a deliberate design decision made with the maintainer for console-only logs.
📚 Learning: 2025-10-13T16:40:22.622Z
Learnt from: Hemil36
Repo: AOSSIE-Org/PictoPy PR: 548
File: backend/app/logging/setup_logging.py:224-243
Timestamp: 2025-10-13T16:40:22.622Z
Learning: In the PictoPy project's centralized logging system (backend/app/logging/setup_logging.py and sync-microservice/app/logging/setup_logging.py), stack traces should not be shown in logs. When rerouting logs through InterceptHandler, exc_info and stack_info should not be preserved.
Applied to files:
backend/app/logging/setup_logging.py
…restore pattern - Add exc_info and stack_info clearing to sync-microservice ColorFormatter - Use save/restore pattern in both backend and sync-microservice to avoid affecting other handlers - Ensures consistent behavior across both components - Addresses CodeRabbit review comment about incomplete fix
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.
This PR fixes a logging issue where exc_info and stack_info were preserved before formatting, causing unwanted tracebacks in logs. These fields are now cleared before rerouting logs.
Problem (Before Fix):
[APP] | ERROR | Division by zero occurred! Traceback (most recent call last): File "test_logging.py", line 8, in <module> 1 / 0 ZeroDivisionError: division by zeroChanges Made:
After Fix:
[APP] | ERROR | Division by zero occurred!Testing:
Closes: #708
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.