Keep full-logging alive in granian workers after post-fork dictConfig - #6992
Conversation
With REFLEX_ENABLE_FULL_LOGGING, granian's worker startup runs logging.config.dictConfig, whose _clearExistingHandlers closes every fork-inherited handler. The full-logging FileHandler was opened with mode="w", which the stdlib refuses to reopen once closed (it would truncate), so every worker-side record was silently dropped from the log file. Worse, log_file_stream() then returned the closed handler's None stream, and the legacy console file writer's rich Console(file=None) fell back to stdout, leaking timestamped plain-text lines that broke the --json output contract. Fix: truncate the log file explicitly at handler creation and open the handler in append mode, so the stdlib reopens it on the next record (appends are also atomic, letting the CLI parent and reopened worker copies safely share one file). log_file_stream() now reopens a closed stream under the handler lock, and the legacy console writes through a stable proxy that resolves the live stream on every write, so it can neither leak to stdout nor crash on a stale closed stream. Found as FINDING-005 in 0.9.9a1 pre-release testing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x
Greptile SummaryThe PR makes full-file logging recover after Granian closes inherited handlers during worker logging reconfiguration.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains in the eligible follow-up review scope. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/reflex-base/src/reflex_base/utils/log.py | Changes file-handler initialization to truncate then append and adds a lock-protected, lazily reopening stream proxy. |
| packages/reflex-base/src/reflex_base/utils/console.py | Routes the cached Rich file console through the stable logging-stream proxy. |
| tests/units/reflex_base/utils/test_log.py | Adds regression tests for external handler closure, cached consoles, stdout isolation, and per-run truncation. |
| packages/reflex-base/news/+full-logging-worker.bugfix.md | Documents restored worker logging and prevention of plain-text leakage into JSON output. |
Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The changelog gate checks each package's own news/ directory, and the changed source lives in packages/reflex-base/, so the fragment must also exist there (keeping the root copy, as sibling PRs do). Condense both fragments to two sentences to match the style of neighboring fragments, and note at the log_file_stream() reopen site that FileHandler._open is private stdlib API relied on deliberately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x
Type of change
Changes To Core Features:
Defect
FINDING-005 from 0.9.9a1 pre-release testing (logging_cli cluster, verifier-confirmed): with
REFLEX_ENABLE_FULL_LOGGING, granian's worker startup (granian/server/mp.py::WorkerProcess.wrap_target) runsgranian.log.configure_logging->logging.config.dictConfigbefore loading the app. StdlibdictConfigunconditionally runs_clearExistingHandlers(), which closes every handler in the process — including the fork-inherited reflex full-loggingFileHandler. Since the handler was opened withmode="w", Python 3.10+FileHandler.emitrefuses to reopen it once closed (it would truncate), so every worker-side record was silently dropped from the log file — precisely the records full logging exists to capture.Worse,
log_file_stream()then returned the closed handler'sNonestream (despite its comment claiming that could never happen), and the legacyconsole.print_to_log_filewriter'srich.Console(file=None)falls back to stdout: every legacy console call in a worker leaked a duplicated[YYYY-mm-dd HH:MM:SS.ffffff] ...plain-text line, breaking the machine-readable--jsonoutput contract (11 non-JSON lines interleaved with JSON records in the repro). Regression introduced by #6863, which routed the legacy file console through the logging FileHandler's stream (0.9.8's file console opened its own append-mode file thatdictConfigcould not touch).Fix
All in
reflex_base/utils/log.py(+ one line inconsole.py), no post-fork hook needed — the pipeline now self-heals lazily, which also protects against any application-sidedictConfig:_file_handler()truncates the log file explicitly at creation and opens the handler in append mode. The stdlib then reopens a closedmode="a"handler on the nextemit, so fork-inherited worker handlers resume writing instead of dropping records; appends are atomic (O_APPEND), letting the CLI parent and the reopened worker copies safely share one file. Per-run truncation semantics are unchanged.log_file_stream()reopens a closed stream under the handler lock instead of handing writers a dead one.log_file_proxy()) that resolves the live stream on every write — it can neither fall back to stdout (the--jsonleak) nor crash withValueErroron a stale closed stream captured before the fork.Test plan
tests/units/reflex_base/utils/test_log.py, written first and shown failing on unfixed main with the exact three failure modes:test_file_handler_survives_external_dictconfig(post-dictConfig record missing from the file),test_log_file_console_targets_file_after_external_close(timestamped line leaked to captured stdout),test_cached_log_file_console_survives_external_close(ValueError: I/O operation on closed file), plustest_file_handler_truncates_previous_runguarding the truncate-then-append semantics. All 52 tests in the module pass with the fix.api_transformerroute emittingconsole.info/console.warn/reflex_base.*logger records,REFLEX_ENABLE_FULL_LOGGING=1 REFLEX_LOG_FILE=... reflex run --backend-onlyunder granian with the fork start method (the default on Python <= 3.13; forced viamultiprocessing.set_start_method("fork")on this 3.14 dev env, where granian otherwise coerces forkserver to spawn and the bug cannot trigger). Before: 0 worker records in the log file, 6 leaked timestamped stdout lines, and with--json11 non-JSON lines interleaved. After: all worker records (legacy shim + pipeline) land in the same file as the parent's, 0 leaked lines, and--jsonstdout is 14/14 valid JSON lines.uv run ruff check ./uv run ruff format .clean;uv run pyright reflex testsand pyright on the touched reflex-base files: 0 errors.🤖 Generated with Claude Code
https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x
Generated by Claude Code