diff --git a/backend/docker_client.py b/backend/docker_client.py index a0fc92a..d9cf19d 100644 --- a/backend/docker_client.py +++ b/backend/docker_client.py @@ -9,6 +9,7 @@ import functools from typing import Any import docker +import requests from docker.errors import APIError, DockerException, NotFound from config import settings @@ -23,19 +24,37 @@ class DockerError(Exception): super().__init__(f"{error}: {detail}" if detail else error) -@functools.lru_cache(maxsize=1) -def get_client() -> docker.DockerClient: +# Read timeout for get_long_client(). docker-py's default of 60s is fine for +# interactive calls, but a prune that deletes gigabytes of volume data or image +# layers routinely takes longer. Kept below nginx's proxy_read_timeout (600s) +# so the caller still gets our error instead of a bare gateway timeout. +LONG_TIMEOUT_SECONDS = 540 + + +def _connect(**kwargs) -> docker.DockerClient: try: base_url = f"unix://{settings.DOCKER_SOCKET}" - client = docker.DockerClient(base_url=base_url) + client = docker.DockerClient(base_url=base_url, **kwargs) client.ping() return client except DockerException as exc: # pragma: no cover - environment dependent raise DockerError("docker_unavailable", str(exc)) from exc +@functools.lru_cache(maxsize=1) +def get_client() -> docker.DockerClient: + return _connect() + + +@functools.lru_cache(maxsize=1) +def get_long_client() -> docker.DockerClient: + """A client for calls that can legitimately run for minutes (prunes).""" + return _connect(timeout=LONG_TIMEOUT_SECONDS) + + def reset_client() -> None: get_client.cache_clear() + get_long_client.cache_clear() def safe_call(fn, *args, **kwargs) -> Any: @@ -48,3 +67,12 @@ def safe_call(fn, *args, **kwargs) -> Any: raise DockerError("docker_api_error", str(exc)) from exc except DockerException as exc: raise DockerError("docker_error", str(exc)) from exc + # docker-py lets transport errors from requests through unwrapped; without + # these a slow daemon surfaced as a bare 500 with no message. + except requests.exceptions.Timeout as exc: + raise DockerError( + "docker_timeout", + "Docker did not answer in time; the operation may still be running on the host", + ) from exc + except requests.exceptions.RequestException as exc: + raise DockerError("docker_unavailable", str(exc)) from exc diff --git a/backend/services/image_service.py b/backend/services/image_service.py index ee91d5b..4a18c38 100644 --- a/backend/services/image_service.py +++ b/backend/services/image_service.py @@ -1,7 +1,7 @@ """Image listing for the images router.""" from __future__ import annotations -from docker_client import DockerError, get_client, safe_call +from docker_client import DockerError, get_client, get_long_client, safe_call from services import update_service COMPOSE_PROJECT_LABEL = "com.docker.compose.project" @@ -51,7 +51,7 @@ def prune_images(all_unused: bool = False) -> dict: """Remove unused images. By default only dangling (untagged) images are removed; ``all_unused=True`` removes every image not referenced by a container (``docker image prune -a``).""" - client = get_client() + client = get_long_client() # dangling=false tells the engine to also consider tagged-but-unused images. filters = {"dangling": False} if all_unused else {"dangling": True} result = safe_call(client.images.prune, filters=filters) diff --git a/backend/services/network_service.py b/backend/services/network_service.py index 0fb8ab8..da17ffc 100644 --- a/backend/services/network_service.py +++ b/backend/services/network_service.py @@ -6,7 +6,7 @@ from typing import Optional from docker.errors import APIError from docker.types import IPAMConfig, IPAMPool -from docker_client import DockerError, get_client, safe_call +from docker_client import DockerError, get_client, get_long_client, safe_call COMPOSE_PROJECT_LABEL = "com.docker.compose.project" DEFAULT_NETWORKS = {"bridge", "host", "none"} @@ -133,5 +133,5 @@ def delete_network(network_id: str) -> None: def prune_networks() -> dict: - client = get_client() + client = get_long_client() return safe_call(client.networks.prune) diff --git a/backend/services/volume_service.py b/backend/services/volume_service.py index 99b71e7..05ce48b 100644 --- a/backend/services/volume_service.py +++ b/backend/services/volume_service.py @@ -7,7 +7,7 @@ from typing import Literal, Optional import yaml -from docker_client import get_client, safe_call +from docker_client import get_client, get_long_client, safe_call COMPOSE_PROJECT_LABEL = "com.docker.compose.project" @@ -86,7 +86,7 @@ def remove_volume(name: str, force: bool = False) -> None: def prune_volumes() -> dict: - client = get_client() + client = get_long_client() return safe_call(client.volumes.prune) diff --git a/backend/tests/test_docker_timeouts.py b/backend/tests/test_docker_timeouts.py new file mode 100644 index 0000000..7f2c169 --- /dev/null +++ b/backend/tests/test_docker_timeouts.py @@ -0,0 +1,81 @@ +"""Slow Docker calls must fail with a message, not a bare 500. + +A volume or image prune that deletes a lot of data easily runs past docker-py's +60s read timeout. requests' ``ReadTimeout`` is not a ``DockerException``, so it +slipped past ``safe_call`` and the UI only showed "Request failed with status +code 500". Prunes now use a client with a long timeout, and any transport +timeout that still happens is turned into a ``DockerError`` (-> 502 + detail). +""" +from __future__ import annotations + +import pytest +import requests + + +def test_safe_call_wraps_read_timeout(): + from docker_client import DockerError, safe_call + + def slow(): + raise requests.exceptions.ReadTimeout("Read timed out. (read timeout=60)") + + with pytest.raises(DockerError) as info: + safe_call(slow) + assert info.value.error == "docker_timeout" + + +def test_safe_call_wraps_connection_error(): + from docker_client import DockerError, safe_call + + def gone(): + raise requests.exceptions.ConnectionError("socket went away") + + with pytest.raises(DockerError) as info: + safe_call(gone) + assert info.value.error == "docker_unavailable" + + +def test_volume_prune_timeout_is_a_502_with_detail(as_admin, monkeypatch): + from services import volume_service + + class Volumes: + def prune(self, filters=None): + raise requests.exceptions.ReadTimeout("Read timed out.") + + class Client: + volumes = Volumes() + + monkeypatch.setattr(volume_service, "get_long_client", lambda: Client()) + r = as_admin.post("/api/volumes/prune") + assert r.status_code == 502 + assert r.json()["error"] == "docker_timeout" + + +@pytest.mark.parametrize( + "module, fn, args", + [ + ("volume_service", "prune_volumes", ()), + ("image_service", "prune_images", (True,)), + ("network_service", "prune_networks", ()), + ], +) +def test_prunes_use_the_long_timeout_client(monkeypatch, module, fn, args): + import importlib + + svc = importlib.import_module(f"services.{module}") + used = [] + + class Any_: + def prune(self, filters=None): + used.append("long") + return {} + + class Client: + volumes = images = networks = Any_() + + def short(): + raise AssertionError("prune must not use the 60s client") + + monkeypatch.setattr(svc, "get_client", short) + monkeypatch.setattr(svc, "get_long_client", lambda: Client()) + getattr(svc, fn)(*args) + assert used == ["long"] diff --git a/backend/version.py b/backend/version.py index 6a975c7..51a2673 100644 --- a/backend/version.py +++ b/backend/version.py @@ -1,3 +1,3 @@ """Single source of truth for the StackPilot release version.""" -APP_VERSION = "0.60.0" +APP_VERSION = "0.60.1" diff --git a/frontend/package.json b/frontend/package.json index 727543e..4baa8d7 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "stackpilot-frontend", "private": true, - "version": "0.60.0", + "version": "0.60.1", "type": "module", "scripts": { "dev": "vite",