fix: Improve PodmanConnectionError for all methods

containers · Feb 19, 2025 · 93dd525 · 93dd525
1 parent 5a80693
commit 93dd525
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 108 deletions.
diff --git a/podman/client.py b/podman/client.py
@@ -6,7 +6,7 @@
 from pathlib import Path
 from typing import Any, Optional
 
-from podman.errors import PodmanConnectionError
+from podman.errors.exceptions import APIError, PodmanConnectionError
 from podman.api import cached_property
 from podman.api.client import APIClient
 from podman.api.path_utils import get_runtime_dir
@@ -61,37 +61,42 @@ def __init__(self, **kwargs) -> None:
         super().__init__()
         config = PodmanConfig()
 
-        self.api_kwargs = kwargs.copy()
+        api_kwargs = kwargs.copy()
 
-        if "connection" in self.api_kwargs:
-            connection = config.services[self.api_kwargs.get("connection")]
-            self.api_kwargs["base_url"] = connection.url.geturl()
+        if "connection" in api_kwargs:
+            connection = config.services[api_kwargs.get("connection")]
+            api_kwargs["base_url"] = connection.url.geturl()
 
             # Override configured identity, if provided in arguments
-            self.api_kwargs["identity"] = kwargs.get("identity", str(connection.identity))
-        elif "base_url" not in self.api_kwargs:
+            api_kwargs["identity"] = kwargs.get("identity", str(connection.identity))
+        elif "base_url" not in api_kwargs:
             path = str(Path(get_runtime_dir()) / "podman" / "podman.sock")
-            self.api_kwargs["base_url"] = "http+unix://" + path
+            api_kwargs["base_url"] = "http+unix://" + path
+        self.api = APIClient(**api_kwargs)
 
-        try:
-            self.api = APIClient(**self.api_kwargs)
-            response = self.api.get("_ping")
+        self._verify_connection()
 
-            if response.status_code != 200:
+    def _verify_connection(self):
+        """Verify connection to Podman daemon during initialization.
+        
+        Raises:
+            PodmanException: If unable to connect to Podman daemon
+        """
+        try:
+            # Attempt to get version info to verify connection
+            self.version()
+        except APIError as e:
+            if "No such file or directory" in str(e):
                 raise PodmanConnectionError(
-                    message=f"Unexpected response from Podman service: {response.status_code}",
-                    environment=os.environ,
-                    host=self.api_kwargs.get("base_url"),
-                    original_error=None,
-                )
-        except PodmanConnectionError:
-            raise
+                    "Error while connecting to Podman daemon: "
+                    f"Could not find socket file - {str(e)}"
+                ) from e
+            raise PodmanConnectionError(
+                f"Error while connecting to Podman daemon: {str(e)}"
+            ) from e
         except Exception as e:
             raise PodmanConnectionError(
-                message=f"Failed to connect to Podman service: {str(e)}",
-                environment=os.environ,
-                host=self.api_kwargs.get("base_url"),
-                original_error=e,
+                f"Error while connecting to Podman daemon: {str(e)}"
             ) from e
 
     def __enter__(self) -> "PodmanClient":
@@ -100,6 +105,18 @@ def __enter__(self) -> "PodmanClient":
     def __exit__(self, exc_type, exc_value, traceback) -> None:
         self.close()
 
+    def __enter__(self) -> "PodmanClient":
+        return self
+
+    def __exit__(self, exc_type, exc_value, traceback) -> None:
+        self.close()
+
+    def __enter__(self) -> "PodmanClient":
+        return self
+
+    def __exit__(self, exc_type, exc_value, traceback) -> None:
+        self.close()
+
     @classmethod
     def from_env(
         cls,
@@ -137,41 +154,26 @@ def from_env(
         Raises:
             ValueError when required environment variable is not set
         """
-        try:
-            environment = environment or os.environ
-            credstore_env = credstore_env or {}
-
-            if version == "auto":
-                version = None
-
-            kwargs = {
-                "version": version,
-                "timeout": timeout,
-                "tls": False,
-                "credstore_env": credstore_env,
-                "max_pool_size": max_pool_size,
-            }
-
-            host = environment.get("CONTAINER_HOST") or environment.get("DOCKER_HOST") or None
-            if host is not None:
-                kwargs["base_url"] = host
-
-            return PodmanClient(**kwargs)
-        except ValueError as e:
-            error_msg = "Invalid environment configuration for Podman client"
-            raise PodmanConnectionError(
-                message=error_msg, environment=environment, host=host, original_error=e
-            )
-        except (ConnectionError, TimeoutError) as e:
-            error_msg = "Failed to connect to Podman service"
-            raise PodmanConnectionError(
-                message=error_msg, environment=environment, host=host, original_error=e
-            )
-        except Exception as e:
-            error_msg = "Failed to initialize Podman client from environment"
-            raise PodmanConnectionError(
-                message=error_msg, environment=environment, host=host, original_error=e
-            )
+        environment = environment or os.environ
+        credstore_env = credstore_env or {}
+
+        if version == "auto":
+            version = None
+
+        kwargs = {
+            'version': version,
+            'timeout': timeout,
+            'tls': False,
+            'credstore_env': credstore_env,
+            'max_pool_size': max_pool_size,
+        }
+
+        host = environment.get("CONTAINER_HOST") or environment.get("DOCKER_HOST") or None
+        if host is not None:
+            kwargs['base_url'] = host
+
+        return PodmanClient(**kwargs)
+
 
     @cached_property
     def containers(self) -> ContainersManager:

diff --git a/podman/errors/exceptions.py b/podman/errors/exceptions.py
@@ -148,56 +148,6 @@ class InvalidArgument(PodmanError):
 class PodmanConnectionError(PodmanError):
     """Exception raised when connection to Podman service fails using environment configuration."""
 
-    def __init__(
-        self,
-        message: str,
-        environment: Optional[dict[str, str]] = None,
-        host: Optional[str] = None,
-        original_error: Optional[Exception] = None,
-    ):
-        """Initialize PodmanConnectionError.
-        Args:
-            message: Description of the error
-            environment: Environment variables used in connection attempt
-            host: URL to Podman service that failed
-            original_error: Original exception that caused this error
-        """
-        super().__init__(message)
-        self.environment = environment
-        self.host = host
-        self.original_error = original_error
-
-    def __str__(self) -> str:
-        """Format error message with details about connection attempt."""
-        msg = [super().__str__()]
-
-        if self.host:
-            msg.append(f"Host: {self.host}")
-
-        if self.environment:
-            relevant_vars = {
-                k: v
-                for k, v in self.environment.items()
-                if k
-                in (
-                    'DOCKER_HOST',
-                    'CONTAINER_HOST',
-                    'DOCKER_TLS_VERIFY',
-                    'CONTAINER_TLS_VERIFY',
-                    'DOCKER_CERT_PATH',
-                    'CONTAINER_CERT_PATH',
-                )
-            }
-            if relevant_vars:
-                msg.append("Environment:")
-                for key, value in relevant_vars.items():
-                    msg.append(f"  {key}={value}")
-
-        if self.original_error:
-            msg.append(f"Caused by: {str(self.original_error)}")
-
-        return " | ".join(msg)
-
 
 class StreamParseError(RuntimeError):
     def __init__(self, reason):