Skip to content

Fixing log errors at the end of tests#7340

Merged
thabofletcher merged 2 commits intomainfrom
fixing-logging-errors
Feb 7, 2026
Merged

Fixing log errors at the end of tests#7340
thabofletcher merged 2 commits intomainfrom
fixing-logging-errors

Conversation

@galvana
Copy link
Contributor

@galvana galvana commented Feb 6, 2026

Description Of Changes

During test teardown (and interpreter shutdown), Python's asyncio emits a late DEBUG-level log message ("Using selector: EpollSelector") via the standard logging module. Because the InterceptHandler routes all standard library logs through Loguru, this message reaches Loguru's stdout sink after pytest has already closed sys.stdout. Loguru's catch=True catches the resulting ValueError but prints a noisy diagnostic block at the end of every test run:

--- Logging error in Loguru Handler #5 ---
Record was: {'elapsed': ..., 'message': 'Using selector: EpollSelector', ...}
Traceback (most recent call last):
File ".../loguru/handler.py", line 206, in emit
self.sink.write(str_record)
File ".../loguru/simple_sinks.py", line 16, in write
self.stream.write(message)
ValueError: I/O operation on closed file.
--- End of logging error ---

This is harmless (no logs are lost, no tests are affected) but it's confusing and noisy.

The fix replaces the raw sys.stdout file object with a callable sink function (_safe_stdout_sink) that wraps sys.stdout.write() in a try/except ValueError. This is idiomatic for Loguru — callable sinks are a first-class feature and the codebase already uses one for the RedisSink. Callable sinks bypass Loguru's SimpleSink wrapper entirely, so the error is caught by our function before Loguru's error reporting ever sees it.

Code Changes

  • Added _safe_stdout_sink callable sink function that writes to sys.stdout and silently drops ValueError from closed file descriptors
  • Updated setup() to use _safe_stdout_sink as the default sink instead of sys.stdout directly

Steps to Confirm

  1. Run the test suite (nox -s pytest or pytest --no-cov inside the container)
  2. Verify the --- Logging error in Loguru Handler #5 --- diagnostic no longer appears at the end of the test output
  3. Verify normal log output still appears correctly during tests (formatting, colors, levels)

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@vercel
Copy link
Contributor

vercel bot commented Feb 6, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Feb 6, 2026 10:41pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored Feb 6, 2026 10:41pm

Request Review

@galvana galvana marked this pull request as ready for review February 6, 2026 22:35
@galvana galvana requested a review from a team as a code owner February 6, 2026 22:36
@galvana galvana requested review from thabofletcher and removed request for a team February 6, 2026 22:36
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 6, 2026

Greptile Overview

Greptile Summary

This PR fixes noisy logging errors that appeared at the end of test runs when asyncio emitted late log messages after pytest closed sys.stdout. The solution replaces the raw sys.stdout file object with a callable sink function (_safe_stdout_sink) that wraps sys.stdout.write() in a try/except to silently catch ValueError exceptions from closed file descriptors.

  • Added _safe_stdout_sink function that safely writes to stdout and catches ValueError when the stream is closed
  • Updated setup() to use _safe_stdout_sink instead of sys.stdout directly for the default sink
  • This is idiomatic for Loguru - callable sinks are first-class and the codebase already uses one for RedisSink

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The change is well-contained, addresses a specific issue with clear cause and effect, follows Loguru's idiomatic patterns (callable sinks), and only modifies the logging setup without changing any business logic. The implementation is simple and defensive - catching only the specific ValueError that occurs when writing to closed stdout.
  • No files require special attention

Important Files Changed

Filename Overview
src/fides/api/util/logger.py Added _safe_stdout_sink callable to gracefully handle closed stdout during test teardown, preventing noisy error diagnostics

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@thabofletcher thabofletcher left a comment

Choose a reason for hiding this comment

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

🚀 🚀 🚀

@thabofletcher thabofletcher added this pull request to the merge queue Feb 7, 2026
Merged via the queue into main with commit 1cc5e03 Feb 7, 2026
54 of 55 checks passed
@thabofletcher thabofletcher deleted the fixing-logging-errors branch February 7, 2026 00:53
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.

2 participants