From 76c1a50eb175f1302321989b679ffda478be273e Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 11 Oct 2026 04:25:40 -0700 Subject: [PATCH] fix(release): an artifact download that fails its checksum is fetched again rc.11-v0.21.7's Windows x64 MSIX smoke failed with 'Artifact checksum mismatch' on the 2.6 GB package. The stored object was correct (re-downloaded: size and SHA256 match the receipt exactly), so the runner received bad bytes. _download_url retried HTTP and connection errors but raised on a completed body that failed its hash, so one bad transfer failed the job and its dependents until a whole-run rerun. A size/SHA256 mismatch now counts as a bad transfer and is fetched again within the same three attempts. A stored object that really differs from its receipt fails all three the same way, and the verified destination is never replaced. --- scripts/releases/r2.py | 13 +++++++++++-- tests/scripts/test_release_r2.py | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/scripts/releases/r2.py b/scripts/releases/r2.py index 7c4e8afe31..2962251b18 100644 --- a/scripts/releases/r2.py +++ b/scripts/releases/r2.py @@ -363,6 +363,10 @@ def read_public_receipt(base: str, key: str) -> dict: conn.close() +class _TransferMismatch(ValueError): + """A completed download whose size or SHA256 disagrees with its receipt.""" + + def download_public_object(base: str, key: str, file: Path, *, expected_size: int, expected_sha256: str) -> None: _download_url(public_artifact_url(base, key), file, lambda _attempt: {}, expected_size=expected_size, expected_sha256=expected_sha256) @@ -393,12 +397,17 @@ def _download_url(url: str, file: Path, request_headers: Callable[[int], dict[st digest.update(chunk) size += len(chunk) if size > expected_size: - raise ValueError(f"Artifact checksum mismatch: {parsed.path}") + raise _TransferMismatch(f"Artifact checksum mismatch: {parsed.path}") output.write(chunk) if size != expected_size or digest.hexdigest() != expected_sha256: - raise ValueError(f"Artifact checksum mismatch: {parsed.path}") + raise _TransferMismatch(f"Artifact checksum mismatch: {parsed.path}") temporary.replace(file) return + except _TransferMismatch: + # The receipt is the authority: a body that disagrees with it is a bad transfer, so + # fetch again; a stored object that really differs fails the same way three times. + if attempt == 3: + raise except R2RequestError as error: if error.status not in _RETRYABLE_STATUSES or attempt == 3: raise diff --git a/tests/scripts/test_release_r2.py b/tests/scripts/test_release_r2.py index cfe50969eb..67826239d8 100644 --- a/tests/scripts/test_release_r2.py +++ b/tests/scripts/test_release_r2.py @@ -793,11 +793,28 @@ def test_download_streams_verified_bytes_and_preserves_destination_on_failure(r2 monkeypatch.setattr(r2.time, 'sleep', lambda _: None) r2.download_object(**args) assert request_times == [NOW, refreshed] + # A body corrupted in transit (right length, wrong bytes) is fetched again, not failed on the spot. + sent = [] + + def corrupt_once(handler): + sent.append(1) + if len(sent) == 1: + handler._send(200, payload[:-1] + b"!") + else: + original_get(handler) + + monkeypatch.setattr(_R2StubHandler, 'do_GET', corrupt_once) + target.write_bytes(b"previous complete file") + r2.download_object(**args) + assert target.read_bytes() == payload and len(sent) == 2 monkeypatch.setattr(_R2StubHandler, 'do_GET', original_get) + # A stored object that really differs from its receipt still fails, after the bounded attempts. r2_server.store[key] = (b"corrupt replacement", '"e"') + r2_server.requests.clear() with pytest.raises(ValueError, match="checksum mismatch"): r2.download_object(**args) + assert sum(1 for method, _, _ in r2_server.requests if method == "GET") == 3 assert target.read_bytes() == payload assert list(target.parent.iterdir()) == [target] del r2_server.store[key]