From 044d386ac7ac85bcb3592259eaca20b11fdde93e Mon Sep 17 00:00:00 2001 From: Developer Date: Fri, 10 Jul 2026 11:43:07 +0000 Subject: [PATCH] fix: split HTTP connect/read timeouts (Jellyfin build + qBit stats) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every HTTP client passed an integer timeout to requests, applying the same value to BOTH connect and read phases. A slow Jellyfin /Items page or qBit /sync/maindata blew through the 10s read budget → ReadTimeoutError. Split into a (connect=5s, read=60s default) tuple via shared http_timeout() helper. The media index build worker uses a 180s read floor. Existing services with low timeout_seconds benefit from bumping to 60+. --- CHANGELOG.md | 13 +++++++ .../clients/authentik.py | 7 +++- .../clients/http_timeout.py | 36 +++++++++++++++++ .../clients/jellyfin.py | 7 +++- .../clients/jellyseerr.py | 7 +++- .../clients/qbittorrent.py | 7 +++- .../integrations/alertmanager.py | 4 +- .../integrations/authentik.py | 4 +- .../integrations/jellyfin.py | 4 +- .../integrations/nextcloud.py | 2 +- .../integrations/prometheus.py | 4 +- .../integrations/qbittorrent.py | 4 +- .../routers/monitoring.py | 11 ++++-- .../widgets/sources.py | 8 ++-- .../workers/media_index_worker.py | 9 ++++- backend/tests/test_http_timeout.py | 39 +++++++++++++++++++ backend/tests/test_qbittorrent_client.py | 15 +++++++ 17 files changed, 152 insertions(+), 29 deletions(-) create mode 100644 backend/src/media_library_viewer_api/clients/http_timeout.py create mode 100644 backend/tests/test_http_timeout.py diff --git a/CHANGELOG.md b/CHANGELOG.md index bb81ae0..25617bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,19 @@ All notable changes to Manage. Breaking changes are marked with **BREAKING**. ## [Unreleased] +### Fixed — HTTP read timeouts + +- Service HTTP clients now use a `(connect, read)` timeout tuple (connect 5s, + read 60s default) instead of a single integer, resolving `ReadTimeoutError` + on slow Jellyfin index builds and qBittorrent stats. The media index build + worker uses a 180s read floor so slow `/Items` pages on large libraries + don't time out mid-build. +- The shared `http_timeout()` helper (`clients/http_timeout.py`) decouples + connect (fail-fast on dead hosts) from read (generous for slow responses). +- Integration `timeout_seconds` defaults were raised from 5/10s to 15/60s. +- Existing services with a low `timeout_seconds` may benefit from bumping it + to 60+ via the service editor. + ### **BREAKING** — Prometheus queries now route through Grafana gateway - The `prometheus` service config changed: `base_url` is replaced by diff --git a/backend/src/media_library_viewer_api/clients/authentik.py b/backend/src/media_library_viewer_api/clients/authentik.py index 8c94a10..c355e62 100644 --- a/backend/src/media_library_viewer_api/clients/authentik.py +++ b/backend/src/media_library_viewer_api/clients/authentik.py @@ -13,13 +13,15 @@ from typing import Any import requests +from media_library_viewer_api.clients.http_timeout import DEFAULT_READ_TIMEOUT, http_timeout + logger = logging.getLogger(__name__) class AuthentikClient: """Small wrapper around the Authentik core directory API.""" - def __init__(self, base_url: str, api_token: str, timeout: float = 10.0): + def __init__(self, base_url: str, api_token: str, timeout: float = DEFAULT_READ_TIMEOUT): if not base_url: raise ValueError("Authentik base_url is required") if not api_token: @@ -29,7 +31,8 @@ class AuthentikClient: if self.base_url.endswith("/api/v3"): self.base_url = self.base_url[:-7] self.api_token = api_token - self.timeout = timeout + # timeout is the per-response READ timeout (seconds); connect timeout is fixed at 5s. + self.timeout = http_timeout(timeout) self.session = requests.Session() self.session.headers.update( { diff --git a/backend/src/media_library_viewer_api/clients/http_timeout.py b/backend/src/media_library_viewer_api/clients/http_timeout.py new file mode 100644 index 0000000..0678b2c --- /dev/null +++ b/backend/src/media_library_viewer_api/clients/http_timeout.py @@ -0,0 +1,36 @@ +"""Shared HTTP timeout helpers. + +``requests`` accepts a single integer timeout and applies it to BOTH the +connect and read phases. For slow upstream services (large Jellyfin +libraries, qBittorrent with many torrents), the read phase needs a much +larger budget than connect. These helpers produce ``(connect, read)`` tuples +so the two phases are decoupled. +""" + +from __future__ import annotations + +#: Short connect timeout — fail fast on unreachable/dead hosts. +DEFAULT_CONNECT_TIMEOUT = 5.0 + +#: Generous read timeout — let slow responses complete. +DEFAULT_READ_TIMEOUT = 60.0 + + +def http_timeout( + read_timeout: float | int | None = None, + connect_timeout: float = DEFAULT_CONNECT_TIMEOUT, +) -> tuple[float, float]: + """Build a ``(connect, read)`` timeout tuple for ``requests``. + + ``read_timeout`` is the per-response read budget (seconds). When omitted + or non-positive, :data:`DEFAULT_READ_TIMEOUT` applies. + """ + effective_read = DEFAULT_READ_TIMEOUT + if read_timeout is not None: + try: + parsed = float(read_timeout) + if parsed > 0: + effective_read = parsed + except (TypeError, ValueError): + pass # fall back to default on non-numeric input + return (connect_timeout, effective_read) diff --git a/backend/src/media_library_viewer_api/clients/jellyfin.py b/backend/src/media_library_viewer_api/clients/jellyfin.py index 64ae848..30a72a9 100644 --- a/backend/src/media_library_viewer_api/clients/jellyfin.py +++ b/backend/src/media_library_viewer_api/clients/jellyfin.py @@ -12,6 +12,8 @@ from typing import Any, cast import requests +from media_library_viewer_api.clients.http_timeout import DEFAULT_READ_TIMEOUT, http_timeout + logger = logging.getLogger(__name__) @@ -35,7 +37,7 @@ DEFAULT_FIELDS = ",".join( class JellyfinClient: """Small wrapper around the Jellyfin/Emby-compatible HTTP API.""" - def __init__(self, base_url: str, api_key: str, timeout: int = 30): + def __init__(self, base_url: str, api_key: str, timeout: float = DEFAULT_READ_TIMEOUT): if not base_url: raise ValueError("Jellyfin URL is required") if not api_key: @@ -47,7 +49,8 @@ class JellyfinClient: if self.base_url.endswith("/web"): self.base_url = self.base_url[:-4] self.api_key = api_key - self.timeout = timeout + # timeout is the per-response READ timeout (seconds); connect timeout is fixed at 5s. + self.timeout = http_timeout(timeout) self.session = requests.Session() self.session.headers.update( { diff --git a/backend/src/media_library_viewer_api/clients/jellyseerr.py b/backend/src/media_library_viewer_api/clients/jellyseerr.py index aebe8ab..4b6d22f 100644 --- a/backend/src/media_library_viewer_api/clients/jellyseerr.py +++ b/backend/src/media_library_viewer_api/clients/jellyseerr.py @@ -11,13 +11,15 @@ from typing import Any import requests +from media_library_viewer_api.clients.http_timeout import DEFAULT_READ_TIMEOUT, http_timeout + logger = logging.getLogger(__name__) class JellyseerrClient: """Small wrapper around the Jellyseerr REST API.""" - def __init__(self, base_url: str, api_key: str, timeout: int = 30): + def __init__(self, base_url: str, api_key: str, timeout: float = DEFAULT_READ_TIMEOUT): if not base_url: raise ValueError("Jellyseerr URL is required") if not api_key: @@ -27,7 +29,8 @@ class JellyseerrClient: if self.base_url.endswith("/api/v1"): self.base_url = self.base_url[:-7] self.api_key = api_key - self.timeout = timeout + # timeout is the per-response READ timeout (seconds); connect timeout is fixed at 5s. + self.timeout = http_timeout(timeout) self.session = requests.Session() self.session.headers.update( { diff --git a/backend/src/media_library_viewer_api/clients/qbittorrent.py b/backend/src/media_library_viewer_api/clients/qbittorrent.py index b8232c7..435ec89 100644 --- a/backend/src/media_library_viewer_api/clients/qbittorrent.py +++ b/backend/src/media_library_viewer_api/clients/qbittorrent.py @@ -12,6 +12,8 @@ from typing import Any import requests +from media_library_viewer_api.clients.http_timeout import DEFAULT_READ_TIMEOUT, http_timeout + logger = logging.getLogger(__name__) @@ -23,7 +25,7 @@ class QbittorrentClient: :class:`requests.Session` that carries the login cookie. """ - def __init__(self, base_url: str, username: str, password: str, timeout: int = 10) -> None: + def __init__(self, base_url: str, username: str, password: str, timeout: float = DEFAULT_READ_TIMEOUT) -> None: if not base_url: raise ValueError("qBittorrent base_url is required") if not username: @@ -34,7 +36,8 @@ class QbittorrentClient: self.base_url += "/api/v2" self._username = username self._password = password - self.timeout = timeout + # timeout is the per-response READ timeout (seconds); connect timeout is fixed at 5s. + self.timeout = http_timeout(timeout) self._session = requests.Session() self._logged_in = False diff --git a/backend/src/media_library_viewer_api/integrations/alertmanager.py b/backend/src/media_library_viewer_api/integrations/alertmanager.py index ccbef80..6b54ab4 100644 --- a/backend/src/media_library_viewer_api/integrations/alertmanager.py +++ b/backend/src/media_library_viewer_api/integrations/alertmanager.py @@ -25,7 +25,7 @@ class AlertmanagerConfig(ServiceConfigBase): """Non-secret Alertmanager connection config.""" base_url: ServiceBaseUrl - timeout_seconds: int = 5 + timeout_seconds: int = 15 class AlertmanagerAlertsWidgetConfig(WidgetConfigBase): @@ -83,7 +83,7 @@ def test_connection( """GET /api/v2/status with optional bearer auth.""" try: base_url = str(config.get("base_url") or "").rstrip("/") - timeout = int(config.get("timeout_seconds") or 5) + timeout = int(config.get("timeout_seconds") or 15) headers: dict[str, str] = {} api_key = str(secrets.get("api_key") or "") if api_key: diff --git a/backend/src/media_library_viewer_api/integrations/authentik.py b/backend/src/media_library_viewer_api/integrations/authentik.py index 6fa97bd..508d077 100644 --- a/backend/src/media_library_viewer_api/integrations/authentik.py +++ b/backend/src/media_library_viewer_api/integrations/authentik.py @@ -33,7 +33,7 @@ def test_connection( try: base_url = str(config.get("base_url") or "").rstrip("/") api_token = str(secrets.get("api_token") or "") - timeout = float(config.get("timeout_seconds") or 10) + timeout = float(config.get("timeout_seconds") or 60) client = AuthentikClient(base_url=base_url, api_token=api_token, timeout=timeout) result = client.users(page=1, page_size=1) total = result.get("total", 0) if isinstance(result, dict) else 0 @@ -46,7 +46,7 @@ class AuthentikConfig(ServiceConfigBase): """Non-secret Authentik connection config.""" base_url: ServiceBaseUrl - timeout_seconds: int = 10 + timeout_seconds: int = 60 DEFINITION = ServiceDefinition( diff --git a/backend/src/media_library_viewer_api/integrations/jellyfin.py b/backend/src/media_library_viewer_api/integrations/jellyfin.py index 7b00058..a13d502 100644 --- a/backend/src/media_library_viewer_api/integrations/jellyfin.py +++ b/backend/src/media_library_viewer_api/integrations/jellyfin.py @@ -29,7 +29,7 @@ def test_connection( try: base_url = str(config.get("base_url") or "") api_key = str(secrets.get("api_key") or "") - timeout = int(config.get("timeout_seconds") or 10) + timeout = int(config.get("timeout_seconds") or 60) client = JellyfinClient(base_url, api_key, timeout=timeout) users = client.users() return TestResult(ok=True, detail="Connected to Jellyfin.", evidence=f"{len(users)} users") @@ -49,7 +49,7 @@ class JellyfinConfig(ServiceConfigBase): base_url: ServiceBaseUrl user_id: str = "" - timeout_seconds: int = 10 + timeout_seconds: int = 60 jellyseerr_url: str = "" jellyseerr_api_key: str = "" diff --git a/backend/src/media_library_viewer_api/integrations/nextcloud.py b/backend/src/media_library_viewer_api/integrations/nextcloud.py index 87731bd..e31864e 100644 --- a/backend/src/media_library_viewer_api/integrations/nextcloud.py +++ b/backend/src/media_library_viewer_api/integrations/nextcloud.py @@ -31,7 +31,7 @@ def test_connection( """GET {base_url}/status.php (unauthenticated server probe).""" try: base_url = str(config.get("base_url") or "").rstrip("/") - resp = requests.get(f"{base_url}/status.php", timeout=10) + resp = requests.get(f"{base_url}/status.php", timeout=(5.0, 60.0)) resp.raise_for_status() payload = resp.json() version = str(payload.get("version", "") or "connected") diff --git a/backend/src/media_library_viewer_api/integrations/prometheus.py b/backend/src/media_library_viewer_api/integrations/prometheus.py index dffdf53..e76c2a2 100644 --- a/backend/src/media_library_viewer_api/integrations/prometheus.py +++ b/backend/src/media_library_viewer_api/integrations/prometheus.py @@ -31,7 +31,7 @@ def test_connection( grafana_url = str(config.get("grafana_url") or "").rstrip("/") api_key = str(secrets.get("grafana_api_key") or "") datasource_uid = str(config.get("datasource_uid") or "prometheus") - timeout = int(config.get("timeout_seconds") or 10) + timeout = int(config.get("timeout_seconds") or 60) if not grafana_url: return TestResult(ok=False, detail="Grafana gateway URL is required.") if not api_key: @@ -73,7 +73,7 @@ class PrometheusConfig(ServiceConfigBase): grafana_url: ServiceBaseUrl datasource_uid: str = "prometheus" - timeout_seconds: int = 10 + timeout_seconds: int = 60 class PrometheusMetricWidgetConfig(WidgetConfigBase): diff --git a/backend/src/media_library_viewer_api/integrations/qbittorrent.py b/backend/src/media_library_viewer_api/integrations/qbittorrent.py index 82cb64d..99f93e7 100644 --- a/backend/src/media_library_viewer_api/integrations/qbittorrent.py +++ b/backend/src/media_library_viewer_api/integrations/qbittorrent.py @@ -35,7 +35,7 @@ def test_connection( base_url = str(config.get("base_url") or "") username = str(secrets.get("username") or "") password = str(secrets.get("password") or "") - timeout = int(config.get("timeout_seconds") or 10) + timeout = int(config.get("timeout_seconds") or 60) client = QbittorrentClient(base_url, username, password, timeout=timeout) data = client.maindata() version = str(data.get("server_state", {}).get("qbittorrent_version", "") or "connected") @@ -53,7 +53,7 @@ class QbittorrentConfig(ServiceConfigBase): """Non-secret qBittorrent connection config.""" base_url: ServiceBaseUrl - timeout_seconds: int = 10 + timeout_seconds: int = 60 class QbittorrentWidgetConfig(WidgetConfigBase): diff --git a/backend/src/media_library_viewer_api/routers/monitoring.py b/backend/src/media_library_viewer_api/routers/monitoring.py index 468c329..9ad960e 100644 --- a/backend/src/media_library_viewer_api/routers/monitoring.py +++ b/backend/src/media_library_viewer_api/routers/monitoring.py @@ -14,6 +14,7 @@ from typing import Any import requests from fastapi import APIRouter, Body, Depends +from media_library_viewer_api.clients.http_timeout import http_timeout from media_library_viewer_api.dependencies import get_settings_store from media_library_viewer_api.services.service_resolution import resolve_service_record from media_library_viewer_api.services.settings_store import SettingsStore @@ -27,8 +28,10 @@ def _base_url(service: ServiceRecord) -> str: return str(service.config.get("base_url") or "").rstrip("/") -def _timeout(service: ServiceRecord, default: int) -> int: - return int(service.config.get("timeout_seconds") or default) +def _timeout(service: ServiceRecord, default: int) -> tuple[float, float]: + """Return a (connect, read) timeout tuple from the service config.""" + read = int(service.config.get("timeout_seconds") or default) + return http_timeout(read) def _auth_headers(service: ServiceRecord) -> dict[str, str]: @@ -189,7 +192,7 @@ def get_prometheus_status( grafana_url = str(service.config.get("grafana_url") or "").rstrip("/") api_key = str(service.secrets.get("grafana_api_key") or "") datasource_uid = str(service.config.get("datasource_uid") or "prometheus") - timeout = int(service.config.get("timeout_seconds") or 10) + timeout = int(service.config.get("timeout_seconds") or 60) if not grafana_url or not api_key: return _status_response(service, error="gateway_not_configured") body = { @@ -211,7 +214,7 @@ def get_prometheus_status( f"{grafana_url}/api/ds/query", json=body, headers={"Authorization": f"Bearer {api_key}", "Content-Type": "application/json"}, - timeout=timeout, + timeout=http_timeout(timeout), ) resp.raise_for_status() except requests.HTTPError as exc: diff --git a/backend/src/media_library_viewer_api/widgets/sources.py b/backend/src/media_library_viewer_api/widgets/sources.py index 3a825c1..a26fb40 100644 --- a/backend/src/media_library_viewer_api/widgets/sources.py +++ b/backend/src/media_library_viewer_api/widgets/sources.py @@ -115,7 +115,7 @@ class MetricSource: grafana_url = str(service.config.get("grafana_url") or "").rstrip("/") api_key = str(service.secrets.get("grafana_api_key") or "") datasource_uid = str(service.config.get("datasource_uid") or "prometheus") - timeout = int(service.config.get("timeout_seconds") or 10) + timeout = int(service.config.get("timeout_seconds") or 60) if widget_kind == "chart": return await self._fetch_chart(grafana_url, api_key, datasource_uid, timeout, config) if widget_kind == "gauge": @@ -277,7 +277,7 @@ class AlertmanagerWidgetSource: if service is None: return {"error": "Alertmanager widget is missing its service"} base_url = str(service.config.get("base_url") or "").rstrip("/") - timeout = int(service.config.get("timeout_seconds") or 5) + timeout = int(service.config.get("timeout_seconds") or 60) severity_filter = config.get("severity_filter") or None headers: dict[str, str] = {} api_key = str(service.secrets.get("api_key") or "") @@ -316,7 +316,7 @@ class JellyfinWidgetSource: return {"error": "Jellyfin widget is missing its service"} base_url = str(service.config.get("base_url") or "") api_key = str(service.secrets.get("api_key") or "") - timeout = int(service.config.get("timeout_seconds") or 10) + timeout = int(service.config.get("timeout_seconds") or 60) client = await asyncio.wait_for( asyncio.to_thread(JellyfinClient, base_url, api_key, timeout), timeout=timeout, @@ -396,7 +396,7 @@ class QbittorrentWidgetSource: base_url = str(service.config.get("base_url") or "") username = str(service.secrets.get("username") or "") password = str(service.secrets.get("password") or "") - timeout = int(service.config.get("timeout_seconds") or 10) + timeout = int(service.config.get("timeout_seconds") or 60) if not base_url or not username or not password: return {"error": "qBittorrent service is missing base_url, username, or password"} diff --git a/backend/src/media_library_viewer_api/workers/media_index_worker.py b/backend/src/media_library_viewer_api/workers/media_index_worker.py index 3398d10..922437f 100644 --- a/backend/src/media_library_viewer_api/workers/media_index_worker.py +++ b/backend/src/media_library_viewer_api/workers/media_index_worker.py @@ -105,8 +105,13 @@ def _resolve_jellyfin(service_id: str) -> tuple[Any, str]: api_key = str(service.get("secrets", {}).get("api_key") or "") if not base_url or not api_key: raise RuntimeError("Jellyfin service is missing base_url or api_key. Edit it on the Services page.") - timeout = int(service.get("config", {}).get("timeout_seconds", 10)) - client = JellyfinClient(base_url, api_key, timeout) + timeout = int(service.get("config", {}).get("timeout_seconds", 60)) + # The build is a background operation and can afford a patient read timeout. + # The UI-grade config timeout_seconds (default 60) governs the widget paths; + # the worker uses a larger floor so slow /Items pages on large libraries + # don't ReadTimeout mid-build. + read_timeout = max(float(timeout), 180.0) + client = JellyfinClient(base_url, api_key, read_timeout) user_id = str(service.get("config", {}).get("user_id") or "") if not user_id: users = client.users() diff --git a/backend/tests/test_http_timeout.py b/backend/tests/test_http_timeout.py new file mode 100644 index 0000000..f10a1b8 --- /dev/null +++ b/backend/tests/test_http_timeout.py @@ -0,0 +1,39 @@ +"""Tests for the shared HTTP timeout helper.""" + +from __future__ import annotations + +from media_library_viewer_api.clients.http_timeout import ( + DEFAULT_CONNECT_TIMEOUT, + DEFAULT_READ_TIMEOUT, + http_timeout, +) + + +class TestHttpTimeout: + def test_http_timeout_returns_tuple(self) -> None: + result = http_timeout(30) + assert result == (DEFAULT_CONNECT_TIMEOUT, 30.0) + + def test_http_timeout_default_when_none(self) -> None: + result = http_timeout(None) + assert result == (DEFAULT_CONNECT_TIMEOUT, DEFAULT_READ_TIMEOUT) + + def test_http_timeout_default_when_zero(self) -> None: + result = http_timeout(0) + assert result == (DEFAULT_CONNECT_TIMEOUT, DEFAULT_READ_TIMEOUT) + + def test_http_timeout_custom_connect(self) -> None: + result = http_timeout(30, connect_timeout=10) + assert result == (10.0, 30.0) + + def test_http_timeout_default_when_negative(self) -> None: + result = http_timeout(-5) + assert result == (DEFAULT_CONNECT_TIMEOUT, DEFAULT_READ_TIMEOUT) + + def test_http_timeout_default_when_garbage_string(self) -> None: + result = http_timeout("garbage") # type: ignore[arg-type] + assert result == (DEFAULT_CONNECT_TIMEOUT, DEFAULT_READ_TIMEOUT) + + def test_http_timeout_accepts_int(self) -> None: + result = http_timeout(45) + assert result == (5.0, 45.0) diff --git a/backend/tests/test_qbittorrent_client.py b/backend/tests/test_qbittorrent_client.py index 0b22878..24995e3 100644 --- a/backend/tests/test_qbittorrent_client.py +++ b/backend/tests/test_qbittorrent_client.py @@ -129,6 +129,21 @@ class QbittorrentClientTests(unittest.TestCase): with self.assertRaises(requests.ConnectionError): client._login() + def test_qbittorrent_client_uses_tuple_timeout(self) -> None: + """The client passes a (connect, read) tuple to requests, not an int.""" + # self.client was constructed with timeout=5 in setUp() and has a mocked session. + assert isinstance(self.client.timeout, tuple) + assert len(self.client.timeout) == 2 + assert self.client.timeout[0] == 5.0 # connect timeout + assert self.client.timeout[1] == 5.0 # read timeout (what we passed) + + # Verify it's actually passed to requests as-is. + self.session.post.return_value = self._login_response() + self.client._login() + call_kwargs = self.session.post.call_args.kwargs + assert call_kwargs["timeout"] == (5.0, 5.0) + assert not isinstance(call_kwargs["timeout"], int) + if __name__ == "__main__": unittest.main()