Replace print statements with logger.error in middleware.py#1033
Replace print statements with logger.error in middleware.py#1033Diegomed11 wants to merge 4 commits intoalphaonelabs:mainfrom
Conversation
👀 Peer Review RequiredHi @Diegomed11! This pull request does not yet have a peer review. Before this PR can be merged, please request a review from one of your peers:
Thank you for contributing! 🎉 |
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Exception handling / logging web/middleware.py |
Replaced print() calls that emitted exception header, type, message, and traceback tags with logger.error() using exc_info=True. Removed an early return None, allowing the original error handling flow to continue (render 500 / notify). |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly and concisely describes the main change: replacing print statements with logger.error calls in middleware.py, which directly matches the changeset. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
📝 Coding Plan
- Generate coding plan for human review comments
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 help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/middleware.py (1)
70-70: 🧹 Nitpick | 🔵 TrivialPre-existing: Unreachable code.
This
return Nonestatement is unreachable since both branches of theif settings.DEBUGblock (lines 59-68) already return. This isn't introduced by your changes, but it's worth noting for a future cleanup.🧹 Remove unreachable code
send_slack_message(error_message) return render(request, "500.html", status=500) - - return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/middleware.py` at line 70, Remove the unreachable "return None" after the settings.DEBUG conditional in the middleware function: the if/else branches that check settings.DEBUG (the block around settings.DEBUG lines 59–68) already return, so delete the trailing "return None" to clean up dead code and ensure the function's control flow is clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/middleware.py`:
- Around line 50-51: Replace the f-string logging calls with lazy `%s`-style
formatting on the existing logger.error calls: instead of evaluating f-strings,
call logger.error with a format string and pass the values as separate arguments
(use type(exception).__name__ for the type and exception itself for the message)
so the formatting is deferred when the log level is disabled.
- Line 53: Replace the direct call to traceback.print_exc() in the middleware
error handling with a logger-based call so the full traceback goes through the
logging system (e.g., use logger.error(..., exc_info=True) or
logger.error(traceback.format_exc())) — locate the traceback.print_exc()
invocation in web/middleware.py (inside the exception handling block where the
middleware handles errors) and change it to a single logger.error call that
includes the exception info/traceback, referencing the existing logger instance
used elsewhere in this file.
---
Outside diff comments:
In `@web/middleware.py`:
- Line 70: Remove the unreachable "return None" after the settings.DEBUG
conditional in the middleware function: the if/else branches that check
settings.DEBUG (the block around settings.DEBUG lines 59–68) already return, so
delete the trailing "return None" to clean up dead code and ensure the
function's control flow is clear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f8ee8718-e6b5-4b73-9495-f9c8de32cc36
📒 Files selected for processing (1)
web/middleware.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/middleware.py`:
- Around line 52-55: Replace the ambient exc_info=True call with an explicit
exception tuple: instead of logger.error("Traceback:", exc_info=True) in
process_exception, pass exc_info as a tuple containing the exception's class,
the exception object, and the exception's __traceback__ (i.e.,
exception.__class__, exception, exception.__traceback__) so logging uses the
provided exception deterministically; keep the existing traceback.format_exc()
usage and error_message construction unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 801d67b5-348b-46d5-8090-a2dedf9d827e
📒 Files selected for processing (1)
web/middleware.py
|
@A1L13N ready for review. Logging cleanup in middleware.py, all feedback resolved. |
Replaced all
print()calls inweb/middleware.pywithlogger.error()for exception logging. Exception details, type, message, and traceback output now go through Python's logging module instead of stdout. The middleware already hadimport loggingand a module-level logger configured, so the only change needed was replacing the print statements.Related issues
No existing issue — found print statements during code review.
Checklist
This PR replaces stdout-based exception prints with structured logging in GlobalExceptionMiddleware (web/middleware.py).
Key modifications
Impact