fix: PlainFileHandler never rotates, regression of #2538 - #4824
Merged
Vasilije1990 merged 2 commits intoAug 31, 2026
Conversation
PlainFileHandler.emit() fully overrides RotatingFileHandler.emit() (needed for structlog-aware dict-message formatting) and never calls shouldRollover()/doRollover(). maxBytes/backupCount are accepted and stored by the inherited __init__, and the class docstring claims automatic rotation, but nothing ever triggers it -- confirmed the string "rollover" doesn't appear anywhere in the file before this fix. topoteretes#2538 was closed by switching this class's base from FileHandler to RotatingFileHandler and passing maxBytes/backupCount through, but the pre-existing emit() override was never updated to actually call the rotation check, so the original unbounded-growth symptom recurs (confirmed locally: one log file to 297MB with zero .1/.2 backups ever created, on an install with LOG_FILE_NAME pinned to a single path). Adds a direct stream-size check instead of calling self.shouldRollover(record) -- its stock implementation estimates the pending message's length via self.format(record), which assumes a plain-string message. Since this handler exists specifically for structlog dict-shaped records, running the stock formatter just to estimate a length risks raising on the very record the check is meant to protect. Tested in isolation: 200 real structlog dict-message calls against a throwaway file (maxBytes=2000, backupCount=2) -- rotation fires, stays bounded, zero exceptions, zero malformed lines. Fixes topoteretes#4823
Contributor
|
@BrightMine35 please resolve conflicts |
…local git merge-tree and a real test merge (both clean, zero conflicts) that this branch has no actual content conflict with upstream/main; the API's 'dirty' mergeable_state appears to be a stale cache rather than a real conflict.
Vasilije1990
approved these changes
Aug 31, 2026
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
2 participants
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.
Summary
PlainFileHandler.emit()fully overridesRotatingFileHandler.emit()(needed for its structlog dict-message formatting) and never callsshouldRollover()/doRollover().maxBytes/backupCountare accepted and stored, and the class docstring claims automatic rotation, but nothing ever triggers it.FileHandlertoRotatingFileHandler, but the pre-existingemit()override was never updated to actually call the rotation check, so the original unbounded-growth symptom recurs under a different shape (one huge file instead of many small ones).LOG_FILE_NAMEpinned to one path grew that file to 297MB with zero.1/.2backups ever appearing.self.shouldRollover(record)— its stock implementation estimates the pending message's length viaself.format(record), which assumes a plain-string message. Since this handler exists specifically for structlog dict-shaped records (record.msgis often adict), running the stock formatter just to estimate a length risks raising on the very record the check is meant to protect.Fixes #4823
Test plan
python -m py_compile cognee/shared/logging_utils.pylogger.info({"event": ..., "logger": ...})calls against a throwaway file withmaxBytes=2000, backupCount=2— rotation fires correctly,.1/.2backups created, current file stays bounded, zero exceptions raised by the new check, zero malformed/truncated lines in any retained file.