refactor(agentkit): engineering-standards uplift (logging / exceptions / timeouts)

agentkit/apps/simple_app/simple_app.py

@@ -52,9 +52,10 @@ def entrypoint(self, func: Callable) -> Callable:
         return func
 
     def ping(self, func: Callable) -> Callable:
-        assert len(list(inspect.signature(func).parameters.keys())) == 0, (
-            f"Health check function `{func.__name__}` should not receive any arguments."
-        )
+        if len(inspect.signature(func).parameters) != 0:
+            raise TypeError(
+                f"Health check function `{func.__name__}` should not receive any arguments."
+            )
 
         self.ping_handler.func = func
         return func

agentkit/auth/_redact.py

@@ -12,44 +12,14 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Secret redaction for log/error output.
+"""Backward-compatible shim.
 
-Auth flows shuttle tokens and STS secrets around; any of them can end up in an
-exception body or a debug print. :func:`redact` scrubs the high-entropy strings
-(JWTs, bearer/STS tokens, ``ak/sk`` style secrets) before they are surfaced.
+The redaction helpers now live in :mod:`agentkit.utils.redact`; this module
+re-exports them so existing importers keep working.
 """
 
 from __future__ import annotations
 
-import re
+from agentkit.utils.redact import mask, redact
 
-# JWTs: three base64url segments separated by dots.
-_JWT = re.compile(r"\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\b")
-# Volcengine STS / access tokens and long opaque secrets (>= 16 url-safe chars).
-_OPAQUE = re.compile(r"\b[A-Za-z0-9/+_-]{16,}={0,2}\b")
-# Explicit secret-bearing query/JSON/header fields.
-_FIELD = re.compile(
-    r"(?i)(\"?(?:access_token|refresh_token|id_token|client_secret|secret_access_key"
-    r"|secretkey|accesskeyid|accesskey|sessiontoken|session_token|authorization"
-    r"|apikey|api_key|token|password)\"?\s*[:=]\s*\"?(?:bearer\s+)?)"
-    r"([^\"&\s,}]+)"
-)
-
-
-def redact(text: str) -> str:
-    """Return ``text`` with credential-looking substrings replaced by ``***``."""
-    if not text:
-        return text
-    text = _FIELD.sub(lambda m: m.group(1) + "***", text)
-    text = _JWT.sub("***", text)
-    text = _OPAQUE.sub("***", text)
-    return text
-
-
-def mask(secret: str | None, *, keep: int = 4) -> str:
-    """Mask a secret, keeping only the last ``keep`` characters for recognition."""
-    if not secret:
-        return "<none>"
-    if len(secret) <= keep:
-        return "*" * len(secret)
-    return "*" * (len(secret) - keep) + secret[-keep:]
+__all__ = ["redact", "mask"]

agentkit/auth/errors.py

@@ -16,8 +16,10 @@
 
 from __future__ import annotations
 
+from agentkit.errors import AgentKitError
 
-class AuthError(Exception):
+
+class AuthError(AgentKitError):
     """Base class for all authentication failures.
 
     Carries an optional ``hint`` with an actionable next step that CLIs can

agentkit/auth/session.py

@@ -207,7 +207,8 @@ def credentials(self, *, force_refresh: bool = False) -> StsCredentials:
                             self._refresh_token = sibling._refresh_token or self._refresh_token
                             return self._sts
                 self._renew_locked()
-            assert self._sts is not None
+            if self._sts is None:
+                raise AuthError("internal: STS credentials not populated after renew")
             return self._sts
 
     # -- data-plane JWT (id_token) --------------------------------------------
@@ -238,7 +239,8 @@ def valid_id_token(self, *, skew_seconds: int = 60, force_refresh: bool = False)
                         self._access_token = sibling._access_token or self._access_token
                         return self._id_token
                 self._refresh_id_token_locked()
-            assert self._id_token is not None
+            if self._id_token is None:
+                raise AuthError("internal: id_token not populated after renew")
             return self._id_token
 
     def _refresh_id_token_locked(self) -> None:

agentkit/client/base_agentkit_client.py

@@ -70,8 +70,13 @@ def __init__(
 
     def _get(self, api_action: str, params: Dict[str, Any] = None) -> str:
         """Legacy method for GET requests."""
+        # Imported lazily: ``agentkit.toolkit`` pulls in ``sdk`` which imports
+        # back into ``agentkit.client``, so a module-level import would cycle
+        # (mirrors ``BaseServiceClient._invoke_api``).
+        from agentkit.toolkit.errors import ApiError
+
         try:
             resp = self.get(api_action, params)
             return resp
         except Exception as e:
-            raise Exception(f"Failed to {api_action}: {str(e)}")
+            raise ApiError(f"Failed to {api_action}: {e}") from e

agentkit/client/base_service_client.py

@@ -18,26 +18,31 @@
 """
 
 import json
-import os
 from typing import Any, Dict, Type, TypeVar, Union, Optional
 from dataclasses import dataclass
 
+import requests
 from requests.adapters import HTTPAdapter
 from urllib3.util.retry import Retry
 from volcengine.ApiInfo import ApiInfo
 from volcengine.base.Service import Service
 from volcengine.Credentials import Credentials as VolcCredentials
 from volcengine.ServiceInfo import ServiceInfo
 
+from agentkit.auth.errors import NetworkError
 from agentkit.platform import (
     VolcConfiguration,
     resolve_credentials,
     resolve_endpoint,
 )
 from agentkit.platform.configuration import Credentials as PlatformCredentials
 from agentkit.platform.provider import CloudProvider
+from agentkit.utils.http_defaults import http_retries, http_timeout
+from agentkit.utils.logging_config import get_logger
 from agentkit.utils.ve_sign import ensure_x_custom_source_header
 
+logger = get_logger(__name__)
+
 T = TypeVar("T")
 _CREDENTIAL_ERROR_TOKENS = frozenset(
     {
@@ -161,8 +166,8 @@ def __init__(
                 region=self.region,
                 session_token=self.session_token or "",
             ),
-            connection_timeout=30,
-            socket_timeout=30,
+            connection_timeout=int(http_timeout()),
+            socket_timeout=int(http_timeout()),
             scheme=self.scheme,
         )
 
@@ -188,10 +193,7 @@ def _install_retry_adapter(self) -> None:
         timeouts are not retried (``read=0``) and only 429/503 are status-
         retried. Honors ``Retry-After``. Disabled with AGENTKIT_HTTP_RETRIES=0.
         """
-        try:
-            retries = max(0, int(os.getenv("AGENTKIT_HTTP_RETRIES", "2")))
-        except ValueError:
-            retries = 2
+        retries = http_retries()
         if retries <= 0:
             return
         session = getattr(self, "session", None)
@@ -251,7 +253,14 @@ def _refresh_credentials_if_needed(self, *, force: bool = False) -> bool:
         creds = self._platform_config.get_vefaas_iam_credentials(force=force)
         if not creds:
             return False
-        return self._apply_credentials(creds)
+        applied = self._apply_credentials(creds)
+        if applied:
+            logger.info(
+                "Refreshed vefaas credentials (force=%s) for service %s",
+                force,
+                self.service,
+            )
+        return applied
 
     def _is_probable_credential_error_code(self, code: str) -> bool:
         c = (code or "").lower()
@@ -319,12 +328,18 @@ def _invoke_api(
             Typed response object
 
         Raises:
-            Exception: If API call fails or returns an error
+            ApiError: If the API call fails or returns an error response.
+            NetworkError: If a transport-level failure occurs.
         """
+        # Imported lazily: ``agentkit.toolkit`` pulls in ``sdk`` which imports
+        # back into ``agentkit.client``, so a module-level import would cycle.
+        from agentkit.toolkit.errors import ApiError
+
         self._refresh_credentials_if_needed()
         last_error: Optional[BaseException] = None
 
         for attempt in (0, 1):
+            logger.debug("Invoking API %s (attempt %d)", api_action, attempt)
             if attempt == 1:
                 self._refresh_credentials_if_needed(force=True)
 
@@ -336,16 +351,34 @@ def _invoke_api(
                         request.model_dump(by_alias=True, exclude_none=True)
                     ),
                 )
+            except (
+                requests.exceptions.ConnectionError,
+                requests.exceptions.Timeout,
+                requests.exceptions.HTTPError,
+                requests.exceptions.RequestException,
+            ) as e:
+                # Transport-level failure: not a credential issue, surface as
+                # a typed network error so callers can distinguish it.
+                raise NetworkError(f"Failed to {api_action}: network error") from e
             except Exception as e:
                 last_error = e
                 if attempt == 0 and self._is_probable_credential_error_text(str(e)):
+                    logger.debug(
+                        "Retrying %s after probable credential error", api_action
+                    )
                     continue
-                raise Exception(f"Failed to {api_action}: {str(e)}") from e
+                raise ApiError(f"Failed to {api_action}: {str(e)}") from e
 
             if not res:
-                raise Exception(f"Empty response from {api_action} request.")
+                raise ApiError(f"Empty response from {api_action} request.")
+
+            try:
+                response_data = json.loads(res)
+            except (ValueError, TypeError) as e:
+                raise ApiError(
+                    f"Failed to {api_action}: malformed response body"
+                ) from e
 
-            response_data = json.loads(res)
             metadata = response_data.get("ResponseMetadata", {})
             if metadata.get("Error"):
                 err = metadata.get("Error", {}) or {}
@@ -355,13 +388,21 @@ def _invoke_api(
                     self._is_probable_credential_error_code(error_code)
                     or self._is_probable_credential_error_text(error_msg)
                 ):
+                    logger.debug(
+                        "Retrying %s after credential error code %s",
+                        api_action,
+                        error_code or "<none>",
+                    )
                     continue
-                raise Exception(f"Failed to {api_action}: {error_msg}")
+                raise ApiError(
+                    f"Failed to {api_action}: {error_msg}",
+                    error_code=error_code or None,
+                )
 
             return response_type(**response_data.get("Result", {}))
 
         if last_error is not None:
-            raise Exception(
+            raise ApiError(
                 f"Failed to {api_action}: {str(last_error)}"
             ) from last_error
-        raise Exception(f"Failed to {api_action}: Unknown error")
+        raise ApiError(f"Failed to {api_action}: Unknown error")

agentkit/errors.py

@@ -0,0 +1,32 @@
+# Copyright (c) 2025 Beijing Volcano Engine Technology Co., Ltd. and/or its affiliates.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Top-level exception root for AgentKit.
+
+This module is a stdlib-only leaf: it has no intra-package imports so that any
+subpackage (``agentkit.auth``, ``agentkit.toolkit``, ...) can subclass the
+single root without creating dependency cycles.
+"""
+
+
+class AgentKitError(Exception):
+    """The single root of the AgentKit exception hierarchy.
+
+    Every domain-specific AgentKit exception (auth, toolkit, API, ...) ultimately
+    inherits from this class so callers can ``except AgentKitError`` to catch any
+    AgentKit-originated failure.
+    """
+
+
+__all__ = ["AgentKitError"]

agentkit/platform/configuration.py

@@ -15,7 +15,6 @@
 from __future__ import annotations
 
 import json
-import logging
 import os
 import threading
 import time
@@ -42,8 +41,9 @@
     get_global_config_value,
     read_global_config_dict,
 )
+from agentkit.utils.logging_config import get_logger
 
-logger = logging.getLogger(__name__)
+logger = get_logger(__name__)
 
 VEFAAS_IAM_CREDENTIAL_PATH = "/var/run/secrets/iam/credential"
 VEFAAS_IAM_CREDENTIAL_FALLBACK_TTL_SECONDS = 60

agentkit/sdk/account/client.py

@@ -66,9 +66,7 @@ def get_service_status(self, service_name: str) -> Optional[str]:
         Returns:
             The status string ('Enabled' or 'Disabled'), or None if service not found.
         """
-        print(1)
         response = self.list_account_linked_services(ListAccountLinkedServicesRequest())
-        print(2)
         if not response.service_statuses:
             return None
         for svc in response.service_statuses:

agentkit/sdk/identity/auth.py

@@ -21,6 +21,7 @@
     resolve_credentials,
     resolve_endpoint,
 )
+from agentkit.toolkit.errors import ApiError
 
 
 def requires_api_key(*, provider_name: str, into: str = "api_key"):
@@ -59,8 +60,8 @@ def _get_api_key() -> str:
 
             try:
                 return response["Result"]["ApiKey"]
-            except Exception as _:
-                raise RuntimeError(f"Get api key failed: {response}")
+            except Exception as e:
+                raise ApiError("GetResourceApiKey did not return an ApiKey") from e
 
         @wraps(func)
         async def async_wrapper(*args: Any, **kwargs: Any) -> Any:

agentkit/toolkit/cli/sandbox/cli_mount.py

@@ -27,6 +27,7 @@
 import typer
 
 from agentkit.sdk.tools.client import AgentkitToolsClient
+from agentkit.utils.http_defaults import http_timeout
 from agentkit.sdk.tools import types as tools_types
 from agentkit.toolkit.cli.sandbox.session_create import SANDBOX_TOOL_ID_ENV
 from agentkit.toolkit.cli.sandbox.session_sync import sync_remote_sessions
@@ -212,7 +213,7 @@ def _download_discovery(oauth_url: str) -> dict[str, object]:
     path = _get_discovery_store_path()
     path.parent.mkdir(parents=True, exist_ok=True)
 
-    with urlopen(discovery_url) as response:
+    with urlopen(discovery_url, timeout=http_timeout()) as response:
         content = response.read()
     path.write_bytes(content)