fix(proxy): defer file logging install to create_app()#304
Merged
chopratejas merged 1 commit intochopratejas:mainfrom Apr 29, 2026
Merged
Conversation
Move _setup_file_logging() from module import to create_app() so importing headroom.proxy.server in tests or library contexts no longer silently attaches a RotatingFileHandler to the user's live proxy.log. This was discovered via PR chopratejas#303: running the test suite produced "Optimization failed: TimeoutError" entries in ~/.headroom/logs/proxy.log because TestClient instantiations from test_proxy_anthropic_compression_diagnostics.py inherited the module-level handler. Add a regression test that imports the server module in a subprocess and asserts no RotatingFileHandler is attached to the headroom logger. Refs chopratejas#303 (review thread). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
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
Move
_setup_file_logging()from module-level import to insidecreate_app(), so importingheadroom.proxy.serverin tests or library contexts no longer silently attaches aRotatingFileHandlerto the user's live~/.headroom/logs/proxy.log.This is the follow-up agreed on in PR #303's review thread — the test-log-pollution side finding from the compression-timeout diagnostics work.
Why this matters
Previously, anything that did
from headroom.proxy.server import …(the test suite, scripts, library users) would inherit the rotating file handler at import time. That's how PR #303'stest_proxy_anthropic_compression_diagnostics.pyended up writingOptimization failed: TimeoutErrorentries into the user's realproxy.logand was briefly mistaken for a live #296 reproduction.Approach
_setup_file_logging()fromheadroom/proxy/server.py.create_app()instead — that's the only path where a real proxy server is being stood up.if not any(isinstance(h, RotatingFileHandler) …)) still prevents duplicate handlers whencreate_app()is called multiple times in a process.Regression test
Added
test_importing_server_does_not_install_file_handler(intests/test_pr208_changes.py::TestSetupFileLogging). It launches a subprocess, importsheadroom.proxy.server, and asserts noRotatingFileHandleris attached to theheadroomlogger. The subprocess is needed because once the parent test process imports the module the side effect would persist across tests.Test plan
pytest tests/test_pr208_changes.py::TestSetupFileLogging— 3 passedpytest tests/test_pr208_changes.py tests/test_proxy_anthropic_cache_stability.py tests/test_proxy_healthchecks.py tests/test_proxy_compress_endpoint.py— 82 passed, 1 skippedcreate_app(ProxyConfig())still installs the handler (verifiedbefore_create_app=False after_create_app=True)🤖 Generated with Claude Code