Skip to content

fix(logging): catch PermissionError in doRollover (#44873) - #45062

Closed
kyssta-exe wants to merge 1 commit into
NousResearch:mainfrom
kyssta-exe:auto-fix/issue-44873
Closed

kyssta-exe wants to merge 1 commit into
NousResearch:mainfrom
kyssta-exe:auto-fix/issue-44873

Conversation

@kyssta-exe

Copy link
Copy Markdown
Contributor

Fixes #44873. On Windows, RotatingFileHandler.doRollover() raises PermissionError [WinError 32] when the log file is locked by another process. The exception was uncaught, causing the stdlib logging error handler to print a full traceback to stderr on every subsequent emit. The fix wraps super().doRollover() in a try/except for PermissionError and OSError, closing and reopening the stream on failure so logging continues writing to the current file.

@liuhao1024

Copy link
Copy Markdown
Contributor

Verification: Clean implementation ✅

Reviewed the PermissionError handling in _ManagedRotatingFileHandler.doRollover():

  • Correct exception hierarchy: catches both PermissionError (Windows WinError 32) and OSError (generic file-lock scenarios).
  • Stream recovery: closes the old stream, sets to None, reopens via _open(), and refreshes the stat snapshot — the handler continues writing to the same file instead of crashing.
  • Silent failure is correct here: the inner try/except Exception: pass on stream reopen is appropriate — if reopening also fails, the handler degrades gracefully (next emit will try again).
  • Test coverage: two tests covering PermissionError and OSError paths, both verifying the handler continues to write after recovery.

Clean fix, no findings.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jun 12, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

Duplicate of #44974 — same catch-and-skip mechanism for #44873 (wrap RotatingFileHandler.doRollover() to swallow Windows PermissionError/WinError 32). #44974 is the earliest open PR using this approach. Competing alternative #44921 uses ConcurrentRotatingFileHandler (different mechanism — related).

@kyssta-exe

Copy link
Copy Markdown
Contributor Author

Duplicate of #44974 — same PermissionError catch in RotatingFileHandler.doRollover (#44873). Closing as superseded.

@kyssta-exe kyssta-exe closed this Jun 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists type/bug Something isn't working

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

3 participants