Skip to content

Replace print statements with logger.error in middleware.py#1033

Open
Diegomed11 wants to merge 4 commits intoalphaonelabs:mainfrom
Diegomed11:fix/replace-print-with-logging-middleware
Open

Replace print statements with logger.error in middleware.py#1033
Diegomed11 wants to merge 4 commits intoalphaonelabs:mainfrom
Diegomed11:fix/replace-print-with-logging-middleware

Conversation

@Diegomed11
Copy link

@Diegomed11 Diegomed11 commented Mar 18, 2026

Replaced all print() calls in web/middleware.py with logger.error() for exception logging. Exception details, type, message, and traceback output now go through Python's logging module instead of stdout. The middleware already had import logging and 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

  • Did you run the pre-commit?
  • Did you test the change?
  • Added screenshots to the PR description (if applicable)

This PR replaces stdout-based exception prints with structured logging in GlobalExceptionMiddleware (web/middleware.py).

Key modifications

  • Replaced print() calls with logger.error() in process_exception to log a header, exception type, message, and traceback.
  • Uses lazy formatting for logger calls and passes exc_info=True so the traceback is captured by the logger.
  • Keeps traceback.format_exc() to assemble an error message sent to Slack or included in the 500 debug context.
  • Removed an unreachable return None introduced earlier, restoring the intended flow that either renders the 500 page (with context in DEBUG) or sends a Slack notification.

Impact

  • Centralizes exception output through the module-level logger (consistent with existing logging configuration), enabling handlers/formatters to capture tracebacks and control output destinations.
  • No change to error-handling behavior or user-facing responses; behavior in DEBUG vs production remains the same.

@github-actions
Copy link
Contributor

👀 Peer Review Required

Hi @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:

  • Go to the PR page and click "Reviewers" on the right sidebar.
  • Select a team member or contributor to review your changes.
  • Once they approve, this reminder will be automatically removed.

Thank you for contributing! 🎉

@github-actions github-actions bot added the files-changed: 1 PR changes 1 file label Mar 18, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 18, 2026

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key(s) in object: 'tools'
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Walkthrough

Replaces direct stdout prints in GlobalExceptionMiddleware.process_exception with structured logging via logger.error(..., exc_info=True), and removes an early return None so the middleware proceeds with the existing downstream flow (e.g., rendering the 500 response or sending Slack notifications).

Changes

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 ⚠️ Warning 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 | 🔵 Trivial

Pre-existing: Unreachable code.

This return None statement is unreachable since both branches of the if settings.DEBUG block (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

📥 Commits

Reviewing files that changed from the base of the PR and between c94caf8 and adeec83.

📒 Files selected for processing (1)
  • web/middleware.py

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🤖 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

📥 Commits

Reviewing files that changed from the base of the PR and between adeec83 and 99ac14f.

📒 Files selected for processing (1)
  • web/middleware.py

@Diegomed11
Copy link
Author

@A1L13N ready for review. Logging cleanup in middleware.py, all feedback resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

files-changed: 1 PR changes 1 file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant