From d297cc3ff6e32350bf610d8eac13c971576a1da4 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 23 Oct 2026 20:58:19 +0200 Subject: [PATCH] fix: bound the wait for a silent list server A server that took the connection and never answered held line mode 93 s, three waits of 30 s for a first byte. A file list now waits 15 s per answer and asks a silent server twice. Downloads keep their own timeout between chunks. --- install.ps1 | 2 +- install.py | 30 ++++++++++++++++++++++++++---- install.sh | 2 +- tests/test_install.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 6 deletions(-) diff --git a/install.ps1 b/install.ps1 index b00b2a73..fdba5838 100644 --- a/install.ps1 +++ b/install.ps1 @@ -11,7 +11,7 @@ $ErrorActionPreference = "Stop" $defaultInstallUrl = "https://raw.githubusercontent.com/Abdess/retrobios/main/install.py" - $defaultInstallSha256 = "1f6b90760303d04cb6beef7fd294371b2e1d4ba733578c59a95e1b0395c2f376" + $defaultInstallSha256 = "aef82904ca3356ded55f7b43bc03fe80dc30da178a28bc598caef8367b3f9766" $maximumInstallerBytes = 2MB $installerExit = 0 $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null } diff --git a/install.py b/install.py index 874efc26..76b83f59 100644 --- a/install.py +++ b/install.py @@ -3322,16 +3322,34 @@ def install_bounded_opener() -> None: urllib.request.install_opener(urllib.request.build_opener(*handlers)) +# The wait for each answer of a file list, its first byte included: a +# server that takes the connection and never answers (a captive portal, a +# stuck proxy) held the run 93 s on three waits of 30 s. A list is a few +# MB, never a slow stream. +LIST_TIMEOUT = 15.0 +# Attempts after a server that took the connection went silent: once more, +# a stall is rarely a blip. +STALLED_ATTEMPTS = 2 + + +def _stalled(exc: BaseException) -> bool: + """Whether exc is a wait that ran out, the connection made.""" + reason = getattr(exc, "reason", None) + return isinstance(exc, socket.timeout) or isinstance(reason, socket.timeout) + + def _fetch_json( - url: str, limit: int, label: str, attempts: int = MAX_RETRIES, timeout: float = 30, + url: str, limit: int, label: str, attempts: int = MAX_RETRIES, + timeout: float | None = None, ) -> object: """A JSON document, tried MAX_RETRIES times like every file download. A file list fetched once ended the run on one timeout over flaky Wi-Fi. A 4xx answer or an invalid document is final: asking again changes - nothing. + nothing, and a server gone silent is asked once more only. """ _network_allowed(label) + timeout = LIST_TIMEOUT if timeout is None else timeout for attempt in range(1, attempts + 1): try: with urllib.request.urlopen(url, timeout=timeout) as resp: @@ -3344,9 +3362,12 @@ def _fetch_json( except (urllib.error.URLError, OSError, http.client.HTTPException) as exc: code, problem = network_words(exc) # A wrong clock or no certificate list fails every attempt alike. - if attempt == attempts or code != "network": + if (attempt == attempts or code != "network" + or (_stalled(exc) and attempt >= STALLED_ATTEMPTS)): if isinstance(exc, http.client.HTTPException): raise ConnectionError(f"connection cut: {exc!r}") from exc + # How many were made, for the line that says so. + exc.attempts = attempt # type: ignore[union-attr] raise delay = _retry_delay(attempt, None) # Said, or three 30-second attempts read as a frozen program. @@ -3416,7 +3437,8 @@ def fetch_manifest(plat: str) -> dict: print(f" Could not fetch the file list for {plat}.", file=sys.stderr) print(f" {words}.", file=sys.stderr) sys.exit(1) - print(f" Could not fetch the file list for {plat} after {MAX_RETRIES} " + tries = getattr(exc, "attempts", MAX_RETRIES) + print(f" Could not fetch the file list for {plat} after {tries} " f"attempts: {words}", file=sys.stderr) for line in _address_lines(url): print(line, file=sys.stderr) diff --git a/install.sh b/install.sh index 07943fb3..e1e8a74a 100755 --- a/install.sh +++ b/install.sh @@ -18,7 +18,7 @@ esac TEMP_INSTALLER="" TEMP_DIRECTORY="" DEFAULT_INSTALL_URL="https://raw.githubusercontent.com/Abdess/retrobios/main/install.py" -DEFAULT_INSTALL_SHA256="1f6b90760303d04cb6beef7fd294371b2e1d4ba733578c59a95e1b0395c2f376" +DEFAULT_INSTALL_SHA256="aef82904ca3356ded55f7b43bc03fe80dc30da178a28bc598caef8367b3f9766" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/tests/test_install.py b/tests/test_install.py index b7cb3285..1748ab29 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -17,6 +17,7 @@ import subprocess import sys import tempfile import threading +import time import unittest from types import SimpleNamespace import unittest.mock @@ -2369,6 +2370,48 @@ class FileListsAreRetried(unittest.TestCase): self.assertIn(f"after {install.MAX_RETRIES} attempts", err.getvalue()) +class ASilentServerIsBounded(unittest.TestCase): + """A server that takes the connection and never answers held line mode + 93 s: three waits of 30 s for a first byte that never came.""" + + def test_two_short_waits_then_the_reason(self): + import socket + import threading + + listener = socket.socket() + listener.bind(("127.0.0.1", 0)) + listener.listen(8) + self.addCleanup(listener.close) + held = [] + + def accept(): + while True: + try: + conn, _addr = listener.accept() + except OSError: + return + held.append(conn) + + threading.Thread(target=accept, daemon=True).start() + self.addCleanup(lambda: [conn.close() for conn in held]) + port = listener.getsockname()[1] + err = io.StringIO() + start = time.monotonic() + with unittest.mock.patch.object(install, "LIST_TIMEOUT", 0.5), \ + unittest.mock.patch.object(install, "_retry_delay", return_value=0), \ + unittest.mock.patch.object(install, "MANIFEST_URL", + f"http://127.0.0.1:{port}/{{platform}}.json"), \ + contextlib.redirect_stderr(err): + with self.assertRaises(SystemExit): + install.fetch_manifest("retroarch") + elapsed = time.monotonic() - start + self.assertLess(elapsed, 5) + self.assertEqual(len(held), install.STALLED_ATTEMPTS) + self.assertIn(f"after {install.STALLED_ATTEMPTS} attempts", err.getvalue()) + # A download keeps its own, longer wait between chunks. + self.assertGreater(install.LIST_TIMEOUT, 1) + + class TargetListFailureIsNotAnAnswer(unittest.TestCase): """A timeout is not 'no targets': it sent users to the full pack."""