Skip to content

Commit d3238af

Browse files
cfsmp3claude
authored andcommitted
fix: handle log file permission errors gracefully
When the log file is owned by a different user (e.g., root vs www-data), the app would crash on startup with PermissionError. This was causing workers to fail immediately. Changes: - Catch PermissionError and OSError when creating the file handler - Fall back to console-only logging instead of crashing - Print helpful warning message with fix instructions to stderr - Update file_logger property to return Optional[RotatingFileHandler] - Update create_logger to skip file handler when unavailable This prevents the entire application from crashing due to a log file permission issue, allowing it to continue operating with console logging while still alerting operators to fix the underlying permission problem. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent abb8ebe commit d3238af

2 files changed

Lines changed: 142 additions & 56 deletions

File tree

log_configuration.py

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@
33
import logging
44
import logging.handlers
55
import os
6+
import sys
67
from logging import Logger, StreamHandler
78
from logging.handlers import RotatingFileHandler
8-
from typing import Union
9+
from typing import Optional, Union
910

1011

1112
class LogConfiguration:
@@ -19,21 +20,47 @@ def __init__(self, folder: str, filename: str, debug: bool = False) -> None:
1920
self._consoleLogger.setLevel(logging.DEBUG)
2021
else:
2122
self._consoleLogger.setLevel(logging.INFO)
22-
# create a file handler
23-
path = os.path.join(folder, 'logs', f'{filename}.log')
24-
self._fileLogger = logging.handlers.RotatingFileHandler(path, maxBytes=1024 * 1024, backupCount=20)
25-
self._fileLogger.setLevel(logging.DEBUG)
26-
# create a logging format
27-
formatter = logging.Formatter('[%(name)s][%(levelname)s][%(asctime)s] %(message)s')
28-
self._fileLogger.setFormatter(formatter)
23+
24+
# create a file handler with permission error handling
25+
self._fileLogger: Optional[RotatingFileHandler] = None
26+
log_dir = os.path.join(folder, 'logs')
27+
path = os.path.join(log_dir, f'{filename}.log')
28+
29+
try:
30+
# Ensure logs directory exists
31+
os.makedirs(log_dir, exist_ok=True)
32+
33+
self._fileLogger = logging.handlers.RotatingFileHandler(
34+
path, maxBytes=1024 * 1024, backupCount=20
35+
)
36+
self._fileLogger.setLevel(logging.DEBUG)
37+
# create a logging format
38+
formatter = logging.Formatter('[%(name)s][%(levelname)s][%(asctime)s] %(message)s')
39+
self._fileLogger.setFormatter(formatter)
40+
except PermissionError as e:
41+
# Log file owned by different user (e.g., root vs www-data)
42+
# Fall back to console-only logging rather than crashing
43+
print(
44+
f"[WARNING] Cannot write to log file {path}: {e}. "
45+
f"Falling back to console-only logging. "
46+
f"Fix: sudo chown www-data:www-data {log_dir} -R",
47+
file=sys.stderr
48+
)
49+
except OSError as e:
50+
# Other filesystem errors (disk full, etc.)
51+
print(
52+
f"[WARNING] Cannot create log file {path}: {e}. "
53+
f"Falling back to console-only logging.",
54+
file=sys.stderr
55+
)
2956

3057
@property
31-
def file_logger(self) -> RotatingFileHandler:
58+
def file_logger(self) -> Optional[RotatingFileHandler]:
3259
"""
3360
Get file logger.
3461
35-
:return: file logger
36-
:rtype: logging.handlers.RotatingFileHandler
62+
:return: file logger or None if file logging unavailable
63+
:rtype: Optional[logging.handlers.RotatingFileHandler]
3764
"""
3865
return self._fileLogger
3966

@@ -59,7 +86,8 @@ def create_logger(self, name: str) -> Logger:
5986
logger = logging.getLogger(name)
6087
logger.setLevel(logging.DEBUG)
6188
# add the handlers to the logger
62-
logger.addHandler(self.file_logger)
63-
logger.addHandler(self.console_logger)
89+
if self._fileLogger is not None:
90+
logger.addHandler(self._fileLogger)
91+
logger.addHandler(self._consoleLogger)
6492

6593
return logger

tests/test_log_configuration.py

Lines changed: 101 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -15,42 +15,42 @@ class TestLogConfiguration(unittest.TestCase):
1515

1616
def _test_init_with_log_value(self, debug, result_level):
1717
"""Test logger initialization with specific debug and level."""
18-
joined_path = 'baz'
1918
folder = 'foo'
2019
filename = 'bar'
20+
log_dir = 'foo/logs'
21+
log_path = 'foo/logs/bar.log'
2122
console_format = '[%(levelname)s] %(message)s'
2223
file_format = '[%(name)s][%(levelname)s][%(asctime)s] %(message)s'
2324
with mock.patch('logging.handlers.RotatingFileHandler') as mock_fh:
2425
with mock.patch('logging.StreamHandler') as mock_sh:
2526
with mock.patch('logging.Formatter') as mock_formatter:
26-
with mock.patch('os.path.join',
27-
return_value=joined_path) as mock_join:
28-
log_config = LogConfiguration(folder, filename, debug)
29-
30-
mock_sh().setLevel.assert_called_once_with(
31-
result_level)
32-
mock_sh().setFormatter.assert_called_once_with(
33-
mock_formatter())
34-
mock_fh.assert_called_once_with(joined_path,
35-
maxBytes=1024 * 1024,
36-
backupCount=20)
37-
mock_fh().setLevel.assert_called_once_with(
38-
logging.DEBUG)
39-
mock_fh().setFormatter.assert_called_once_with(
40-
mock_formatter())
41-
mock_formatter.assert_has_calls([
42-
mock.call(console_format),
43-
mock.call(file_format)
44-
])
45-
mock_join.assert_called_once_with(folder, 'logs',
46-
'%s.log' % filename)
27+
with mock.patch('os.path.join', side_effect=[log_dir, log_path]):
28+
with mock.patch('os.makedirs') as mock_makedirs:
29+
log_config = LogConfiguration(folder, filename, debug)
4730

48-
self.assertEqual(log_config._consoleLogger, mock_sh())
49-
self.assertEqual(log_config.console_logger, mock_sh())
50-
self.assertEqual(log_config._fileLogger, mock_fh())
51-
self.assertEqual(log_config.file_logger, mock_fh())
31+
mock_makedirs.assert_called_once_with(log_dir, exist_ok=True)
32+
mock_sh().setLevel.assert_called_once_with(
33+
result_level)
34+
mock_sh().setFormatter.assert_called_once_with(
35+
mock_formatter())
36+
mock_fh.assert_called_once_with(log_path,
37+
maxBytes=1024 * 1024,
38+
backupCount=20)
39+
mock_fh().setLevel.assert_called_once_with(
40+
logging.DEBUG)
41+
mock_fh().setFormatter.assert_called_once_with(
42+
mock_formatter())
43+
mock_formatter.assert_has_calls([
44+
mock.call(console_format),
45+
mock.call(file_format)
46+
])
47+
48+
self.assertEqual(log_config._consoleLogger, mock_sh())
49+
self.assertEqual(log_config.console_logger, mock_sh())
50+
self.assertEqual(log_config._fileLogger, mock_fh())
51+
self.assertEqual(log_config.file_logger, mock_fh())
5252

53-
return log_config
53+
return log_config
5454

5555
def test_init_correctly_initializes_the_instance_when_debug(self):
5656
"""Test log initialization with debug mode and level."""
@@ -60,24 +60,82 @@ def test_init_correctly_initializes_the_instance_when_no_debug(self):
6060
"""Test log initialization with info level."""
6161
self._test_init_with_log_value(False, logging.INFO)
6262

63+
def test_init_handles_permission_error(self):
64+
"""Test that permission errors fall back to console-only logging."""
65+
folder = 'foo'
66+
filename = 'bar'
67+
log_dir = 'foo/logs'
68+
log_path = 'foo/logs/bar.log'
69+
with mock.patch('logging.handlers.RotatingFileHandler') as mock_fh:
70+
mock_fh.side_effect = PermissionError("Permission denied")
71+
with mock.patch('logging.StreamHandler') as mock_sh:
72+
with mock.patch('os.path.join', side_effect=[log_dir, log_path]):
73+
with mock.patch('os.makedirs'):
74+
log_config = LogConfiguration(folder, filename, False)
75+
76+
# Console logger should still be set up
77+
self.assertEqual(log_config._consoleLogger, mock_sh())
78+
# File logger should be None
79+
self.assertIsNone(log_config._fileLogger)
80+
self.assertIsNone(log_config.file_logger)
81+
82+
def test_init_handles_os_error(self):
83+
"""Test that OSError falls back to console-only logging."""
84+
folder = 'foo'
85+
filename = 'bar'
86+
log_dir = 'foo/logs'
87+
log_path = 'foo/logs/bar.log'
88+
with mock.patch('logging.handlers.RotatingFileHandler') as mock_fh:
89+
mock_fh.side_effect = OSError("Disk full")
90+
with mock.patch('logging.StreamHandler') as mock_sh:
91+
with mock.patch('os.path.join', side_effect=[log_dir, log_path]):
92+
with mock.patch('os.makedirs'):
93+
log_config = LogConfiguration(folder, filename, False)
94+
95+
# Console logger should still be set up
96+
self.assertEqual(log_config._consoleLogger, mock_sh())
97+
# File logger should be None
98+
self.assertIsNone(log_config._fileLogger)
99+
63100
def test_create_logger(self):
64101
"""Test logger creation."""
65102
with mock.patch.object(LogConfiguration, '__init__',
66103
return_value=None):
67104
with mock.patch('logging.getLogger') as mock_get:
68-
with mock.patch.object(LogConfiguration, 'file_logger'):
69-
with mock.patch.object(LogConfiguration, 'console_logger'):
70-
log_config = LogConfiguration('foo', 'bar')
71-
name = 'foobar'
72-
73-
result = log_config.create_logger(name)
74-
75-
mock_get.assert_called_once_with(name)
76-
mock_get().setLevel.assert_called_once_with(
77-
logging.DEBUG)
78-
mock_get().addHandler.assert_has_calls([
79-
mock.call(log_config.file_logger),
80-
mock.call(log_config.console_logger)
81-
])
82-
83-
self.assertEqual(result, mock_get())
105+
log_config = LogConfiguration('foo', 'bar')
106+
log_config._fileLogger = mock.MagicMock()
107+
log_config._consoleLogger = mock.MagicMock()
108+
name = 'foobar'
109+
110+
result = log_config.create_logger(name)
111+
112+
mock_get.assert_called_once_with(name)
113+
mock_get().setLevel.assert_called_once_with(
114+
logging.DEBUG)
115+
mock_get().addHandler.assert_has_calls([
116+
mock.call(log_config._fileLogger),
117+
mock.call(log_config._consoleLogger)
118+
])
119+
120+
self.assertEqual(result, mock_get())
121+
122+
def test_create_logger_without_file_logger(self):
123+
"""Test logger creation when file logger is unavailable."""
124+
with mock.patch.object(LogConfiguration, '__init__',
125+
return_value=None):
126+
with mock.patch('logging.getLogger') as mock_get:
127+
log_config = LogConfiguration('foo', 'bar')
128+
log_config._fileLogger = None
129+
log_config._consoleLogger = mock.MagicMock()
130+
name = 'foobar'
131+
132+
result = log_config.create_logger(name)
133+
134+
mock_get.assert_called_once_with(name)
135+
mock_get().setLevel.assert_called_once_with(
136+
logging.DEBUG)
137+
# Only console logger should be added
138+
mock_get().addHandler.assert_called_once_with(
139+
log_config._consoleLogger)
140+
141+
self.assertEqual(result, mock_get())

0 commit comments

Comments
 (0)