fix: handle log file permission errors gracefully #1000
Merged
Conversation
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
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 <[email protected]>
cfsmp3
requested review from
canihavesomecoffee and
thealphadollar
as code owners
canihavesomecoffee
requested changes
Jan 4, 2026
Comment on lines
+40
to
+48
| except PermissionError as e: | ||
| # Log file owned by different user (e.g., root vs www-data) | ||
| # Fall back to console-only logging rather than crashing | ||
| print( | ||
| f"[WARNING] Cannot write to log file {path}: {e}. " | ||
| f"Falling back to console-only logging. " | ||
| f"Fix: sudo chown www-data:www-data {log_dir} -R", | ||
| file=sys.stderr | ||
| ) |
Member
There was a problem hiding this comment.
If we don't crash, this will just go unnoticed and we'll lose a shit ton of logs. I'd rather see extra changes in the deploy script that guarantee the logs are correctly owned...
Per review feedback: ensure logs directory is owned by www-data during deployment, preventing silent fallback to console-only logging. The pre-deploy script now: - Checks if logs directory has correct ownership - Automatically fixes ownership if possible (when run as root) - Fails deployment if ownership cannot be fixed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Contributor
Author
|
Good point! I've added a check in
This ensures the issue is caught and fixed during deployment rather than silently falling back to console-only logging. The graceful fallback in |
|
canihavesomecoffee
approved these changes
Jan 4, 2026
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
Fixes crash on startup when log file is owned by a different user (e.g., root vs www-data).
Problem: When the web service runs under
www-databut the log file/var/www/sample-platform/logs/platform.logis owned byroot, theLogConfigurationclass would throw aPermissionErrorand crash the entire application on startup.Solution: Handle permission errors gracefully by falling back to console-only logging instead of crashing.
Changes
PermissionErrorandOSErrorwhen creating the file handlerfile_loggerproperty to returnOptional[RotatingFileHandler]create_loggerto skip file handler when unavailableTest plan
🤖 Generated with Claude Code