diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md new file mode 100644 index 0000000..dab4a45 --- /dev/null +++ b/RELEASE_NOTES.md @@ -0,0 +1,57 @@ +# Release Notes + +## Unreleased + +### Breaking Changes + +- Sandbox data-plane HTTP JSON APIs now follow standard HTTP error handling: + - `2xx` responses return the business response body. + - `4xx` responses raise `ClientError`. + - `5xx` responses raise `ServerError`. +- Error responses such as `{"code": "...", "requestId": "...", "message": "..."}` + are no longer returned as normal dictionaries for Sandbox HTTP JSON APIs. The + fields are exposed on the raised exception as `error_code`, `request_id`, and + `message`. +- Existing code that checked returned dictionaries for `code` and `requestId` + must migrate to `try` / `except ClientError` / `except ServerError`. +- `HTTPError.__str__()` output format has changed. The old format unconditionally + included `"Request ID: None. Details: {}"` even when those fields were empty. + The new format only includes non-empty fields and uses `". "` as separator. + Code that parses this string representation (e.g. log parsers or test assertions + on `str(error)`) must be updated. + +### Migration + +Before: + +```python +resp = ci.cmd(command="echo hello", cwd="/tmp", timeout=30) +if "code" in resp and "requestId" in resp: + raise RuntimeError(resp["message"]) +``` + +After: + +```python +from agentrun.utils.exception import ClientError, ServerError + +try: + resp = ci.cmd(command="echo hello", cwd="/tmp", timeout=30) +except ClientError as e: + print(e.status_code, e.error_code, e.request_id, e.message) + raise +except ServerError as e: + print(e.status_code, e.error_code, e.request_id, e.message) + raise +``` + +Command execution failures are still business-level failures and should be +handled by checking `resp["result"]["exitCode"]` after a successful HTTP +response. + +### Scope + +This change is intentionally limited to Sandbox data-plane HTTP JSON APIs. It +does not change WebSocket/CDP/VNC URL generation, Playwright connections, file +upload/download helpers, video download helpers, or non-Sandbox data-plane +clients. diff --git a/agentrun/sandbox/api/__sandbox_data_async_template.py b/agentrun/sandbox/api/__sandbox_data_async_template.py index 2cf9d3c..987890c 100644 --- a/agentrun/sandbox/api/__sandbox_data_async_template.py +++ b/agentrun/sandbox/api/__sandbox_data_async_template.py @@ -4,13 +4,23 @@ This template is used to generate sandbox data API code. """ -from typing import Any, Dict, Optional +from typing import Any, Dict, Optional, Tuple, Union + +import httpx from agentrun.utils.config import Config from agentrun.utils.data_api import DataAPI, ResourceType +from agentrun.utils.exception import ClientError, ServerError +from agentrun.utils.log import logger class SandboxDataAPI(DataAPI): + _REQUEST_ID_HEADERS = ( + "x-acs-request-id", + "x-agentrun-request-id", + "x-request-id", + "x-fc-request-id", + ) def __init__( self, @@ -63,6 +73,126 @@ def __refresh_access_token( self.auth(config=cfg) self.access_token_map[sandbox_id or template_name] = self.access_token + @classmethod + def _extract_error_fields( + cls, response: httpx.Response, response_body: Any + ) -> Tuple[Optional[str], Optional[str], str]: + error_code = None + request_id = None + message = "" + + if isinstance(response_body, dict): + raw_code = response_body.get("code") + if raw_code is not None: + error_code = str(raw_code) + + raw_request_id = response_body.get("requestId") + if raw_request_id is not None: + request_id = str(raw_request_id) + + raw_message = response_body.get("message") + if raw_message is not None: + message = str(raw_message) + elif isinstance(response_body, str): + message = response_body.strip() + + if request_id is None: + for header in cls._REQUEST_ID_HEADERS: + value = response.headers.get(header) + if value: + request_id = value + break + + if not message: + message = ( + response.reason_phrase or f"HTTP {response.status_code} error" + ) + + return error_code, request_id, message + + @staticmethod + def _parse_error_response_body(response: httpx.Response) -> Any: + if not response.text: + return {} + try: + return response.json() + except ValueError: + return response.text + + @staticmethod + def _parse_success_response(response: httpx.Response) -> Dict[str, Any]: + if not response.text: + return {} + try: + return response.json() + except ValueError as e: + error_msg = f"Failed to parse JSON response: {e}" + logger.error(error_msg) + raise ClientError( + status_code=response.status_code, + message=error_msg, + response_body=response.text, + response_headers=dict(response.headers), + ) from e + + @classmethod + def _raise_for_error_response(cls, response: httpx.Response) -> None: + response_body = cls._parse_error_response_body(response) + error_code, request_id, message = cls._extract_error_fields( + response, response_body + ) + if response.status_code >= 500: + raise ServerError( + status_code=response.status_code, + message=message, + request_id=request_id, + error_code=error_code, + response_body=response_body, + response_headers=dict(response.headers), + ) + raise ClientError( + status_code=response.status_code, + message=message, + request_id=request_id, + error_code=error_code, + response_body=response_body, + response_headers=dict(response.headers), + ) + + async def _make_request_async( + self, + method: str, + url: str, + data: Optional[Union[Dict[str, Any], str]] = None, + headers: Optional[Dict[str, str]] = None, + query: Optional[Dict[str, Any]] = None, + config: Optional[Config] = None, + ) -> Dict[str, Any]: + method, url, req_headers, req_json, req_content = self._prepare_request( + method, url, data, headers, query, config=config + ) + + try: + async with httpx.AsyncClient( + timeout=self.config.get_timeout() + ) as client: + response = await client.request( + method, + url, + headers=req_headers, + json=req_json, + content=req_content, + ) + logger.debug(f"Response: {response.text}") + + if response.status_code >= 400: + self._raise_for_error_response(response) + + return self._parse_success_response(response) + except httpx.RequestError as e: + error_msg = f"Request error: {e!s}" + raise ClientError(status_code=0, message=error_msg) from e + async def check_health_async(self): return await self.get_async("/health") diff --git a/agentrun/sandbox/api/sandbox_data.py b/agentrun/sandbox/api/sandbox_data.py index c021a72..2383e31 100644 --- a/agentrun/sandbox/api/sandbox_data.py +++ b/agentrun/sandbox/api/sandbox_data.py @@ -14,13 +14,23 @@ This template is used to generate sandbox data API code. """ -from typing import Any, Dict, Optional +from typing import Any, Dict, Optional, Tuple, Union + +import httpx from agentrun.utils.config import Config from agentrun.utils.data_api import DataAPI, ResourceType +from agentrun.utils.exception import ClientError, ServerError +from agentrun.utils.log import logger class SandboxDataAPI(DataAPI): + _REQUEST_ID_HEADERS = ( + "x-acs-request-id", + "x-agentrun-request-id", + "x-request-id", + "x-fc-request-id", + ) def __init__( self, @@ -73,6 +83,158 @@ def __refresh_access_token( self.auth(config=cfg) self.access_token_map[sandbox_id or template_name] = self.access_token + @classmethod + def _extract_error_fields( + cls, response: httpx.Response, response_body: Any + ) -> Tuple[Optional[str], Optional[str], str]: + error_code = None + request_id = None + message = "" + + if isinstance(response_body, dict): + raw_code = response_body.get("code") + if raw_code is not None: + error_code = str(raw_code) + + raw_request_id = response_body.get("requestId") + if raw_request_id is not None: + request_id = str(raw_request_id) + + raw_message = response_body.get("message") + if raw_message is not None: + message = str(raw_message) + elif isinstance(response_body, str): + message = response_body.strip() + + if request_id is None: + for header in cls._REQUEST_ID_HEADERS: + value = response.headers.get(header) + if value: + request_id = value + break + + if not message: + message = ( + response.reason_phrase or f"HTTP {response.status_code} error" + ) + + return error_code, request_id, message + + @staticmethod + def _parse_error_response_body(response: httpx.Response) -> Any: + if not response.text: + return {} + try: + return response.json() + except ValueError: + return response.text + + @staticmethod + def _parse_success_response(response: httpx.Response) -> Dict[str, Any]: + if not response.text: + return {} + try: + return response.json() + except ValueError as e: + error_msg = f"Failed to parse JSON response: {e}" + logger.error(error_msg) + raise ClientError( + status_code=response.status_code, + message=error_msg, + response_body=response.text, + response_headers=dict(response.headers), + ) from e + + @classmethod + def _raise_for_error_response(cls, response: httpx.Response) -> None: + response_body = cls._parse_error_response_body(response) + error_code, request_id, message = cls._extract_error_fields( + response, response_body + ) + if response.status_code >= 500: + raise ServerError( + status_code=response.status_code, + message=message, + request_id=request_id, + error_code=error_code, + response_body=response_body, + response_headers=dict(response.headers), + ) + raise ClientError( + status_code=response.status_code, + message=message, + request_id=request_id, + error_code=error_code, + response_body=response_body, + response_headers=dict(response.headers), + ) + + async def _make_request_async( + self, + method: str, + url: str, + data: Optional[Union[Dict[str, Any], str]] = None, + headers: Optional[Dict[str, str]] = None, + query: Optional[Dict[str, Any]] = None, + config: Optional[Config] = None, + ) -> Dict[str, Any]: + method, url, req_headers, req_json, req_content = self._prepare_request( + method, url, data, headers, query, config=config + ) + + try: + async with httpx.AsyncClient( + timeout=self.config.get_timeout() + ) as client: + response = await client.request( + method, + url, + headers=req_headers, + json=req_json, + content=req_content, + ) + logger.debug(f"Response: {response.text}") + + if response.status_code >= 400: + self._raise_for_error_response(response) + + return self._parse_success_response(response) + except httpx.RequestError as e: + error_msg = f"Request error: {e!s}" + raise ClientError(status_code=0, message=error_msg) from e + + def _make_request( + self, + method: str, + url: str, + data: Optional[Union[Dict[str, Any], str]] = None, + headers: Optional[Dict[str, str]] = None, + query: Optional[Dict[str, Any]] = None, + config: Optional[Config] = None, + ) -> Dict[str, Any]: + method, url, req_headers, req_json, req_content = self._prepare_request( + method, url, data, headers, query, config=config + ) + + try: + with httpx.Client(timeout=self.config.get_timeout()) as client: + response = client.request( + method, + url, + headers=req_headers, + json=req_json, + content=req_content, + ) + logger.debug(f"Response: {response.text}") + + if response.status_code >= 400: + self._raise_for_error_response(response) + + return self._parse_success_response(response) + except httpx.RequestError as e: + error_msg = f"Request error: {e!s}" + raise ClientError(status_code=0, message=error_msg) from e + async def check_health_async(self): return await self.get_async("/health") diff --git a/agentrun/utils/exception.py b/agentrun/utils/exception.py index dcdbf1b..1b62035 100644 --- a/agentrun/utils/exception.py +++ b/agentrun/utils/exception.py @@ -1,6 +1,6 @@ """异常定义""" -from typing import Optional +from typing import Any, Dict, Optional class AgentRunError(Exception): @@ -58,18 +58,34 @@ def __init__( status_code: int, message: str, request_id: Optional[str] = None, + error_code: Optional[str] = None, + response_body: Any = None, + response_headers: Optional[Dict[str, Any]] = None, **kwargs, ): self.status_code = status_code self.request_id = request_id - self.details = kwargs + self.error_code = error_code + self.response_body = response_body + self.response_headers = response_headers super().__init__(message, **kwargs) def __str__(self) -> str: - return ( - f"HTTP {self.status_code}: {self.message}. Request ID:" - f" {self.request_id}. Details: {self.details_str()}" - ) + parts = [f"HTTP {self.status_code}: {self.message}"] + if self.error_code: + parts.append(f"Error Code: {self.error_code}") + if self.request_id: + parts.append(f"Request ID: {self.request_id}") + + extra_details = { + key: value + for key, value in self.details.items() + if key not in {"error_code", "response_body", "response_headers"} + } + if extra_details: + parts.append(f"Details: {self.kwargs_str(**extra_details)}") + + return ". ".join(parts) def to_resource_error( self, resource_type: str, resource_id: Optional[str] = "" @@ -92,24 +108,10 @@ def to_resource_error( class ClientError(HTTPError): """客户端异常类""" - def __init__( - self, - status_code: int, - message: str, - request_id: Optional[str] = None, - **kwargs, - ): - super().__init__(status_code, message, request_id=request_id, **kwargs) - class ServerError(HTTPError): """服务端异常类""" - def __init__( - self, status_code: int, message: str, request_id: Optional[str] = None - ): - super().__init__(status_code, message, request_id=request_id) - class ResourceNotExistError(AgentRunError): """资源不存在异常""" diff --git a/tests/unittests/conftest.py b/tests/unittests/conftest.py new file mode 100644 index 0000000..608c88a --- /dev/null +++ b/tests/unittests/conftest.py @@ -0,0 +1,54 @@ +"""Unit test fixtures / 单元测试公共 fixture. + +本模块统一为 tests/unittests/ 目录提供测试隔离保障。 + +Why: + agentrun.utils.config 在模块导入时调用 load_dotenv(), 会把仓库根目录 + 的 .env 注入到 os.environ。Config() 在构造时会用 get_env_with_default + 读取这些环境变量作为默认值, 导致： + - respx mock 的 URL 是基于 SDK 默认 account_id / region 构造, + 而实际请求打到开发者本地 .env 里的真实 account / region, + 触发 "AllMockedAssertionError: ... not mocked!" + - 硬编码期望默认 account_id / region 的断言直接 AssertionError + +How: + autouse fixture 在每个 test 进入前, 通过 monkeypatch.delenv 清理 + Config.__init__ 读取的那一组 env。monkeypatch 会在 test 结束时 + 自动恢复原值, 不影响仓库 .env 文件本身, 也不影响显式使用 + patch.dict(os.environ, ...) 设置 test 用凭据的测试 (patch.dict + 会覆盖 delenv 的结果)。 +""" + +from typing import List + +import pytest + +# Config.__init__ 默认值会读取的环境变量白名单 +# 仅清理这些 key, 不动用户测试自己 set 的其他 AGENTRUN_* 变量 +# (例如 memory_collection test 里的 AGENTRUN_MYSQL_PUBLIC_HOST) +_SDK_CONFIG_ENV_KEYS: List[str] = [ + # 凭据类 / Credentials + "AGENTRUN_ACCESS_KEY_ID", + "ALIBABA_CLOUD_ACCESS_KEY_ID", + "AGENTRUN_ACCESS_KEY_SECRET", + "ALIBABA_CLOUD_ACCESS_KEY_SECRET", + "AGENTRUN_SECURITY_TOKEN", + "ALIBABA_CLOUD_SECURITY_TOKEN", + # 账号与区域 / Account & Region + "AGENTRUN_ACCOUNT_ID", + "FC_ACCOUNT_ID", + "AGENTRUN_REGION", + "FC_REGION", + # Endpoint 类 / Endpoints + "AGENTRUN_CONTROL_ENDPOINT", + "AGENTRUN_DATA_ENDPOINT", + "DEVS_ENDPOINT", + "BAILIAN_ENDPOINT", +] + + +@pytest.fixture(autouse=True) +def _isolate_sdk_config_env(monkeypatch: pytest.MonkeyPatch) -> None: + """自动为每个 unit test 清理 SDK Config 相关 env, 避免被本地 .env 污染.""" + for key in _SDK_CONFIG_ENV_KEYS: + monkeypatch.delenv(key, raising=False) diff --git a/tests/unittests/sandbox/api/test_sandbox_data.py b/tests/unittests/sandbox/api/test_sandbox_data.py index b68f422..47e59ac 100644 --- a/tests/unittests/sandbox/api/test_sandbox_data.py +++ b/tests/unittests/sandbox/api/test_sandbox_data.py @@ -2,9 +2,15 @@ from unittest.mock import AsyncMock, MagicMock, patch +import httpx import pytest +import respx from agentrun.sandbox.api.sandbox_data import SandboxDataAPI +from agentrun.utils.config import Config +from agentrun.utils.exception import ClientError, ServerError + +DATA_ENDPOINT = "https://sandbox-data.example.com" @pytest.fixture @@ -179,3 +185,140 @@ async def test_get_sandbox_async(self, api): api._SandboxDataAPI__refresh_access_token = MagicMock() await api.get_sandbox_async("sb-1") api.get_async.assert_called_once_with("/", config=None) + + +class TestSandboxDataAPIHTTPHandling: + + @staticmethod + def make_api(): + config = Config( + account_id="test-account", + data_endpoint=DATA_ENDPOINT, + access_key_id="", + access_key_secret="", + ) + return SandboxDataAPI(sandbox_id="sb-1", config=config) + + @respx.mock + def test_sync_success_returns_business_body(self): + api = self.make_api() + body = {"executionId": "exec-1", "status": "completed"} + respx.post(f"{DATA_ENDPOINT}/sandboxes/sb-1/processes/cmd").mock( + return_value=httpx.Response(200, json=body) + ) + + result = api.post("/processes/cmd", data={"command": "ls"}) + + assert result == body + + @respx.mock + def test_sync_success_code_body_is_not_treated_as_error(self): + api = self.make_api() + body = {"code": "SUCCESS", "data": {"sandboxId": "sb-1"}} + respx.get(f"{DATA_ENDPOINT}/sandboxes/sb-1/health").mock( + return_value=httpx.Response(200, json=body) + ) + + result = api.get("/health") + + assert result == body + + @respx.mock + def test_sync_success_non_json_body_raises_client_error(self): + api = self.make_api() + respx.get(f"{DATA_ENDPOINT}/sandboxes/sb-1/health").mock( + return_value=httpx.Response(200, text="ok") + ) + + with pytest.raises(ClientError) as exc_info: + api.get("/health") + + error = exc_info.value + assert error.status_code == 200 + assert "Failed to parse JSON response" in error.message + assert error.response_body == "ok" + + @respx.mock + def test_sync_client_error_extracts_error_envelope(self): + api = self.make_api() + body = { + "code": "ERR_FORBIDDEN", + "requestId": "req-body", + "message": "Signature verification failed", + } + respx.get(f"{DATA_ENDPOINT}/sandboxes/sb-1/health").mock( + return_value=httpx.Response( + 403, + json=body, + headers={"x-acs-request-id": "req-header"}, + ) + ) + + with pytest.raises(ClientError) as exc_info: + api.get("/health") + + error = exc_info.value + assert error.status_code == 403 + assert error.error_code == "ERR_FORBIDDEN" + assert error.request_id == "req-body" + assert error.message == "Signature verification failed" + assert error.response_body == body + assert error.response_headers is not None + + @respx.mock + def test_sync_client_error_uses_text_body_and_header_request_id(self): + api = self.make_api() + respx.get(f"{DATA_ENDPOINT}/sandboxes/sb-1/missing").mock( + return_value=httpx.Response( + 404, + text="sandbox not found", + headers={"x-request-id": "req-header"}, + ) + ) + + with pytest.raises(ClientError) as exc_info: + api.get("/missing") + + error = exc_info.value + assert error.status_code == 404 + assert error.error_code is None + assert error.request_id == "req-header" + assert error.message == "sandbox not found" + assert error.response_body == "sandbox not found" + + @respx.mock + @pytest.mark.asyncio + async def test_async_server_error_extracts_error_envelope(self): + api = self.make_api() + body = { + "code": "ERR_INTERNAL_SERVER", + "requestId": "req-500", + "message": "Internal server error", + } + respx.get(f"{DATA_ENDPOINT}/sandboxes/sb-1/health").mock( + return_value=httpx.Response(503, json=body) + ) + + with pytest.raises(ServerError) as exc_info: + await api.get_async("/health") + + error = exc_info.value + assert error.status_code == 503 + assert error.error_code == "ERR_INTERNAL_SERVER" + assert error.request_id == "req-500" + assert error.message == "Internal server error" + assert error.response_body == body + + @respx.mock + @pytest.mark.asyncio + async def test_async_request_error_still_raises_client_error(self): + api = self.make_api() + respx.get(f"{DATA_ENDPOINT}/sandboxes/sb-1/health").mock( + side_effect=httpx.RequestError("Connection failed") + ) + + with pytest.raises(ClientError) as exc_info: + await api.get_async("/health") + + assert exc_info.value.status_code == 0 + assert "Connection failed" in exc_info.value.message diff --git a/tests/unittests/utils/test_exception.py b/tests/unittests/utils/test_exception.py index a08c36c..488a3bf 100644 --- a/tests/unittests/utils/test_exception.py +++ b/tests/unittests/utils/test_exception.py @@ -64,22 +64,53 @@ def test_init(self): status_code=404, message="Not Found", request_id="req-123", + error_code="ERR_NOT_FOUND", + response_body={"code": "ERR_NOT_FOUND"}, + response_headers={"x-request-id": "req-123"}, extra="info", ) assert error.status_code == 404 assert error.message == "Not Found" assert error.request_id == "req-123" + assert error.error_code == "ERR_NOT_FOUND" + assert error.response_body == {"code": "ERR_NOT_FOUND"} + assert error.response_headers == {"x-request-id": "req-123"} assert error.details["extra"] == "info" + assert "error_code" not in error.details + assert "response_body" not in error.details + assert "response_headers" not in error.details def test_str(self): """测试字符串表示""" error = HTTPError( - status_code=500, message="Internal Error", request_id="req-456" + status_code=500, + message="Internal Error", + request_id="req-456", + error_code="ERR_INTERNAL", ) result = str(error) assert "HTTP 500" in result assert "Internal Error" in result + assert "ERR_INTERNAL" in result assert "req-456" in result + assert "response_headers" not in result + + def test_str_keeps_custom_details(self): + """测试字符串保留额外详情但不展开响应元数据""" + error = HTTPError( + status_code=400, + message="Bad Request", + request_id="req-1", + error_code="ERR_BAD_REQUEST", + response_body={"code": "ERR_BAD_REQUEST"}, + response_headers={"x-request-id": "req-1"}, + field="name", + ) + result = str(error) + assert "field" in result + assert "name" in result + assert "response_body" not in result + assert "response_headers" not in result def test_to_resource_error_not_found(self): """测试转换为 ResourceNotExistError (does not exist)""" @@ -147,11 +178,15 @@ def test_init(self): status_code=400, message="Bad Request", request_id="req-789", + error_code="ERR_BAD_REQUEST", + response_body={"code": "ERR_BAD_REQUEST"}, field="value", ) assert error.status_code == 400 assert error.message == "Bad Request" assert error.request_id == "req-789" + assert error.error_code == "ERR_BAD_REQUEST" + assert error.response_body == {"code": "ERR_BAD_REQUEST"} class TestServerError: @@ -160,11 +195,17 @@ class TestServerError: def test_init(self): """测试初始化""" error = ServerError( - status_code=503, message="Service Unavailable", request_id="req-000" + status_code=503, + message="Service Unavailable", + request_id="req-000", + error_code="ERR_UNAVAILABLE", + response_headers={"x-request-id": "req-000"}, ) assert error.status_code == 503 assert error.message == "Service Unavailable" assert error.request_id == "req-000" + assert error.error_code == "ERR_UNAVAILABLE" + assert error.response_headers == {"x-request-id": "req-000"} class TestResourceNotExistError: