fix: split HTTP connect/read timeouts (Jellyfin build + qBit stats)
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+.
This commit is contained in:
@@ -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(
|
||||
{
|
||||
|
||||
@@ -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)
|
||||
@@ -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(
|
||||
{
|
||||
|
||||
@@ -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(
|
||||
{
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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 = ""
|
||||
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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"}
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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)
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user