Temp file cleanup#80
Open
emmanuelmingo wants to merge 2 commits intoindictechcom:masterfrom
Open
Conversation
Introduces a custom exception hierarchy, structured rotating file-based
logging, standardised API error responses, and proper error tracking for
asynchronous Celery tasks.
Changes
-------
exceptions.py (new)
- WikifileTransferError as root base for all application exceptions
- APIError(HTTPException) as base for HTTP-facing errors; subclasses:
ValidationError (422), NotFoundError (404), AuthenticationError (401),
UploadError (502), ExternalAPIError (502)
- Internal exceptions: DownloadError, DatabaseError, TaskError
logging_config.py (new)
- configure_logging() sets up a RotatingFileHandler (5 MB / 3 backups)
writing to logs/app.log plus a console handler
- Extracted from app.py so logging config lives in its own module
error_handlers.py (updated)
- register_error_handlers() now logs HTTP errors at WARNING and
unexpected exceptions at ERROR with full traceback
- All responses follow a consistent envelope:
{"success": bool, "data": {}, "errors": [...]}
app.py (updated)
- Import and call configure_logging() from logging_config
- Raise ValidationError / AuthenticationError / ExternalAPIError /
DownloadError / DatabaseError at every failure point instead of
returning manual JSON error responses
- /api/task_status: fix PROGRESS state data (use task.info) and
SUCCESS state data (use task.result); remove non-serialisable result
- Replace bare except: clauses with specific exception types
- Standardise response envelope to use "errors" list throughout
utils.py (updated)
- download_image: raise DownloadError instead of returning None;
specific messages for network errors, missing imageinfo, disk failures
- process_upload: raise UploadError instead of returning None;
specific messages for CSRF, network, file-read, and wiki-rejection
- get_localized_wikitext: replace bare except with RequestException
and (KeyError, IndexError) handlers; log at appropriate levels
- All file handles opened with context managers (with open(...))
tasks.py (updated)
- Add structured logging with task_id on every log line
- Wrap each stage (CSRF fetch, file upload, response parse) in
try/except raising TaskError so Celery marks the task FAILURE
- File opened with context manager to prevent handle leaks
- Remove unused import os
tests/ (new)
- tests/conftest.py: session-scoped Flask test client with MWOAuth
and Celery mocked out; auth_client fixture for authenticated routes
- tests/test_exceptions.py: 33 tests covering inheritance chain,
HTTP status codes, werkzeug compatibility, and message handling
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add cleanup_temp_file() helper to utils.py that safely removes a temp file and logs a warning on failure without raising - Wrap synchronous process_upload() call in app.py with try/finally so the temp file is deleted on both success and UploadError - Wrap the entire async upload_image_task body in tasks.py with try/finally so the temp file is deleted regardless of outcome - Remove unused UploadError import from app.py
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Improve temporary file handling and prevent resource leaks
This PR fixes file handle leaks and ensures temporary files created during uploads are always cleaned up, regardless of whether the upload succeeds or fails.
Problem
Temporary files downloaded to temp_images/ were never deleted after an upload completed. Over time this would cause disk usage to grow unboundedly. Additionally, the async Celery task opened file handles without guaranteeing cleanup on failure.
Changes
utils.py — Added cleanup_temp_file() helper that safely removes a temp file and logs a warning on failure without raising, so cleanup errors never interrupt the response.
app.py — Wrapped the synchronous process_upload() call in a try/finally block so the temp file is deleted on both success and UploadError.
tasks.py — Wrapped the entire async task body in a try/finally block so the temp file is deleted after the Celery task finishes, whether it succeeded or failed.
Outcome
No orphaned files left in temp_images/ after any upload path (sync or async)
No leaked file handles
Cleanup failures are logged as warnings and never surface to the client