fix: survive short network outages when downloading (#2953)
* fix: use exponential backoff when a download fails The retry used a flat 3 second wait over 3 attempts, which covers only about 6 seconds of downtime. A CI runner lost DNS for approximately a minute and the PyPy download failed. Wait 3, 6, 12, 24, then 48 seconds over 6 attempts. Assisted-by: ClaudeCode:claude-opus-5 * ci: raise the uv HTTP retry count uv stops after 3 retries, which was too few when a runner lost DNS for approximately a minute during a test run. Assisted-by: ClaudeCode:claude-opus-5 * fix: do not retry a download after a client error A bad URL will not fix itself, and the longer backoff made the wait before the report about 93 seconds. Report 4xx responses at once, and keep retrying 5xx. Assisted-by: ClaudeCode:claude-opus-5
This commit is contained in:
@@ -34,6 +34,10 @@ concurrency:
|
|||||||
group: test-${{ github.ref }}
|
group: test-${{ github.ref }}
|
||||||
cancel-in-progress: true
|
cancel-in-progress: true
|
||||||
|
|
||||||
|
env:
|
||||||
|
# runners see short DNS/network outages; uv's default of 3 retries is too few
|
||||||
|
UV_HTTP_RETRIES: "6"
|
||||||
|
|
||||||
jobs:
|
jobs:
|
||||||
lint:
|
lint:
|
||||||
name: Linters (mypy, ruff, etc.)
|
name: Linters (mypy, ruff, etc.)
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ __lazy_modules__ = {
|
|||||||
"tarfile",
|
"tarfile",
|
||||||
"typing",
|
"typing",
|
||||||
"urllib",
|
"urllib",
|
||||||
|
"urllib.error",
|
||||||
"urllib.request",
|
"urllib.request",
|
||||||
"zipfile",
|
"zipfile",
|
||||||
}
|
}
|
||||||
@@ -20,6 +21,7 @@ import shutil
|
|||||||
import ssl
|
import ssl
|
||||||
import tarfile
|
import tarfile
|
||||||
import time
|
import time
|
||||||
|
import urllib.error
|
||||||
import urllib.request
|
import urllib.request
|
||||||
from contextlib import contextmanager
|
from contextlib import contextmanager
|
||||||
from pathlib import Path, PurePath
|
from pathlib import Path, PurePath
|
||||||
@@ -86,17 +88,24 @@ def download(url: str, dest: Path, *, sha256: str | None = None) -> None:
|
|||||||
# so we use certifi (this sounds odd but requests also does this by default)
|
# so we use certifi (this sounds odd but requests also does this by default)
|
||||||
cafile = os.environ.get("SSL_CERT_FILE", certifi.where())
|
cafile = os.environ.get("SSL_CERT_FILE", certifi.where())
|
||||||
context = ssl.create_default_context(cafile=cafile)
|
context = ssl.create_default_context(cafile=cafile)
|
||||||
repeat_num = 3
|
# exponential backoff, so that a network outage of about a minute is survivable
|
||||||
|
repeat_num = 6
|
||||||
for i in range(repeat_num):
|
for i in range(repeat_num):
|
||||||
try:
|
try:
|
||||||
with urllib.request.urlopen(url, context=context) as response:
|
with urllib.request.urlopen(url, context=context) as response:
|
||||||
dest.write_bytes(response.read())
|
dest.write_bytes(response.read())
|
||||||
break
|
break
|
||||||
|
|
||||||
|
except urllib.error.HTTPError as error:
|
||||||
|
# a client error, such as a bad URL, will not fix itself
|
||||||
|
if i == repeat_num - 1 or 400 <= error.code < 500:
|
||||||
|
raise
|
||||||
|
time.sleep(3 * 2**i)
|
||||||
|
|
||||||
except OSError:
|
except OSError:
|
||||||
if i == repeat_num - 1:
|
if i == repeat_num - 1:
|
||||||
raise
|
raise
|
||||||
time.sleep(3)
|
time.sleep(3 * 2**i)
|
||||||
|
|
||||||
if sha256:
|
if sha256:
|
||||||
with dest.open("rb") as f:
|
with dest.open("rb") as f:
|
||||||
|
|||||||
@@ -1,6 +1,10 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import io
|
||||||
import ssl
|
import ssl
|
||||||
|
import time
|
||||||
|
import urllib.error
|
||||||
|
import urllib.request
|
||||||
|
|
||||||
import certifi
|
import certifi
|
||||||
import pytest
|
import pytest
|
||||||
@@ -9,9 +13,12 @@ from cibuildwheel.util.file import download
|
|||||||
|
|
||||||
TYPE_CHECKING = False
|
TYPE_CHECKING = False
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
|
from collections.abc import Callable, Iterator
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
from typing import Self
|
||||||
|
|
||||||
DOWNLOAD_URL = "https://cdn.jsdelivr.net/gh/pypa/cibuildwheel@v1.6.3/requirements-dev.txt"
|
DOWNLOAD_URL = "https://cdn.jsdelivr.net/gh/pypa/cibuildwheel@v1.6.3/requirements-dev.txt"
|
||||||
|
PAYLOAD = b"payload"
|
||||||
|
|
||||||
|
|
||||||
def test_download(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
|
def test_download(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
|
||||||
@@ -35,3 +42,131 @@ def test_download_bad_ssl_cert_file(monkeypatch: pytest.MonkeyPatch, tmp_path: P
|
|||||||
dest = tmp_path / "file.txt"
|
dest = tmp_path / "file.txt"
|
||||||
with pytest.raises(ssl.SSLError):
|
with pytest.raises(ssl.SSLError):
|
||||||
download(DOWNLOAD_URL, dest)
|
download(DOWNLOAD_URL, dest)
|
||||||
|
|
||||||
|
|
||||||
|
def _no_sleep(seconds: float) -> None:
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def http_error() -> Iterator[Callable[[str, int], urllib.error.HTTPError]]:
|
||||||
|
"""Build HTTPErrors, and close them, as each one holds a temporary file."""
|
||||||
|
errors = []
|
||||||
|
|
||||||
|
def make(url: str, code: int) -> urllib.error.HTTPError:
|
||||||
|
error = urllib.error.HTTPError(url, code, "error", {}, io.BytesIO(b"")) # type: ignore[arg-type]
|
||||||
|
errors.append(error)
|
||||||
|
return error
|
||||||
|
|
||||||
|
yield make
|
||||||
|
|
||||||
|
for error in errors:
|
||||||
|
error.close()
|
||||||
|
|
||||||
|
|
||||||
|
class FakeResponse:
|
||||||
|
def __enter__(self) -> Self:
|
||||||
|
return self
|
||||||
|
|
||||||
|
def __exit__(self, *args: object) -> None:
|
||||||
|
return None
|
||||||
|
|
||||||
|
def read(self) -> bytes:
|
||||||
|
return PAYLOAD
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def fake_network(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
) -> Callable[[int], tuple[list[str], list[float]]]:
|
||||||
|
"""Fail the first ``failures`` downloads, recording attempts and sleeps."""
|
||||||
|
|
||||||
|
def setup(failures: int) -> tuple[list[str], list[float]]:
|
||||||
|
attempts: list[str] = []
|
||||||
|
sleeps: list[float] = []
|
||||||
|
|
||||||
|
def fake_urlopen(url: str, context: object = None) -> FakeResponse: # noqa: ARG001
|
||||||
|
attempts.append(url)
|
||||||
|
if len(attempts) <= failures:
|
||||||
|
msg = "temporary DNS failure"
|
||||||
|
raise OSError(msg)
|
||||||
|
return FakeResponse()
|
||||||
|
|
||||||
|
monkeypatch.setattr(urllib.request, "urlopen", fake_urlopen)
|
||||||
|
monkeypatch.setattr(time, "sleep", sleeps.append)
|
||||||
|
return attempts, sleeps
|
||||||
|
|
||||||
|
return setup
|
||||||
|
|
||||||
|
|
||||||
|
def test_download_retries_transient_failures(
|
||||||
|
fake_network: Callable[[int], tuple[list[str], list[float]]], tmp_path: Path
|
||||||
|
) -> None:
|
||||||
|
attempts, sleeps = fake_network(3)
|
||||||
|
dest = tmp_path / "file.txt"
|
||||||
|
|
||||||
|
download(DOWNLOAD_URL, dest)
|
||||||
|
|
||||||
|
assert dest.read_bytes() == PAYLOAD
|
||||||
|
assert len(attempts) == 4
|
||||||
|
# the wait must grow, so that a long outage is survivable
|
||||||
|
assert sleeps == sorted(sleeps)
|
||||||
|
assert sleeps[-1] > sleeps[0]
|
||||||
|
|
||||||
|
|
||||||
|
def test_download_backoff_covers_a_minute_outage(
|
||||||
|
fake_network: Callable[[int], tuple[list[str], list[float]]], tmp_path: Path
|
||||||
|
) -> None:
|
||||||
|
_, sleeps = fake_network(99)
|
||||||
|
dest = tmp_path / "file.txt"
|
||||||
|
|
||||||
|
with pytest.raises(OSError, match="temporary DNS failure"):
|
||||||
|
download(DOWNLOAD_URL, dest)
|
||||||
|
|
||||||
|
assert sum(sleeps) >= 60
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("code", [400, 404, 410])
|
||||||
|
def test_download_does_not_retry_client_errors(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
tmp_path: Path,
|
||||||
|
code: int,
|
||||||
|
http_error: Callable[[str, int], urllib.error.HTTPError],
|
||||||
|
) -> None:
|
||||||
|
"""A bad URL is not going to fix itself, so report it at once."""
|
||||||
|
attempts: list[str] = []
|
||||||
|
|
||||||
|
def fake_urlopen(url: str, context: object = None) -> FakeResponse: # noqa: ARG001
|
||||||
|
attempts.append(url)
|
||||||
|
raise http_error(url, code)
|
||||||
|
|
||||||
|
monkeypatch.setattr(urllib.request, "urlopen", fake_urlopen)
|
||||||
|
monkeypatch.setattr(time, "sleep", _no_sleep)
|
||||||
|
|
||||||
|
with pytest.raises(urllib.error.HTTPError):
|
||||||
|
download(DOWNLOAD_URL, tmp_path / "file.txt")
|
||||||
|
|
||||||
|
assert len(attempts) == 1
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("code", [500, 503])
|
||||||
|
def test_download_retries_server_errors(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
tmp_path: Path,
|
||||||
|
code: int,
|
||||||
|
http_error: Callable[[str, int], urllib.error.HTTPError],
|
||||||
|
) -> None:
|
||||||
|
attempts: list[str] = []
|
||||||
|
|
||||||
|
def fake_urlopen(url: str, context: object = None) -> FakeResponse: # noqa: ARG001
|
||||||
|
attempts.append(url)
|
||||||
|
if len(attempts) == 1:
|
||||||
|
raise http_error(url, code)
|
||||||
|
return FakeResponse()
|
||||||
|
|
||||||
|
monkeypatch.setattr(urllib.request, "urlopen", fake_urlopen)
|
||||||
|
monkeypatch.setattr(time, "sleep", _no_sleep)
|
||||||
|
|
||||||
|
download(DOWNLOAD_URL, tmp_path / "file.txt")
|
||||||
|
|
||||||
|
assert len(attempts) == 2
|
||||||
|
|||||||
Reference in New Issue
Block a user