Skip to content

Conversation

Shubham-Jain52
Copy link

fix(logging): ensure verbose mode intercepts all standard logs

Closes #108

Description

This pull request resolves an issue where the LoggingManager's verbose mode did not reliably capture logs from Python's standard logging module.

The previous implementation was unreliable. This fix replaces it with a robust InterceptStandardLoggingHandler that redirects all standard logs to Loguru.

To ensure the intercepted message is displayed correctly, this fix also:

  1. Updates the handler to create a complete message string: [LEVEL] [logger_name] message.
  2. Updates the verbose setup_logging format to {message} to prevent Loguru from discarding this custom string.

A new unit test is included to verify that a standard log message is now fully captured and formatted as expected.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • All new and existing tests pass locally with my changes

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

@Shubham-Jain52 Shubham-Jain52 marked this pull request as ready for review October 19, 2025 05:33
@harshalmore31
Copy link
Collaborator

@Shubham-Jain52 Thanks for the contribution, Great refactor! Much cleaner approach. 👏

Minor Issues

1. Remove accidental .gitignore change

The me/ directory looks unrelated.

2. Format change breaks ALL logs (BLOCKING)

format="{message}"  # Removes timestamps/file info from EVERYTHING

Fix:

format="<green>{time:HH:mm:ss}</green> | <level>{level:8}</level> | {message}"

3. Remove duplicate level

# Current: f"[{record.levelname}] [{record.name}] {record.getMessage()}"
# Shows: "WARNING | [WARNING] [urllib3] ..." ← duplicate

# Fix: f"[{record.name}] {record.getMessage()}"
# Shows: "WARNING | [urllib3] ..." ✅

4. Expand test coverage

  • Test Memori's own logger.info() still works
  • Test different log levels (debug, error)
  • Verify timestamps appear in output
  • Test exception logging

The core approach is solid! Just needs these fixes before merge. 🚀

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 Bug: LoggingManager Verbose Mode Does Not Always Disable Other Loggers (Hacktoberfest)

2 participants