Skip to content

Commit fedc9c8

Browse files
tianttclaude
andcommitted
refactor(agentkit): raise core HTTP/credential/SDK paths to a logging/exception/timeout standard
Establishes one standard for each of three engineering dimensions on the inner-loop HTTP/credential/SDK layer, and applies it. Good building blocks already existed but were unused (get_logger had 0 callers; redact()/mask() were auth-only). Foundations (new, dependency-light leaves): - agentkit/errors.py: single stdlib-only root AgentKitError. toolkit.errors .AgentKitError and auth.errors.AuthError both reparent onto it, so a caller can `except agentkit.errors.AgentKitError` to catch any AgentKit failure while agentkit.auth stays free of any agentkit.toolkit import. - agentkit/utils/http_defaults.py: single source of truth for timeout/retry defaults + env vars (AGENTKIT_HTTP_TIMEOUT/RETRIES/STREAM_TIMEOUT), clamped. - agentkit/utils/redact.py: redact()/mask() moved here; auth/_redact.py is now a re-export shim. RedactionFilter installed on every log handler. Logging: swap logging.getLogger()/idioms for get_logger(__name__) in the scoped core modules; library stays silent-by-default. Exceptions: wrap transport errors into NetworkError at the HTTP-owning layer; typed ApiError (with optional error_code) for backend failures; always chain with `from e`; `raise e` -> bare `raise`; runtime-validation asserts -> raises. Secret-leak fixes: drop raw response/secret interpolation from identity/auth (API key), utils/request, cr.py debug log, ve_sign check_error. request() now re-raises instead of silently returning None on failure. Timeouts: base_service_client reads http_timeout()/http_retries() instead of hardcoded 30/30; cli_mount urlopen bounded via http_timeout(). Tests: +22 offline tests (hierarchy, http_defaults clamping, RedactionFilter, _signed_request/_invoke_api error paths). Full suite: only the 1 pre-existing failure on main (test_cli_add_harness env leak) remains; 0 new failures. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 695c92d commit fedc9c8

23 files changed

Lines changed: 737 additions & 107 deletions

agentkit/apps/simple_app/simple_app.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,10 @@ def entrypoint(self, func: Callable) -> Callable:
5252
return func
5353

5454
def ping(self, func: Callable) -> Callable:
55-
assert len(list(inspect.signature(func).parameters.keys())) == 0, (
56-
f"Health check function `{func.__name__}` should not receive any arguments."
57-
)
55+
if len(inspect.signature(func).parameters) != 0:
56+
raise TypeError(
57+
f"Health check function `{func.__name__}` should not receive any arguments."
58+
)
5859

5960
self.ping_handler.func = func
6061
return func

agentkit/auth/_redact.py

Lines changed: 5 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -12,44 +12,14 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
"""Secret redaction for log/error output.
15+
"""Backward-compatible shim.
1616
17-
Auth flows shuttle tokens and STS secrets around; any of them can end up in an
18-
exception body or a debug print. :func:`redact` scrubs the high-entropy strings
19-
(JWTs, bearer/STS tokens, ``ak/sk`` style secrets) before they are surfaced.
17+
The redaction helpers now live in :mod:`agentkit.utils.redact`; this module
18+
re-exports them so existing importers keep working.
2019
"""
2120

2221
from __future__ import annotations
2322

24-
import re
23+
from agentkit.utils.redact import mask, redact
2524

26-
# JWTs: three base64url segments separated by dots.
27-
_JWT = re.compile(r"\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\b")
28-
# Volcengine STS / access tokens and long opaque secrets (>= 16 url-safe chars).
29-
_OPAQUE = re.compile(r"\b[A-Za-z0-9/+_-]{16,}={0,2}\b")
30-
# Explicit secret-bearing query/JSON/header fields.
31-
_FIELD = re.compile(
32-
r"(?i)(\"?(?:access_token|refresh_token|id_token|client_secret|secret_access_key"
33-
r"|secretkey|accesskeyid|accesskey|sessiontoken|session_token|authorization"
34-
r"|apikey|api_key|token|password)\"?\s*[:=]\s*\"?(?:bearer\s+)?)"
35-
r"([^\"&\s,}]+)"
36-
)
37-
38-
39-
def redact(text: str) -> str:
40-
"""Return ``text`` with credential-looking substrings replaced by ``***``."""
41-
if not text:
42-
return text
43-
text = _FIELD.sub(lambda m: m.group(1) + "***", text)
44-
text = _JWT.sub("***", text)
45-
text = _OPAQUE.sub("***", text)
46-
return text
47-
48-
49-
def mask(secret: str | None, *, keep: int = 4) -> str:
50-
"""Mask a secret, keeping only the last ``keep`` characters for recognition."""
51-
if not secret:
52-
return "<none>"
53-
if len(secret) <= keep:
54-
return "*" * len(secret)
55-
return "*" * (len(secret) - keep) + secret[-keep:]
25+
__all__ = ["redact", "mask"]

agentkit/auth/errors.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@
1616

1717
from __future__ import annotations
1818

19+
from agentkit.errors import AgentKitError
1920

20-
class AuthError(Exception):
21+
22+
class AuthError(AgentKitError):
2123
"""Base class for all authentication failures.
2224
2325
Carries an optional ``hint`` with an actionable next step that CLIs can

agentkit/auth/session.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,8 @@ def credentials(self, *, force_refresh: bool = False) -> StsCredentials:
207207
self._refresh_token = sibling._refresh_token or self._refresh_token
208208
return self._sts
209209
self._renew_locked()
210-
assert self._sts is not None
210+
if self._sts is None:
211+
raise AuthError("internal: STS credentials not populated after renew")
211212
return self._sts
212213

213214
# -- data-plane JWT (id_token) --------------------------------------------
@@ -238,7 +239,8 @@ def valid_id_token(self, *, skew_seconds: int = 60, force_refresh: bool = False)
238239
self._access_token = sibling._access_token or self._access_token
239240
return self._id_token
240241
self._refresh_id_token_locked()
241-
assert self._id_token is not None
242+
if self._id_token is None:
243+
raise AuthError("internal: id_token not populated after renew")
242244
return self._id_token
243245

244246
def _refresh_id_token_locked(self) -> None:

agentkit/client/base_agentkit_client.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,13 @@ def __init__(
7070

7171
def _get(self, api_action: str, params: Dict[str, Any] = None) -> str:
7272
"""Legacy method for GET requests."""
73+
# Imported lazily: ``agentkit.toolkit`` pulls in ``sdk`` which imports
74+
# back into ``agentkit.client``, so a module-level import would cycle
75+
# (mirrors ``BaseServiceClient._invoke_api``).
76+
from agentkit.toolkit.errors import ApiError
77+
7378
try:
7479
resp = self.get(api_action, params)
7580
return resp
7681
except Exception as e:
77-
raise Exception(f"Failed to {api_action}: {str(e)}")
82+
raise ApiError(f"Failed to {api_action}: {e}") from e

agentkit/client/base_service_client.py

Lines changed: 56 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,26 +18,31 @@
1818
"""
1919

2020
import json
21-
import os
2221
from typing import Any, Dict, Type, TypeVar, Union, Optional
2322
from dataclasses import dataclass
2423

24+
import requests
2525
from requests.adapters import HTTPAdapter
2626
from urllib3.util.retry import Retry
2727
from volcengine.ApiInfo import ApiInfo
2828
from volcengine.base.Service import Service
2929
from volcengine.Credentials import Credentials as VolcCredentials
3030
from volcengine.ServiceInfo import ServiceInfo
3131

32+
from agentkit.auth.errors import NetworkError
3233
from agentkit.platform import (
3334
VolcConfiguration,
3435
resolve_credentials,
3536
resolve_endpoint,
3637
)
3738
from agentkit.platform.configuration import Credentials as PlatformCredentials
3839
from agentkit.platform.provider import CloudProvider
40+
from agentkit.utils.http_defaults import http_retries, http_timeout
41+
from agentkit.utils.logging_config import get_logger
3942
from agentkit.utils.ve_sign import ensure_x_custom_source_header
4043

44+
logger = get_logger(__name__)
45+
4146
T = TypeVar("T")
4247
_CREDENTIAL_ERROR_TOKENS = frozenset(
4348
{
@@ -161,8 +166,8 @@ def __init__(
161166
region=self.region,
162167
session_token=self.session_token or "",
163168
),
164-
connection_timeout=30,
165-
socket_timeout=30,
169+
connection_timeout=int(http_timeout()),
170+
socket_timeout=int(http_timeout()),
166171
scheme=self.scheme,
167172
)
168173

@@ -188,10 +193,7 @@ def _install_retry_adapter(self) -> None:
188193
timeouts are not retried (``read=0``) and only 429/503 are status-
189194
retried. Honors ``Retry-After``. Disabled with AGENTKIT_HTTP_RETRIES=0.
190195
"""
191-
try:
192-
retries = max(0, int(os.getenv("AGENTKIT_HTTP_RETRIES", "2")))
193-
except ValueError:
194-
retries = 2
196+
retries = http_retries()
195197
if retries <= 0:
196198
return
197199
session = getattr(self, "session", None)
@@ -251,7 +253,14 @@ def _refresh_credentials_if_needed(self, *, force: bool = False) -> bool:
251253
creds = self._platform_config.get_vefaas_iam_credentials(force=force)
252254
if not creds:
253255
return False
254-
return self._apply_credentials(creds)
256+
applied = self._apply_credentials(creds)
257+
if applied:
258+
logger.info(
259+
"Refreshed vefaas credentials (force=%s) for service %s",
260+
force,
261+
self.service,
262+
)
263+
return applied
255264

256265
def _is_probable_credential_error_code(self, code: str) -> bool:
257266
c = (code or "").lower()
@@ -319,12 +328,18 @@ def _invoke_api(
319328
Typed response object
320329
321330
Raises:
322-
Exception: If API call fails or returns an error
331+
ApiError: If the API call fails or returns an error response.
332+
NetworkError: If a transport-level failure occurs.
323333
"""
334+
# Imported lazily: ``agentkit.toolkit`` pulls in ``sdk`` which imports
335+
# back into ``agentkit.client``, so a module-level import would cycle.
336+
from agentkit.toolkit.errors import ApiError
337+
324338
self._refresh_credentials_if_needed()
325339
last_error: Optional[BaseException] = None
326340

327341
for attempt in (0, 1):
342+
logger.debug("Invoking API %s (attempt %d)", api_action, attempt)
328343
if attempt == 1:
329344
self._refresh_credentials_if_needed(force=True)
330345

@@ -336,16 +351,34 @@ def _invoke_api(
336351
request.model_dump(by_alias=True, exclude_none=True)
337352
),
338353
)
354+
except (
355+
requests.exceptions.ConnectionError,
356+
requests.exceptions.Timeout,
357+
requests.exceptions.HTTPError,
358+
requests.exceptions.RequestException,
359+
) as e:
360+
# Transport-level failure: not a credential issue, surface as
361+
# a typed network error so callers can distinguish it.
362+
raise NetworkError(f"Failed to {api_action}: network error") from e
339363
except Exception as e:
340364
last_error = e
341365
if attempt == 0 and self._is_probable_credential_error_text(str(e)):
366+
logger.debug(
367+
"Retrying %s after probable credential error", api_action
368+
)
342369
continue
343-
raise Exception(f"Failed to {api_action}: {str(e)}") from e
370+
raise ApiError(f"Failed to {api_action}: {str(e)}") from e
344371

345372
if not res:
346-
raise Exception(f"Empty response from {api_action} request.")
373+
raise ApiError(f"Empty response from {api_action} request.")
374+
375+
try:
376+
response_data = json.loads(res)
377+
except (ValueError, TypeError) as e:
378+
raise ApiError(
379+
f"Failed to {api_action}: malformed response body"
380+
) from e
347381

348-
response_data = json.loads(res)
349382
metadata = response_data.get("ResponseMetadata", {})
350383
if metadata.get("Error"):
351384
err = metadata.get("Error", {}) or {}
@@ -355,13 +388,21 @@ def _invoke_api(
355388
self._is_probable_credential_error_code(error_code)
356389
or self._is_probable_credential_error_text(error_msg)
357390
):
391+
logger.debug(
392+
"Retrying %s after credential error code %s",
393+
api_action,
394+
error_code or "<none>",
395+
)
358396
continue
359-
raise Exception(f"Failed to {api_action}: {error_msg}")
397+
raise ApiError(
398+
f"Failed to {api_action}: {error_msg}",
399+
error_code=error_code or None,
400+
)
360401

361402
return response_type(**response_data.get("Result", {}))
362403

363404
if last_error is not None:
364-
raise Exception(
405+
raise ApiError(
365406
f"Failed to {api_action}: {str(last_error)}"
366407
) from last_error
367-
raise Exception(f"Failed to {api_action}: Unknown error")
408+
raise ApiError(f"Failed to {api_action}: Unknown error")

agentkit/errors.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# Copyright (c) 2025 Beijing Volcano Engine Technology Co., Ltd. and/or its affiliates.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Top-level exception root for AgentKit.
16+
17+
This module is a stdlib-only leaf: it has no intra-package imports so that any
18+
subpackage (``agentkit.auth``, ``agentkit.toolkit``, ...) can subclass the
19+
single root without creating dependency cycles.
20+
"""
21+
22+
23+
class AgentKitError(Exception):
24+
"""The single root of the AgentKit exception hierarchy.
25+
26+
Every domain-specific AgentKit exception (auth, toolkit, API, ...) ultimately
27+
inherits from this class so callers can ``except AgentKitError`` to catch any
28+
AgentKit-originated failure.
29+
"""
30+
31+
32+
__all__ = ["AgentKitError"]

agentkit/platform/configuration.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
from __future__ import annotations
1616

1717
import json
18-
import logging
1918
import os
2019
import threading
2120
import time
@@ -42,8 +41,9 @@
4241
get_global_config_value,
4342
read_global_config_dict,
4443
)
44+
from agentkit.utils.logging_config import get_logger
4545

46-
logger = logging.getLogger(__name__)
46+
logger = get_logger(__name__)
4747

4848
VEFAAS_IAM_CREDENTIAL_PATH = "/var/run/secrets/iam/credential"
4949
VEFAAS_IAM_CREDENTIAL_FALLBACK_TTL_SECONDS = 60

agentkit/sdk/account/client.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,7 @@ def get_service_status(self, service_name: str) -> Optional[str]:
6666
Returns:
6767
The status string ('Enabled' or 'Disabled'), or None if service not found.
6868
"""
69-
print(1)
7069
response = self.list_account_linked_services(ListAccountLinkedServicesRequest())
71-
print(2)
7270
if not response.service_statuses:
7371
return None
7472
for svc in response.service_statuses:

agentkit/sdk/identity/auth.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
resolve_credentials,
2222
resolve_endpoint,
2323
)
24+
from agentkit.toolkit.errors import ApiError
2425

2526

2627
def requires_api_key(*, provider_name: str, into: str = "api_key"):
@@ -59,8 +60,8 @@ def _get_api_key() -> str:
5960

6061
try:
6162
return response["Result"]["ApiKey"]
62-
except Exception as _:
63-
raise RuntimeError(f"Get api key failed: {response}")
63+
except Exception as e:
64+
raise ApiError("GetResourceApiKey did not return an ApiKey") from e
6465

6566
@wraps(func)
6667
async def async_wrapper(*args: Any, **kwargs: Any) -> Any:

0 commit comments

Comments
 (0)