Propagate atomic write failures and clean temporary files - #194
Gonghan-Princess wants to merge 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #194 +/- ##
==========================================
+ Coverage 98.48% 98.86% +0.37%
==========================================
Files 13 13
Lines 794 792 -2
==========================================
+ Hits 782 783 +1
+ Misses 12 9 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@Gonghan-Princess thanks for the PR. Before going further, could you please explain what concrete issue led you to this fix? Did you encounter this behavior in a real application, and if so, could you provide the environment and steps to reproduce it? I'm asking because In particular, the existing The new tests assume that a Could you please explain the original problem and how you reproduced it? |
Describe your changes
Atomic writes can currently report success even when the write never reaches the destination:
_write_file_atomiccatches everyFileNotFoundError, including a real failure fromos.replace. Its comment assumes the temporary file deletes itself on context exit, but the file is opened withdelete=Falseand closed before replacement.This also leaves temporary files behind if writing, flushing, or syncing fails before
temp_pathis assigned.FileNotFoundErrorhandler so callers receive the actual failure.finallycleanup to handle early failures.The successful close-before-replace sequence, permission inheritance, and append behavior are unchanged. This branch starts from upstream
main, independently of the archive-fix PR.Related issue
No separate issue. Reproduced locally by patching
fsutil.io.os.replaceto raiseFileNotFoundErrorduringfsutil.write_file(existing_path, "replacement", atomic=True): before this change the call returnedNoneand retained the old content; after this change it raises the original exception and retains the old content without temporary files.Validation
Windows, Python 3.12.10:
python -X utf8 -m pytest tests --cov=fsutil --cov-report=term-missing --cov-fail-under=90: 170 passed, 3 skipped; 98.74% total coverage, 100% forio.py.python -X utf8 -m pre_commit run --all-files --show-diff-on-failure --verbose: all configured hooks passed.python -X utf8 -m mypy --install-types --non-interactive: passed.git diff --check: passed.Cross-platform CI has not been run locally; the three skipped tests are existing Windows-only skips.
Checklist before requesting a review
AI assistance and provenance
AI assistance was used to investigate the bug, implement the minimal change, write regression tests, run checks, and draft this description. No third-party code, assets, or datasets were introduced; the new code and tests are submitted under the project's existing license.