fix(spiders): use os.replace for atomic checkpoint/cache writes on Windows
CheckpointManager.save() and ResponseCacheManager.put() write to a temp file and then move it into place with Path.rename(). On Windows, os.rename cannot overwrite an existing destination and raises FileExistsError (WinError 183), so every write after the first one fails: checkpoint saving raises and breaks resume, while the development response cache swallows the error and keeps returning the stale entry. Path.replace() (os.replace) overwrites the destination atomically on every platform and behaves identically to rename() on POSIX, so this is a no-op on Linux and macOS and only fixes the broken overwrite on Windows. Add a regression test for the cache overwrite path; the checkpoint overwrite is already covered by test_multiple_saves_overwrite.
This commit is contained in:
@@ -49,6 +49,27 @@ class TestResponseCacheManager:
|
||||
assert dict(restored.headers) == dict(original.headers)
|
||||
assert dict(restored.request_headers) == dict(original.request_headers)
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_put_overwrites_existing_entry(self):
|
||||
"""Re-caching the same fingerprint must replace the stored response.
|
||||
|
||||
Regression test for a Windows-only failure: ``Path.rename`` cannot
|
||||
overwrite an existing destination on Windows (raising ``WinError 183``),
|
||||
so the second ``put`` was caught by the error handler, the temp file was
|
||||
removed, and ``get`` kept returning the stale body. ``Path.replace``
|
||||
overwrites atomically on every platform.
|
||||
"""
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
cache = ResponseCacheManager(tmpdir)
|
||||
fp = b"\x05" * 20
|
||||
|
||||
await cache.put(fp, _make_response(body=b"<html>first</html>"), "GET")
|
||||
await cache.put(fp, _make_response(body=b"<html>second</html>"), "GET")
|
||||
|
||||
restored = await cache.get(fp)
|
||||
assert restored is not None
|
||||
assert restored.body == b"<html>second</html>"
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_get_cache_miss(self):
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
|
||||
Reference in New Issue
Block a user