diff --git a/hermes_cli/release_channels.py b/hermes_cli/release_channels.py index 3fe05a957a..0e556cbcdb 100644 --- a/hermes_cli/release_channels.py +++ b/hermes_cli/release_channels.py @@ -1,6 +1,8 @@ """R2 channel wire protocol. Names are data; no local channel registry exists.""" from __future__ import annotations +from contextlib import contextmanager +from contextvars import ContextVar from dataclasses import dataclass import hashlib import json @@ -253,6 +255,25 @@ class _NoRedirect(HTTPRedirectHandler): raise ChannelError("Channel archive redirects are not permitted") +_RETRY_READS: ContextVar[bool] = ContextVar("release_channel_read_retries", default=False) + + +@contextmanager +def retrying_reads(): + """Let channel reads inside the block wait out a CDN blip with PM's bounded retries. + + Only an explicit ``hermes update`` opts in. Passive checks (``hermes --version``, the + banner, the Desktop/dashboard update check) make one attempt: offline also reads as + transient (DNS EAI_AGAIN, ENETUNREACH), so retrying there adds backoff to a synchronous + status line, stretches a hung CDN from 30 s to ~2 min and logs a WARNING per retry. + """ + token = _RETRY_READS.set(True) + try: + yield + finally: + _RETRY_READS.reset(token) + + class ChannelReader: def __init__(self, base_url: str, repository: str | None = None, opener=None): self.base_url = public_base(base_url) @@ -264,8 +285,8 @@ class ChannelReader: if sha256 is not None: require_sha256(sha256) - # Channel reads are idempotent and precede the PM download path, so use the same bounded - # transient-failure policy instead of maintaining a second retry classifier here. + # Channel reads are idempotent and precede the PM download path, so an explicit update + # uses the same bounded transient-failure policy instead of a second retry classifier. from pm.network import retry_network def read() -> bytes: @@ -275,7 +296,7 @@ class ChannelReader: return response.read(MAX_METADATA + 1) try: - body = retry_network(read) + body = retry_network(read) if _RETRY_READS.get() else read() except HTTPError as exc: if exc.code == 404: raise ChannelNotFound(f"Channel object not found: {key}") from exc diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 88d424722c..d5dc953052 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -1309,6 +1309,7 @@ def _cmd_update_impl(args, gateway_mode: bool): target_repository = None selected_channel = _source_update_channel(args) if not getattr(args, "branch", None): + from hermes_cli.release_channels import retrying_reads from hermes_cli.source_releases import resolve_source_target from copy import deepcopy @@ -1319,8 +1320,9 @@ def _cmd_update_impl(args, gateway_mode: bool): Path(completion_request["home"]) / "config.yaml"), _m().PROJECT_ROOT)) print(f"→ Update channel: {selected_channel}") try: - target = resolve_source_target( - selected_channel, None if use_zip_update else git_cmd, _m().PROJECT_ROOT) + with retrying_reads(): + target = resolve_source_target( + selected_channel, None if use_zip_update else git_cmd, _m().PROJECT_ROOT) except (OSError, ValueError, subprocess.SubprocessError) as exc: print(f"✗ Could not resolve the {selected_channel} source channel: {exc}. No update was applied.") _m()._resume_windows_gateways_after_update(_windows_gateway_resume) diff --git a/tests/hermes_cli/test_release_channels.py b/tests/hermes_cli/test_release_channels.py index b3d0d1a6a2..d26501ae84 100644 --- a/tests/hermes_cli/test_release_channels.py +++ b/tests/hermes_cli/test_release_channels.py @@ -72,10 +72,10 @@ def test_names_are_validated_without_normalizing(name): validate_name(name) -def test_reader_retries_transient_http_and_honors_retry_after(monkeypatch): +def test_update_reads_retry_transient_http_and_honor_retry_after(monkeypatch): from email.message import Message from urllib.error import HTTPError - from hermes_cli.release_channels import ChannelReader + from hermes_cli.release_channels import ChannelReader, retrying_reads url = "http://127.0.0.1:12345/releases/fixture.json" headers = Message() @@ -105,34 +105,40 @@ def test_reader_retries_transient_http_and_honors_retry_after(monkeypatch): monkeypatch.setattr("pm.network.time.sleep", waits.append) reader = ChannelReader("http://127.0.0.1:12345", opener=opener) - assert reader.read_bytes("releases/fixture.json") == b"fixture" + with retrying_reads(): + assert reader.read_bytes("releases/fixture.json") == b"fixture" assert attempts == [url, url] assert waits == [7.0] -def test_reader_exhausts_transient_retry_budget_before_channel_error(monkeypatch): +def test_passive_reads_and_missing_objects_make_one_attempt(monkeypatch): + """Offline looks transient (ENETUNREACH); a passive check must not back off on it.""" + import errno from email.message import Message - from urllib.error import HTTPError - from hermes_cli.release_channels import ChannelError, ChannelReader - from pm import network + from urllib.error import HTTPError, URLError + from hermes_cli.release_channels import ChannelError, ChannelNotFound, ChannelReader, retrying_reads - url = "http://127.0.0.1:12345/releases/fixture.json" - headers = Message() - headers["Retry-After"] = "1" - attempts = [] waits = [] + monkeypatch.setattr("pm.network.time.sleep", waits.append) - def opener(request, timeout): - assert timeout == 30 - attempts.append(request.full_url) - raise HTTPError(url, 503, "unavailable", headers, None) + def reader(fault, calls): + def opener(request, timeout): + calls.append(request.full_url) + raise fault + return ChannelReader("https://releases.example", opener=opener) - monkeypatch.setattr(network.time, "sleep", waits.append) - reader = ChannelReader("http://127.0.0.1:12345", opener=opener) - with pytest.raises(ChannelError, match="HTTP 503"): - reader.read_bytes("releases/fixture.json") - assert len(attempts) == network._ATTEMPTS - assert len(waits) == network._ATTEMPTS - 1 + calls = [] + offline = URLError(OSError(errno.ENETUNREACH, "Network is unreachable")) + with pytest.raises(ChannelError, match="unavailable"): + reader(offline, calls).read_bytes("releases/channels/main.json") + assert len(calls) == 1 + + calls.clear() + missing = HTTPError("https://releases.example/x", 404, "missing", Message(), None) + with retrying_reads(), pytest.raises(ChannelNotFound): + reader(missing, calls).read_bytes("releases/channels/stable.json") + assert len(calls) == 1 + assert waits == [] def test_reader_rejects_cycles_identity_substitution_and_cross_authority():