From f0db3d7d1405544fdf5ba6eaed1791d50518d73a Mon Sep 17 00:00:00 2001 From: Ahmed Elshahat Date: Sun, 7 Jun 2026 04:24:12 +0300 Subject: [PATCH] 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. --- scrapling/spiders/cache.py | 2 +- scrapling/spiders/checkpoint.py | 2 +- tests/spiders/test_cache.py | 21 +++++++++++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/scrapling/spiders/cache.py b/scrapling/spiders/cache.py index 40d39d3..0305aef 100644 --- a/scrapling/spiders/cache.py +++ b/scrapling/spiders/cache.py @@ -64,7 +64,7 @@ class ResponseCacheManager: async with await anyio.open_file(temp_path, "wb") as f: await f.write(serialized) - await temp_path.rename(self._cache_path(fingerprint)) + await temp_path.replace(self._cache_path(fingerprint)) except Exception as e: if await temp_path.exists(): await temp_path.unlink() diff --git a/scrapling/spiders/checkpoint.py b/scrapling/spiders/checkpoint.py index 25de362..95515bf 100644 --- a/scrapling/spiders/checkpoint.py +++ b/scrapling/spiders/checkpoint.py @@ -50,7 +50,7 @@ class CheckpointManager: async with await anyio.open_file(temp_path, "wb") as f: await f.write(serialized) - await temp_path.rename(self._checkpoint_path) + await temp_path.replace(self._checkpoint_path) log.info(f"Checkpoint saved: {len(data.requests)} requests, {len(data.seen)} seen URLs") except Exception as e: diff --git a/tests/spiders/test_cache.py b/tests/spiders/test_cache.py index fc9bc59..eb1b04b 100644 --- a/tests/spiders/test_cache.py +++ b/tests/spiders/test_cache.py @@ -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"first"), "GET") + await cache.put(fp, _make_response(body=b"second"), "GET") + + restored = await cache.get(fp) + assert restored is not None + assert restored.body == b"second" + @pytest.mark.anyio async def test_get_cache_miss(self): with tempfile.TemporaryDirectory() as tmpdir: