Skip to content

fix(logging): use ConcurrentRotatingFileHandler to fix Windows rollover PermissionError (#44873) - #44904

Closed
tuancookiez-hub wants to merge 1 commit into
NousResearch:mainfrom
tuancookiez-hub:fix/windows-log-rotation-permission-error
Closed

fix(logging): use ConcurrentRotatingFileHandler to fix Windows rollover PermissionError (#44873)#44904
tuancookiez-hub wants to merge 1 commit into
NousResearch:mainfrom
tuancookiez-hub:fix/windows-log-rotation-permission-error

Conversation

@tuancookiez-hub

Copy link
Copy Markdown
Contributor

Fixes #44873.

Summary

RotatingFileHandler.doRollover() raises PermissionError [WinError 32]
on Windows when background threads (gateway, agent loop, TTS, subagent
RPC) hold concurrent handles to agent.log, producing noisy tracebacks
on every log emit and pinning the file at the 5 MiB threshold.

The fix swaps stdlib RotatingFileHandler for
concurrent_log_handler.ConcurrentRotatingFileHandler, which uses
copy-then-truncate on Windows (works while the source is open) and
keeps the atomic rename on POSIX. Same constructor signature, drop-in
replacement, no API change.

As defense in depth, the existing doRollover override now wraps
super().doRollover() in try/except PermissionError so any future
transient failure is swallowed silently instead of spamming stderr.

Changes (4 files, ~134 lines)

  • hermes_logging.py — parent class swap, try/except wrap, idempotency check, docstring
  • pyproject.toml — exact-pinned concurrent-log-handler==0.9.29 with justification comment
  • uv.lock — lock entry for the new dep + portalocker
  • tests/test_hermes_logging.pyisinstance checks updated to use the new _ROTATING_BASE selector

Verification

Multi-thread Windows repro at maxBytes=256:

  • 8 threads × 80 emits (640 records)
  • 3 backup files created
  • 0 errors
  • 0 stderr tracebacks

Test suite: 60/62 pass. The 2 test_managed_mode_* failures are
pre-existing on Windows (Python's os.chmod is a no-op for Unix
permission bits) and out of scope per the project's one-fix-per-PR
convention.

Tradeoffs

  • One new direct dep. Apache-2.0, 110M+ lifetime downloads, used by
    Django and AWS CLI for the same reason. Could be gated as an
    extras_require (win-only) but every Windows install hits this once
    agent.log reaches 5 MiB, so base install seems right.
  • CLH creates .__filename.lock files next to each log (multi-process
    safety). Harmless; same pattern stdlib's SMTPHandler uses.

…ver()``

fails with ``PermissionError [WinError 32]`` whenever any other thread
holds an append-mode handle — which is always in Hermes. Result: stderr
gets a full traceback on every ``emit()`` and ``agent.log`` pins at the
5 MiB threshold.

Swap in ``concurrent_log_handler.ConcurrentRotatingFileHandler``
(copy-then-truncate on Windows, atomic rename on POSIX) and wrap
``doRollover`` in ``try/except PermissionError`` as defense in depth.
Tests updated to use the new module-level ``_ROTATING_BASE`` selector.

Closes NousResearch#44873
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]: RotatingFileHandler.doRollover() raises PermissionError [WinError 32] on Windows — uncaught, spams stderr on every emit

1 participant