diff --git a/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/agent.md b/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/agent.md index 1140350..11e5e6d 100644 --- a/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/agent.md +++ b/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/agent.md @@ -67,16 +67,21 @@ with client.sessions.events.stream(session.id) as stream: | Error code/exception | HTTP status | Meaning | Handling | | --- | --- | --- | --- | | `InputRequired` / `ModelRequired` / `InvalidInput` | raised locally | missing required params such as app_id/prompt/model | add params per the exception message; Application failures do not raise — check status_code | -| `invalid_request_error` (InvalidRequestError) | 400 | invalid agentstudio request parameters | check parameter names/types, e.g. empty events raises a local ValueError | +| `invalid_request_error` / `request_too_large` (InvalidRequestError) | 400/413 | invalid agentstudio request parameters, or payload too large | check parameter names/types, e.g. empty events raises a local ValueError | | `authentication_error` (AuthenticationError) | 401 | api_key missing or invalid | check api_key or DASHSCOPE_API_KEY | -| `permission_denied_error` (PermissionDeniedError) | 403 | no access to the resource | check workspace/uid and resource ownership | +| `permission_denied_error` / `permission_error` (PermissionDeniedError) | 403 | no access to the resource | check workspace/uid and resource ownership | | `not_found_error` (NotFoundError) | 404 | agent/session and other resources do not exist | verify the id and region/workspace are correct | | `conflict_error` (ConflictError) | 409 | version in agents.update does not match the server | retrieve the latest version first, then update | -| `rate_limit_error` (RateLimitError) | 429 | rate limited | retry with backoff (client default max_retries=2) | -| `api_error` (InternalServerError) / `overloaded_error` (OverloadedError) | 500/502/504 / 503 | server error/overloaded | troubleshoot with `request_id` or retry | +| `rate_limit_error` / `billing_error` (RateLimitError) | 429 | rate limited or out of quota | retry with backoff (client default max_retries=2) | +| `api_error` / `timeout_error` (InternalServerError), `overloaded_error` (OverloadedError) | 500/502/504 / 503 | server error/overloaded | troubleshoot with `request_id` or retry | | `APITimeoutError` / `APIConnectionError` | no response | connection/read/write timeout | increase timeout, check network, then retry | | `StreamError` / `StreamClosedError` | mid-stream | SSE protocol error/stream closed | call `events.stream` again to reopen the stream | +Match on the exception class, not on `.code`: `.code` echoes the server's raw +string, so a 403 may arrive as either `permission_denied_error` or +`permission_error`, and it is normalised to `api_error` when the server omits +it. The class is always the one listed above. + ## Java SDK Java SDK v2.22.23 entry points: diff --git a/dashscope/agentstudio/exceptions.py b/dashscope/agentstudio/exceptions.py index 124ab5e..d057339 100644 --- a/dashscope/agentstudio/exceptions.py +++ b/dashscope/agentstudio/exceptions.py @@ -10,10 +10,20 @@ "request_id": "req_..." } -The pre-release backend currently emits ``error_code``/``error_message`` -instead of nested ``error.{code,message}``. We accept both shapes and -normalize to the documented form. The compatibility branch is marked -with ``# TODO(bma-fix)`` so we can remove it once the backend aligns. +Codes come from the server response and are preserved as-is on ``.code``. +When no code is present, :func:`from_response` reports the generic +``api_error`` rather than inventing a public code from the status number. +The raw payload stays on ``.raw``. + +The exception *type* is resolved separately: a recognized server code wins, +otherwise the HTTP status picks the class. Callers can therefore keep using +``except NotFoundError`` while ``.code`` still carries whatever the server +actually said (including service-specific codes such as +``bma_invalid_event``). + +The pre-release backend emits ``error_code``/``error_message`` instead of +nested ``error.{code,message}``. Both shapes are accepted; the compatibility +branches are marked ``# TODO(bma-fix)`` for removal once the backend aligns. """ from __future__ import annotations @@ -21,6 +31,25 @@ from typing import Any, Dict, Mapping, Optional from dashscope.common.error import DashScopeException +from dashscope.common.error_registry import ( + SDK_AGENTSTUDIO_API_CONNECTION_ERROR, + SDK_AGENTSTUDIO_API_TIMEOUT_ERROR, + SDK_AGENTSTUDIO_STREAM_CLOSED_ERROR, + SDK_AGENTSTUDIO_STREAM_ERROR, + INTERNAL_ERROR, +) + + +def describe_exception(err: BaseException) -> str: + """Render an exception into a message worth showing the caller. + + ``httpx`` timeout exceptions usually carry no message at all, so a bare + ``str(err)`` would surface an empty string. Fall back to the type name. + """ + detail = str(err).strip() + if detail: + return f"{type(err).__name__}: {detail}" + return type(err).__name__ class AgentStudioError(DashScopeException): @@ -75,13 +104,13 @@ def __repr__(self) -> str: # pragma: no cover - debug helper class APIConnectionError(AgentStudioError): """Raised when the HTTP request fails before a response is read.""" - code = "api_connection_error" + code = SDK_AGENTSTUDIO_API_CONNECTION_ERROR.name class APITimeoutError(APIConnectionError): """Raised on connect / read timeouts.""" - code = "api_timeout_error" + code = SDK_AGENTSTUDIO_API_TIMEOUT_ERROR.name # --------------------------------------------------------------------------- @@ -90,7 +119,12 @@ class APITimeoutError(APIConnectionError): class APIStatusError(AgentStudioError): - """Raised when the server returns a non-2xx status.""" + """Raised when the server returns a non-2xx status. + + Subclasses below give callers a stable type to catch. ``.code`` is always + the server's own value, so the class carries no information that the code + attribute does not -- it exists for ``except`` clauses. + """ code = "api_status_error" @@ -124,7 +158,7 @@ class OverloadedError(APIStatusError): class InternalServerError(APIStatusError): - code = "api_error" + code = INTERNAL_ERROR.anthropic_error_code # --------------------------------------------------------------------------- @@ -135,26 +169,29 @@ class InternalServerError(APIStatusError): class StreamError(AgentStudioError): """Raised when an SSE stream encounters a fatal protocol error.""" - code = "stream_error" + code = SDK_AGENTSTUDIO_STREAM_ERROR.name class StreamClosedError(StreamError): """Raised when consumers attempt I/O on an already-closed stream.""" - code = "stream_closed" + code = SDK_AGENTSTUDIO_STREAM_CLOSED_ERROR.name # --------------------------------------------------------------------------- # Dispatcher # --------------------------------------------------------------------------- - _STATUS_TO_DEFAULT: Dict[int, type] = { 400: InvalidRequestError, 401: AuthenticationError, 403: PermissionDeniedError, 404: NotFoundError, + # 408 pairs with 504: both are timeouts, which the registry classes + # as InternalServerError (timeout_error). + 408: InternalServerError, 409: ConflictError, + 413: InvalidRequestError, 429: RateLimitError, 500: InternalServerError, 502: InternalServerError, @@ -162,15 +199,24 @@ class StreamClosedError(StreamError): 504: InternalServerError, } +# Covers every anthropic_error_code the error registry can emit, plus the +# registry's request_timeout key spelling and the legacy +# permission_denied_error / conflict_error spellings. Each entry agrees +# with _STATUS_TO_DEFAULT for the status the registry pairs it with. _CODE_TO_CLASS: Dict[str, type] = { "invalid_request_error": InvalidRequestError, "authentication_error": AuthenticationError, + "permission_error": PermissionDeniedError, "permission_denied_error": PermissionDeniedError, "not_found_error": NotFoundError, "conflict_error": ConflictError, + "request_too_large": InvalidRequestError, "rate_limit_error": RateLimitError, + "billing_error": RateLimitError, "overloaded_error": OverloadedError, "api_error": InternalServerError, + "timeout_error": InternalServerError, + "request_timeout": InternalServerError, } @@ -179,16 +225,21 @@ def from_response( status_code: int, body: Any, headers: Optional[Mapping[str, str]] = None, -) -> AgentStudioError: - """Build an :class:`AgentStudioError` instance from a HTTP response. +) -> APIStatusError: + """Build an :class:`APIStatusError` instance from a HTTP response. - Accepts both the documented ``{type, error:{code,message}, request_id}`` - shape and the pre-release ``{type, error:{error_code, error_message}}`` - shape. Falls back to a Spring default ``{timestamp,status,error,path}`` - when the body is not JSON-serializable. + Accepts the documented ``{type, error:{code,message}, request_id}`` shape, + the pre-release ``error:{error_code,error_message}`` shape, the classic + flat DashScope ``{code, message, request_id}`` envelope, and falls back to + a Spring default ``{timestamp,status,error,path}`` page. The ``x-request-id`` response header is preferred over the body ``request_id`` field (server-generated IDs are more reliable for tracing). + + The server's code is preserved as-is on ``.code``; only when no code is + present does it fall back to the generic ``api_error``. The exception + class is resolved separately -- a recognized code wins, otherwise the + HTTP status decides. """ code: Optional[str] = None @@ -211,30 +262,28 @@ def from_response( message = err.get("message") or err.get( "error_message", ) # TODO(bma-fix) - # Spring default fallback. - if ( - message is None - and "error" in body - and isinstance(body["error"], str) - ): + # Spring default fallback. Its ``error`` field is a human phrase + # ("Not Found"), so it informs the message but never the code. + if message is None and isinstance(body.get("error"), str): message = body["error"] - code = body.get("error") or "api_error" + # Flat DashScope envelope: code/message at the top level. + if code is None: + code = body.get("code") if message is None: message = body.get("message") + # Classify before normalizing: the synthesized generic code below must not + # drive classification, or a bodiless 404 would report InternalServerError. + cls = _CODE_TO_CLASS.get(code) if code else None + if cls is None: + cls = _STATUS_TO_DEFAULT.get(status_code, APIStatusError) + + if not code: + code = INTERNAL_ERROR.anthropic_error_code + if message is None: message = f"HTTP {status_code}" - if code is None: - code = _STATUS_TO_DEFAULT.get( # type: ignore[attr-defined] - status_code, - APIStatusError, - ).code - - cls = ( - _CODE_TO_CLASS.get(code) - or _STATUS_TO_DEFAULT.get(status_code) - or APIStatusError - ) + return cls( message, code=code, diff --git a/dashscope/agentstudio/streaming.py b/dashscope/agentstudio/streaming.py index bdbdc68..9dbe87d 100644 --- a/dashscope/agentstudio/streaming.py +++ b/dashscope/agentstudio/streaming.py @@ -122,7 +122,11 @@ def _produce() -> None: if item is None: return if isinstance(item, Exception): - raise item + if isinstance(item, exceptions.AgentStudioError): + raise item + raise exceptions.StreamError( + exceptions.describe_exception(item), + ) from item yield item finally: self.close() @@ -205,7 +209,11 @@ async def _produce() -> None: if item is None: return if isinstance(item, Exception): - raise item + if isinstance(item, exceptions.AgentStudioError): + raise item + raise exceptions.StreamError( + exceptions.describe_exception(item), + ) from item yield item finally: await self.aclose() diff --git a/dashscope/agentstudio/transport.py b/dashscope/agentstudio/transport.py index 9083ba4..28ed1aa 100644 --- a/dashscope/agentstudio/transport.py +++ b/dashscope/agentstudio/transport.py @@ -32,6 +32,7 @@ AGENTSTUDIO_DEFAULT_TIMEOUT, AGENTSTUDIO_MAX_RETRIES, ) +from dashscope.common.error_registry import AUTH_FAILED from dashscope.common.utils import get_sdk_headers logger = logging.getLogger("dashscope.agentstudio") @@ -223,7 +224,7 @@ def build_headers( raise exceptions.AuthenticationError( "api_key is required. Pass it via Client(api_key=...) or " "the DASHSCOPE_API_KEY environment variable.", - code="authentication_error", + status_code=AUTH_FAILED.status_code, ) headers: Dict[str, str] = { @@ -363,14 +364,18 @@ def request( # pylint: disable=too-many-branches except httpx.TimeoutException as exc: last_exc = exc if attempt >= self.max_retries: - raise exceptions.APITimeoutError(str(exc)) from exc + raise exceptions.APITimeoutError( + exceptions.describe_exception(exc), + ) from exc except Exception as exc: if isinstance(exc, exceptions.AgentStudioError): raise should_retry, _ = _should_retry_exception(exc) last_exc = exc if attempt >= self.max_retries or not should_retry: - raise exceptions.APIConnectionError(str(exc)) from exc + raise exceptions.APIConnectionError( + exceptions.describe_exception(exc), + ) from exc else: if stream: if resp.status_code >= 400: @@ -587,14 +592,18 @@ async def request( # pylint: disable=too-many-branches except httpx.TimeoutException as exc: last_exc = exc if attempt >= self.max_retries: - raise exceptions.APITimeoutError(str(exc)) from exc + raise exceptions.APITimeoutError( + exceptions.describe_exception(exc), + ) from exc except Exception as exc: if isinstance(exc, exceptions.AgentStudioError): raise should_retry, _ = _should_retry_exception(exc) last_exc = exc if attempt >= self.max_retries or not should_retry: - raise exceptions.APIConnectionError(str(exc)) from exc + raise exceptions.APIConnectionError( + exceptions.describe_exception(exc), + ) from exc else: if stream: if resp.status_code >= 400: diff --git a/dashscope/api_entities/aiohttp_request.py b/dashscope/api_entities/aiohttp_request.py index 611e72e..986bfbd 100644 --- a/dashscope/api_entities/aiohttp_request.py +++ b/dashscope/api_entities/aiohttp_request.py @@ -18,9 +18,14 @@ SSE_CONTENT_TYPE, HTTPMethod, ) -from dashscope.common.error import UnsupportedHTTPMethod from dashscope.common.logging import logger -from dashscope.common.utils import async_to_sync +from dashscope.common.error import UnsupportedHTTPMethod +from dashscope.common.error_registry import INTERNAL_ERROR +from dashscope.common.utils import ( + async_to_sync, + _handle_aiohttp_failed_response, + truncate_error_message, +) class AioHttpRequest(AioBaseRequest): @@ -88,7 +93,7 @@ def __init__( else: self.timeout = timeout # type: ignore[has-type] - def add_header(self, key, value): + def add_header(self, key: str, value: str) -> None: self.headers[key] = value def add_headers(self, headers): @@ -100,9 +105,8 @@ def call(self): return (item for item in response) else: output = next(response) - try: - next(response) - except StopIteration: + # Consume remaining items to ensure generator completes + for _ in response: pass return output @@ -112,9 +116,8 @@ async def aio_call(self): return (item async for item in response) else: result = await response.__anext__() - try: - await response.__anext__() - except StopAsyncIteration: + # Consume remaining items to ensure generator completes + async for _ in response: pass return result @@ -166,20 +169,32 @@ async def _handle_response( # pylint: disable=too-many-branches if "request_id" in msg: request_id = msg["request_id"] except json.JSONDecodeError: + error_code = INTERNAL_ERROR.error_code + error_message = data or INTERNAL_ERROR.format_msg() + logger.error( + "Request failed: status=%s, code=%s, message=%s", + response.status, + error_code, + truncate_error_message(error_message), + ) yield DashScopeAPIResponse( request_id=request_id, status_code=HTTPStatus.INTERNAL_SERVER_ERROR, - code="Unknown", - message=data, + code=error_code, + message=error_message, headers=headers, ) continue - if is_error: + if is_error and msg is not None: yield DashScopeAPIResponse( request_id=request_id, status_code=status_code, - code=msg["code"], - message=msg["message"], + code=msg.get("code") + or msg.get("error_code") + or f"http_{status_code}", + message=msg.get("message") + or msg.get("error_message") + or f"HTTP {status_code} error", headers=headers, ) else: @@ -228,45 +243,15 @@ async def _handle_response( # pylint: disable=too-many-branches headers=headers, ) else: - if "application/json" in response.content_type: - error = await response.json() - if "request_id" in error: - request_id = error["request_id"] - if "message" not in error: - message = "" - logger.error( - "Request: %s failed, status: %s", - self.url, - response.status, - ) - else: - message = error["message"] - logger.error( - "Request: %s failed, status: %s, message: %s", - self.url, - response.status, - error["message"], - ) - yield DashScopeAPIResponse( - request_id=request_id, - status_code=response.status, - code=error["code"], - message=message, - headers=headers, - ) - else: - msg = await response.read() - yield DashScopeAPIResponse( - request_id=request_id, - status_code=response.status, - code="Unknown", - message=msg.decode("utf-8"), - headers=headers, - ) + yield await _handle_aiohttp_failed_response(response) # pylint: disable=too-many-branches async def _handle_request(self): try: + # Session management: + # - External session: managed by caller, we never close it + # - Shared session: managed by get_shared_aio_session(), + # uses connection pooling and is closed when no longer needed if self._external_aio_session is not None: session = self._external_aio_session else: diff --git a/dashscope/api_entities/api_request_factory.py b/dashscope/api_entities/api_request_factory.py index df1030d..601211e 100644 --- a/dashscope/api_entities/api_request_factory.py +++ b/dashscope/api_entities/api_request_factory.py @@ -23,6 +23,78 @@ from dashscope.protocol.websocket import WebsocketStreamingMode +def _build_http_url( + base_address: str, + is_service: bool, + task_group: str, + task: str, + function: str, + extra_url_parameters: dict, + workspace: Optional[str] = None, +) -> str: + """Build HTTP/HTTPS URL from components with validation.""" + if base_address is None: + base_address = resolve_base_url( + dashscope.base_http_api_url, + workspace, + ) + + # Validate base_address has proper scheme (http:// or https://) + if base_address and not base_address.startswith( + ("http://", "https://"), + ): + from dashscope.common.error import InvalidBaseURL + + raise InvalidBaseURL( + f"Invalid URL '{base_address}': No scheme supplied. " + f"Perhaps you meant https://{base_address}?", + ) + + if not base_address.endswith("/"): + http_url = base_address + "/" + else: + http_url = base_address + + if is_service: + http_url = http_url + SERVICE_API_PATH + "/" + + if task_group: + http_url += f"{task_group}/" + if task: + http_url += f"{task}/" + if function: + http_url += function + if extra_url_parameters is not None and extra_url_parameters: + http_url += "?" + urlencode(extra_url_parameters) + + return http_url + + +def _build_websocket_url( + base_address: str, + workspace: Optional[str] = None, +) -> str: + """Build and validate WebSocket URL.""" + if base_address is not None: + websocket_url = base_address + else: + websocket_url = resolve_base_url( + dashscope.base_websocket_api_url, + workspace, + ) + + # Validate websocket_url has proper scheme (ws:// or wss://) + if websocket_url and not websocket_url.startswith(("ws://", "wss://")): + from dashscope.common.error import InvalidBaseURL + + raise InvalidBaseURL( + f"Invalid URL '{websocket_url}': No scheme supplied. " + f"Perhaps you meant wss://{websocket_url}?", + ) + + return websocket_url + + def _build_api_request( # pylint: disable=too-many-branches # pylint: disable=too-many-arguments,too-many-locals model: str, @@ -139,27 +211,15 @@ def _build_api_request( # pylint: disable=too-many-branches base_address = resolve_base_url(base_address, workspace) if api_protocol in [ApiProtocol.HTTP, ApiProtocol.HTTPS]: - if base_address is None: - base_address = resolve_base_url( - dashscope.base_http_api_url, - workspace, - ) - if not base_address.endswith("/"): - http_url = base_address + "/" - else: - http_url = base_address - - if is_service: - http_url = http_url + SERVICE_API_PATH + "/" - - if task_group: - http_url += f"{task_group}/" - if task: - http_url += f"{task}/" - if function: - http_url += function - if extra_url_parameters is not None and extra_url_parameters: - http_url += "?" + urlencode(extra_url_parameters) + http_url = _build_http_url( + base_address, + is_service, + task_group, + task, + function, + extra_url_parameters, + workspace, + ) if enable_encryption is True: encryption = Encryption() @@ -182,13 +242,7 @@ def _build_api_request( # pylint: disable=too-many-branches session=session, ) elif api_protocol == ApiProtocol.WEBSOCKET: - if base_address is not None: - websocket_url = base_address - else: - websocket_url = resolve_base_url( - dashscope.base_websocket_api_url, - workspace, - ) + websocket_url = _build_websocket_url(base_address, workspace) request = WebSocketRequest( url=websocket_url, api_key=api_key, diff --git a/dashscope/api_entities/http_request.py b/dashscope/api_entities/http_request.py index 96ee9ea..a551e82 100644 --- a/dashscope/api_entities/http_request.py +++ b/dashscope/api_entities/http_request.py @@ -24,12 +24,14 @@ HTTPMethod, ) from dashscope.common.error import UnsupportedHTTPMethod +from dashscope.common.error_registry import INTERNAL_ERROR from dashscope.common.logging import logger from dashscope.common.utils import ( _handle_aio_stream, _handle_aiohttp_failed_response, _handle_http_failed_response, _handle_stream, + truncate_error_message, ) from dashscope.api_entities.encryption import Encryption @@ -164,6 +166,36 @@ def _send_with_retry( return send() +def _sse_parse_failure_response( + status: int, + data: str, + request_id: str, + headers: dict, +) -> DashScopeAPIResponse: + """Build the 500 for an SSE body the SDK cannot parse. + + An unparsable body is a server-side defect rather than a bad request, and + the payload is the only evidence of it, so it is kept in ``message``. + + ``status`` feeds the log line only: the sync and async callers read it from + different attributes. + """ + error_message = data or INTERNAL_ERROR.format_msg() + logger.error( + "Request failed: status=%s, code=%s, message=%s", + status, + INTERNAL_ERROR.error_code, + truncate_error_message(error_message), + ) + return DashScopeAPIResponse( + request_id=request_id, + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, + code=INTERNAL_ERROR.error_code, + message=error_message, + headers=headers, + ) + + class HttpRequest(AioBaseRequest): def __init__( self, @@ -210,17 +242,12 @@ def __init__( # Auto-detect session type and store accordingly if session is not None: - session_type = type(session).__name__ - session_module = type(session).__module__ - - # Check if it's an aiohttp ClientSession - if ( - session_type == "ClientSession" and "aiohttp" in session_module - ) or isinstance(session, aiohttp.ClientSession): + if isinstance(session, aiohttp.ClientSession): self._external_session = None self._external_aio_session = session else: - # Treat as requests Session + # Used by synchronous calls only; _handle_aio_request warns if + # an async call then has to ignore it. self._external_session = session self._external_aio_session = None else: @@ -267,7 +294,7 @@ def __init__( else: self.timeout = timeout # type: ignore[has-type] - def add_header(self, key, value): + def add_header(self, key: str, value: str) -> None: self.headers[key] = value def add_headers(self, headers): @@ -279,9 +306,8 @@ def call(self): return (item for item in response) else: output = next(response) - try: - next(response) - except StopIteration: + # Consume remaining items to ensure generator completes + for _ in response: pass return output @@ -291,9 +317,8 @@ async def aio_call(self): return (item async for item in response) else: result = await response.__anext__() - try: - await response.__anext__() - except StopAsyncIteration: + # Consume remaining items to ensure generator completes + async for _ in response: pass return result @@ -305,6 +330,16 @@ async def _handle_aio_request(self): # pylint: disable=too-many-branches session = self._external_aio_session should_close = False else: + if self._external_session is not None: + # Only an aiohttp.ClientSession can be reused here, so the + # caller's custom connector or proxy would silently not + # apply to this async call. + logger.warning( + "ignoring the supplied %s session for this async " + "call; pass an aiohttp.ClientSession to reuse a " + "custom connection", + type(self._external_session).__name__, + ) session = await get_shared_aio_session() should_close = False @@ -369,7 +404,7 @@ async def _handle_aio_request(self): # pylint: disable=too-many-branches if should_close: await session.close() except Exception: - logger.exception("Request failed") + logger.exception("Async request failed") raise @staticmethod @@ -429,20 +464,23 @@ async def _handle_aio_response( # pylint: disable=too-many-branches, too-many-s if "request_id" in msg: request_id = msg["request_id"] except json.JSONDecodeError: - yield DashScopeAPIResponse( - request_id=request_id, - status_code=HTTPStatus.INTERNAL_SERVER_ERROR, - code="Unknown", - message=data, - headers=headers, + yield _sse_parse_failure_response( + response.status, + data, + request_id, + headers, ) continue if is_error: yield DashScopeAPIResponse( request_id=request_id, status_code=status_code, - code=msg["code"], - message=msg["message"], + code=msg.get("code") + or msg.get("error_code") + or f"http_{status_code}", + message=msg.get("message") + or msg.get("error_message") + or f"HTTP {status_code} error", headers=headers, ) else: @@ -538,13 +576,11 @@ def _handle_response( # pylint: disable=too-many-branches if "request_id" in msg: request_id = msg["request_id"] except json.JSONDecodeError: - yield DashScopeAPIResponse( - request_id=request_id, - status_code=HTTPStatus.BAD_REQUEST, - output=None, - code="Unknown", - message=data, - headers=headers, + yield _sse_parse_failure_response( + response.status_code, + data, + request_id, + headers, ) continue if is_error: @@ -552,10 +588,12 @@ def _handle_response( # pylint: disable=too-many-branches request_id=request_id, status_code=status_code, output=None, - code=msg["code"] - if "code" in msg - else None, # noqa E501 - message=msg["message"] if "message" in msg else None, + code=msg.get("code") + or msg.get("error_code") + or f"http_{status_code}", + message=msg.get("message") + or msg.get("error_message") + or f"HTTP {status_code} error", headers=headers, ) # noqa E501 else: @@ -600,6 +638,7 @@ def _handle_response( # pylint: disable=too-many-branches yield _handle_http_failed_response(response) def _handle_request(self): # pylint: disable=too-many-branches + session = None try: # Use external session if provided, # otherwise use shared session with connection pooling @@ -636,10 +675,14 @@ def _handle_request(self): # pylint: disable=too-many-branches timeout=self.timeout, ), ) + for rsp in self._handle_response(response): + yield rsp elif self.method == HTTPMethod.GET: params = {} if hasattr(self, "data") and self.data is not None: params = getattr(self.data, "parameters", {}) + if params: + params = self.__handle_parameters(params) response = _send_with_retry( lambda: session.get( url=self.url, @@ -648,12 +691,18 @@ def _handle_request(self): # pylint: disable=too-many-branches timeout=self.timeout, ), ) + for rsp in self._handle_response(response): + yield rsp else: raise UnsupportedHTTPMethod( f"Unsupported http method: {self.method}", ) - for rsp in self._handle_response(response): - yield rsp except Exception: - logger.exception("Request failed") + logger.exception("Sync request failed") raise + finally: + # Note: We don't close the session here because: + # - External sessions are managed by the caller + # - Shared sessions use connection pooling and are + # managed centrally by _get_shared_sync_session() + pass diff --git a/dashscope/api_entities/websocket_request.py b/dashscope/api_entities/websocket_request.py index 2358b4e..3676309 100644 --- a/dashscope/api_entities/websocket_request.py +++ b/dashscope/api_entities/websocket_request.py @@ -13,7 +13,6 @@ from dashscope.api_entities.dashscope_response import DashScopeAPIResponse from dashscope.common.constants import ( DEFAULT_REQUEST_TIMEOUT_SECONDS, - SERVICE_503_MESSAGE, WEBSOCKET_ERROR_CODE, ) from dashscope.common.error import ( @@ -21,6 +20,12 @@ UnexpectedMessageReceived, UnknownMessageReceived, ) +from dashscope.common.error_registry import ( + SERVICE_UNAVAILABLE, + AUTH_FAILED, + INTERNAL_ERROR, + PERMISSION_DENIED, +) from dashscope.common.logging import logger from dashscope.common.utils import async_to_sync from dashscope.protocol.websocket import ( @@ -36,6 +41,46 @@ ) +def _is_service_unavailable(exc: BaseException) -> bool: + """Decide whether a failed connect means the service is unavailable. + + A connect failure never receives an HTTP status, so only an explicit + phrase qualifies. Matching a bare "503" also fires on addresses such as + ``host:5030`` or ``10.50.30.1``. + """ + return "service unavailable" in str(exc).lower() + + +def _handshake_error(status): + """Map a WebSocket handshake status to the public error to report. + + ``None`` means the status is not one this SDK classifies, and the caller + re-raises rather than inventing a response. + """ + if status == HTTPStatus.UNAUTHORIZED: + return AUTH_FAILED + if status == HTTPStatus.FORBIDDEN: + # The credentials worked but lack permission. Reporting a 403 as a 401 + # sends the caller to rotate an API key that is actually valid. + return PERMISSION_DENIED + if status == HTTPStatus.SERVICE_UNAVAILABLE: + return SERVICE_UNAVAILABLE + if status == HTTPStatus.INTERNAL_SERVER_ERROR: + return INTERNAL_ERROR + return None + + +def _internal_error_message(exc: BaseException, detail: str = "") -> str: + """Compose the user-visible message for a 500 surfaced over WebSocket. + + The registry text alone says nothing about which failure happened, so the + originating detail is appended. ``detail`` wins when the caller has a more + specific string than ``str(exc)``, such as a handshake error message. + """ + suffix = detail or f"{type(exc).__name__}: {exc}" + return f"{INTERNAL_ERROR.error_msg} (SDK Internal Error: {suffix})" + + class WebSocketRequest(AioBaseRequest): def __init__( self, @@ -215,34 +260,53 @@ async def connection_handler( ) except aiohttp.ClientConnectorError as e: logger.exception(e) + if _is_service_unavailable(e): + yield DashScopeAPIResponse( + request_id=task_id if task_id else "", + status_code=SERVICE_UNAVAILABLE.status_code, + code=SERVICE_UNAVAILABLE.error_code, + message=SERVICE_UNAVAILABLE.error_msg, + ) + return + yield DashScopeAPIResponse( - request_id="", - status_code=-1, - code="ClientConnectorError", - message=str(e), + request_id=task_id if task_id else "", + status_code=INTERNAL_ERROR.status_code, + code=INTERNAL_ERROR.error_code, + message=_internal_error_message(e), ) except aiohttp.WSServerHandshakeError as e: - code = e.status - msg = e.message - if e.status in [HTTPStatus.FORBIDDEN, HTTPStatus.UNAUTHORIZED]: - msg = "Unauthorized, your api-key is invalid!" - elif e.status == HTTPStatus.SERVICE_UNAVAILABLE: - msg = SERVICE_503_MESSAGE - else: - pass + original_msg = e.message or "" + handshake_error = _handshake_error(e.status) + + if handshake_error is None: + # Log unexpected status codes for debugging + logger.warning( + "WebSocket handshake failed with unexpected " + "status %s: %s", + e.status, + original_msg, + ) + raise e + + message = ( + _internal_error_message(e, original_msg) + if handshake_error is INTERNAL_ERROR + else handshake_error.error_msg + ) yield DashScopeAPIResponse( - request_id=task_id, - status_code=code, - code=code, - message=msg, + request_id=task_id if task_id else "", + status_code=handshake_error.status_code, + code=handshake_error.error_code, + message=message, ) except Exception as e: logger.exception(e) yield DashScopeAPIResponse( - request_id="", - status_code=-1, - code="Unknown", - message=f"Error type: {type(e)}, message: {e}", + request_id=task_id if task_id else "", + status_code=INTERNAL_ERROR.status_code, + code=INTERNAL_ERROR.error_code, + message=_internal_error_message(e), ) def _to_DashScopeAPIResponse(self, task_id, is_binary, result): diff --git a/dashscope/audio/qwen_tts_realtime/qwen_tts_realtime.py b/dashscope/audio/qwen_tts_realtime/qwen_tts_realtime.py index 31eb038..a5df826 100644 --- a/dashscope/audio/qwen_tts_realtime/qwen_tts_realtime.py +++ b/dashscope/audio/qwen_tts_realtime/qwen_tts_realtime.py @@ -10,7 +10,7 @@ import websocket # pylint: disable=wrong-import-order import dashscope -from dashscope.common.error import ModelRequired +from dashscope.common.error import AuthenticationError, ModelRequired from dashscope.common.logging import logger from dashscope.common.utils import get_sdk_headers, get_user_agent @@ -105,6 +105,8 @@ def __init__( self.last_first_text_time = None self.last_first_audio_delay = None self.metrics = [] + self._auth_failed = False # Flag to track authentication failures + self._connection_error = None # Store connection error details def _generate_event_id(self): """ @@ -132,6 +134,10 @@ def connect(self) -> None: """ connect to server, create session and return default session configuration # noqa: E501 """ + # Reset error flags before connecting + self._auth_failed = False + self._connection_error = None + self.ws = websocket.WebSocketApp( self.url, header=self._get_websocket_header(), @@ -148,7 +154,16 @@ def connect(self) -> None: not (self.ws.sock and self.ws.sock.connected) and (time.time() - start_time) < timeout ): + # Check for authentication errors first + if self._auth_failed and self._connection_error is not None: + raise self._connection_error + time.sleep(0.1) # Brief sleep to avoid busy polling + + # Final check after timeout + if self._auth_failed and self._connection_error is not None: + raise self._connection_error + if not (self.ws.sock and self.ws.sock.connected): raise TimeoutError( "websocket connection could not established within 5s. " @@ -416,6 +431,20 @@ def on_close( # pylint: disable=unused-argument # Callback for WebSocket error def on_error(self, ws, error): # pylint: disable=unused-argument logger.error(f"websocket error: {error}") + + # Detect authentication errors from error message + error_str = str(error) + if ( + "401" in error_str + or "Unauthorized" in error_str + or "InvalidApiKey" in error_str + ): + self._auth_failed = True + self._connection_error = AuthenticationError( + "API Key is invalid. Please check your API key configuration.", + ) + logger.error("Authentication failed: Invalid API Key") + # Do not raise exception here, let the connection close naturally # Raising exception in callback can cause unexpected thread termination diff --git a/dashscope/cli/agentic_rl.py b/dashscope/cli/agentic_rl.py index f850f71..1d44beb 100644 --- a/dashscope/cli/agentic_rl.py +++ b/dashscope/cli/agentic_rl.py @@ -7,7 +7,6 @@ import asyncio import json import logging -import traceback as tb_module from pathlib import Path from typing import Optional, List, Dict, Any @@ -24,7 +23,12 @@ ) from dashscope.finetune.reinforcement.common.errors import OutputError from dashscope.finetune.customize_types import FineTune -from dashscope.cli.common import error, normalize_local_path_or_url +from dashscope.cli.common import ( + error, + normalize_local_path_or_url, + set_verbose_errors, + _handle_exception, +) app = typer.Typer( @@ -49,25 +53,14 @@ def callback(ctx: typer.Context): typer.echo(ctx.get_help()) -_cli_verbose = False - - def _apply_verbose(verbose: bool): - global _cli_verbose - _cli_verbose = verbose + set_verbose_errors(verbose) if not verbose: from dashscope.finetune.reinforcement.common.log import logger logger.setLevel(logging.WARNING) -def _root_cause(e: Exception) -> Exception: - root = e - while root.__cause__: - root = root.__cause__ - return root - - # ================= Configuration & Utility Functions ================= def validate_output_format(fmt: str) -> str: """Validate and normalize output format.""" @@ -215,9 +208,8 @@ async def _register_fc_async( } except Exception as e: - root = _root_cause(e) - err_console.print(f"[red]❌ FC registration failed: {root}[/red]") - raise typer.Exit(1) + _handle_exception(e, "FC registration failed", err_console) + raise # pragma: no cover @app.command("register-functions", hidden=True) @@ -292,9 +284,8 @@ async def _test_fc_async( return result except Exception as e: - root = _root_cause(e) - err_console.print(f"[red]❌ Function test failed: {root}[/red]") - raise typer.Exit(1) + _handle_exception(e, "Function test failed", err_console) + raise # pragma: no cover @app.command("test-functions", hidden=True) @@ -370,9 +361,7 @@ async def _upload_data_async( } except Exception as e: - root = _root_cause(e) - err_console.print(f"[red]❌ Dataset upload failed: {root}[/red]") - raise typer.Exit(1) + _handle_exception(e, "Dataset upload failed", err_console) @app.command("upload-data", hidden=True) @@ -441,15 +430,13 @@ async def _run_workflow_async( Raises: ValueError: If required parameters are missing - RuntimeError: If workflow execution fails + DashScopeException: If workflow execution fails. The SDK already + classifies it with the registry's status_code/error_code, so it + must propagate unwrapped for the CLI to display both. """ - try: - client = AgenticRL(api_key=api_key or "") - client.init(config_path=config_path, **run_kwargs) - result = await client.run() - return result - except Exception as e: - raise RuntimeError("Workflow execution failed") from e + client = AgenticRL(api_key=api_key or "") + client.init(config_path=config_path, **run_kwargs) + return await client.run() @app.command() @@ -543,8 +530,16 @@ def run( # Handle API response errors if result.status_code != 200: raise OutputError( - f"API returned status {result.status_code}:" - f" {result.message}", + ( + f"API error [status={result.status_code}, " + f"code={result.code}]: {result.message}" + ), + response={ + "status_code": result.status_code, + "code": result.code, + "message": result.message, + "request_id": result.request_id, + }, ) progress.update( @@ -566,24 +561,12 @@ def run( ) except Exception as e: - root = _root_cause(e) label = ( "Validation error" if isinstance(e, ValueError) else "Workflow execution failed" ) - err_console.print(f"[red]❌ {label}: {root}[/red]") - if _cli_verbose: - err_console.print( - "".join( - tb_module.format_exception( - type(root), - root, - root.__traceback__, - ), - ), - ) - raise typer.Exit(1) + _handle_exception(e, label, err_console) @app.command() @@ -605,7 +588,16 @@ def get( # Handle API response errors if result.status_code != 200: raise OutputError( - f"API returned status {result.status_code}: {result.message}", + ( + f"API error [status={result.status_code}, " + f"code={result.code}]: {result.message}" + ), + response={ + "status_code": result.status_code, + "code": result.code, + "message": result.message, + "request_id": result.request_id, + }, ) # Validate output is not None before accessing attributes @@ -621,9 +613,7 @@ def get( fmt=output_format, ) except Exception as e: - root = _root_cause(e) - err_console.print(f"[red]❌ Query failed: {root}[/red]") - raise typer.Exit(1) + _handle_exception(e, "Query failed", err_console) @app.command() @@ -643,16 +633,23 @@ def cancel( # Handle API response errors if result.status_code != 200: raise OutputError( - f"API returned status {result.status_code}: {result.message}", + ( + f"API error [status={result.status_code}, " + f"code={result.code}]: {result.message}" + ), + response={ + "status_code": result.status_code, + "code": result.code, + "message": result.message, + "request_id": result.request_id, + }, ) err_console.print( f"[green]✅ Job {job_id} canceled successfully[/green]", ) except Exception as e: - root = _root_cause(e) - err_console.print(f"[red]❌ Cancellation failed: {root}[/red]") - raise typer.Exit(1) + _handle_exception(e, "Cancellation failed", err_console) @app.command() @@ -681,22 +678,32 @@ def logs( # Handle API response errors if result.status_code != 200: raise OutputError( - f"API returned status {result.status_code}: {result.message}", + ( + f"API error [status={result.status_code}, " + f"code={result.code}]: {result.message}" + ), + response={ + "status_code": result.status_code, + "code": result.code, + "message": result.message, + "request_id": result.request_id, + }, ) # Validate output is not None before accessing attributes logs_data = "" - if result.output is not None and isinstance(result.output, dict): - logs_data = result.output.get("logs", "") + if result.output is not None: + if isinstance(result.output, dict): + logs_data = result.output.get("logs", "") + else: + logs_data = getattr(result.output, "logs", "") format_output( {"job_id": job_id, "logs": logs_data}, fmt=output_format, ) except Exception as e: - root = _root_cause(e) - err_console.print(f"[red]❌ Log retrieval failed: {root}[/red]") - raise typer.Exit(1) + _handle_exception(e, "Log retrieval failed", err_console) @app.command("list") @@ -723,7 +730,16 @@ def list_jobs( # Handle API response errors if result.status_code != 200: raise OutputError( - f"API returned status {result.status_code}: {result.message}", + ( + f"API error [status={result.status_code}, " + f"code={result.code}]: {result.message}" + ), + response={ + "status_code": result.status_code, + "code": result.code, + "message": result.message, + "request_id": result.request_id, + }, ) output_data = serialize_for_output( @@ -731,9 +747,7 @@ def list_jobs( ) format_output(output_data, fmt=output_format) except Exception as e: - root = _root_cause(e) - err_console.print(f"[red]❌ List query failed: {root}[/red]") - raise typer.Exit(1) + _handle_exception(e, "List query failed", err_console) # if __name__ == "__main__": diff --git a/dashscope/cli/common.py b/dashscope/cli/common.py index 68f0eea..1e01ded 100644 --- a/dashscope/cli/common.py +++ b/dashscope/cli/common.py @@ -2,14 +2,17 @@ """Shared utilities, constants, and helpers for the dashscope CLI.""" import logging import os +import traceback from functools import wraps from http import HTTPStatus -from typing import Callable, NoReturn, TypeVar +from typing import Callable, Dict, NoReturn, TypeVar from urllib.parse import urlparse import typer from rich.console import Console +from dashscope.common.error import DashScopeException + logger = logging.getLogger("dashscope.cli") CommandFunction = TypeVar("CommandFunction", bound=Callable) @@ -21,6 +24,84 @@ DEFAULT_PAGE_SIZE = 10 DEFAULT_START_PAGE = 1 +# --------------------------------------------------------------------------- +# Error message templates, keyed by the server-side error code +# --------------------------------------------------------------------------- +ERROR_MESSAGE_TEMPLATES: Dict[str, str] = { + "AuthFailed": ( + "Authentication failed. " "Please check your API key and try again." + ), + "InvalidToken": ( + "Authentication failed. " "Please check your API key and try again." + ), + "TokenExpired": ( + "Authentication failed. " "Please check your API key and try again." + ), + "Unauthorized": ( + "Authentication failed. " "Please check your API key and try again." + ), + "InvalidParameter": ( + "Invalid parameter provided. " "Please check your input parameters." + ), + "InvalidParam": ( + "Invalid parameter provided. " "Please check your input parameters." + ), + "ModelRequired": ( + "Model parameter is required. " "Please specify a valid model." + ), + "InvalidModel": ( + "Invalid model specified. " "Please check the model name." + ), + "InvalidInput": ("Invalid input data. " "Please check your input format."), + "InvalidFileFormat": ( + "Invalid file format. " "Please check the file type." + ), + "InputDataRequired": ( + "Input data is required. " "Please provide the necessary input." + ), + "InputRequired": ( + "Input is required. " "Please provide the necessary input." + ), + "UnsupportedDataType": ( + "Unsupported data type. " "Please check the data format." + ), + "InvalidTask": ("Invalid task specified. " "Please check the task type."), + "UnsupportedTask": ( + "Unsupported task type. " "Please check the available tasks." + ), + "UnsupportedModel": ( + "Unsupported model. " "Please check the available models." + ), + "UnsupportedApiProtocol": ( + "Unsupported API protocol. " "Please check the protocol version." + ), + "NotImplemented": "This feature is not yet implemented.", + "MultiInputsWithBinaryNotSupported": ( + "Binary input is not supported with multiple inputs." + ), + "UnexpectedMessageReceived": ( + "Unexpected message received from the server." + ), + "UnsupportedData": "Unsupported data format.", + "UnknownMessageReceived": ("Unknown message received from the server."), + "ServiceUnavailableError": ( + "Service is temporarily unavailable. " "Please try again later." + ), + "UnsupportedHTTPMethod": ( + "Unsupported HTTP method. " "Please check the request method." + ), + "AsyncTaskCreateFailed": ( + "Failed to create async task. " "Please check your request." + ), + "UploadFileException": ( + "File upload failed. " "Please check the file and try again." + ), + "TimeoutException": "Request timed out. Please try again.", + "AssistantError": ( + "Assistant encountered an error. " "Please check the error details." + ), +} + # --------------------------------------------------------------------------- # Rich consoles # --------------------------------------------------------------------------- @@ -28,19 +109,176 @@ err_console = Console(stderr=True) # --------------------------------------------------------------------------- -# Response helpers +# Error handling utilities # --------------------------------------------------------------------------- -def print_failed_message(rsp): - """Print a standardised error message for a failed API response.""" - err_console.print( - f"[red]Failed[/red] request_id: {rsp.request_id}, " - f"status_code: {rsp.status_code}, " - f"code: {rsp.code}, message: {rsp.message}", +def _get_cli_error_message(sdk_error_code: str, sdk_error_message: str) -> str: + """Get CLI-friendly error message with context. + + Args: + sdk_error_code: The error code from the SDK response + sdk_error_message: The error message from the SDK response + + Returns: + CLI-friendly error message + """ + # Use template message if available, otherwise use SDK message + template_message = ERROR_MESSAGE_TEMPLATES.get(sdk_error_code) + if template_message: + # Append SDK-specific message if available + if sdk_error_message: + return f"{sdk_error_message}" + return template_message + + # No template available, use SDK message directly + return ( + sdk_error_message + if sdk_error_message + else f"Error code: {sdk_error_code}" + ) + + +def _error_code_of(exception: Exception) -> str: + """Return the error code to display for *exception*. + + Exceptions from the RL/agent modules carry a registry-namespaced + ``error_code``; the core ``DashScopeException`` subclasses do not, so they + fall back to the class name. + """ + return getattr(exception, "error_code", None) or type(exception).__name__ + + +def _request_id_of(exception: Exception) -> str: + """Return the request id for *exception*, or ``"N/A"`` when unknown. + + Some call sites attach the failed response as a dict on ``exception`` + rather than setting ``request_id`` directly. + """ + request_id = getattr(exception, "request_id", None) + if not request_id: + response = getattr(exception, "response", None) + if isinstance(response, dict): + request_id = response.get("request_id") + return request_id or "N/A" + + +def _is_structured_error(exception: Exception) -> bool: + """Whether *exception* can be reported with a code and request id.""" + return isinstance(exception, DashScopeException) or hasattr( + exception, + "error_code", ) +_verbose_errors = False + + +def set_verbose_errors(enabled: bool) -> None: + """Make the CLI error handlers print a traceback on failure.""" + global _verbose_errors + _verbose_errors = bool(enabled) + + +def _format_error_parts( + request_id: str, + status_code: str, + error_code: str, + error_message: str, + command_name: str = None, +) -> str: + """Build formatted error output parts. + + Args: + request_id: The request ID from the response + status_code: The HTTP status code + error_code: The error code to display + error_message: The error message to display + command_name: The CLI command name (optional) + + Returns: + Formatted error message string + """ + parts = [] + if command_name: + parts.append(f"[red]{command_name} failed[/red]") + else: + parts.append("[red]Request failed[/red]") + + if request_id and request_id != "N/A": + parts.append(f"request_id: {request_id}") + if status_code and status_code != "N/A": + parts.append(f"status_code: {status_code}") + parts.append(f"code: {error_code}") + parts.append(f"message: {error_message}") + + return ", ".join(parts) + + +# --------------------------------------------------------------------------- +# Response helpers +# --------------------------------------------------------------------------- + + +def print_failed_message(rsp, command_name: str = None): + """Print a standardised error message for a failed API response. + + Renders the server-side error code as-is and enhances the message with + CLI context. Safely handles responses with missing or None attributes. + + Args: + rsp: The API response object + command_name: The CLI command name (optional, for better context) + """ + # Use try-except to handle missing attributes gracefully (works with Mock + # objects) + try: + request_id = rsp.request_id + except AttributeError: + request_id = None + + try: + status_code = rsp.status_code + except AttributeError: + status_code = None + + try: + code = rsp.code + except AttributeError: + code = None + + try: + message = rsp.message + except AttributeError: + message = None + + # Normalize None and empty strings + request_id = request_id if request_id else "N/A" + status_code = status_code if status_code is not None else "N/A" + code = code if code else "" + message = message if message else "" + + if code: + formatted_error = _format_error_parts( + request_id=request_id, + status_code=status_code, + error_code=code, + error_message=_get_cli_error_message(code, message), + command_name=command_name, + ) + err_console.print(formatted_error) + else: + # Fallback for responses without error code + parts = ["[red]Failed[/red]"] + if request_id != "N/A": + parts.append(f"request_id: {request_id}") + if status_code != "N/A": + parts.append(f"status_code: {status_code}") + if message: + parts.append(f"message: {message}") + err_console.print(", ".join(parts)) + + def _exit_code_for(rsp) -> int: """Map an API response to a structured CLI exit code. @@ -73,16 +311,20 @@ def _exit_code_for(rsp) -> int: return 1 -def ensure_ok(rsp, check_business_error: bool = True): +def ensure_ok( + rsp, + check_business_error: bool = True, + command_name: str = None, +): """Return *rsp.output* when the response is OK; otherwise print the error - and exit with code 1. + and exit with the tiered code from :func:`_exit_code_for`. This eliminates the repetitive ``if rsp.status_code == OK … else …`` pattern that appears in every command handler. Enhanced to check both HTTP status and business-level error codes: - HTTP 200 but InvalidParameter → still treated as failure - - HTTP 4xx/5xx → clear error message + - HTTP 4xx/5xx → clear error message with CLI context Args: rsp: The API response object @@ -90,15 +332,22 @@ def ensure_ok(rsp, check_business_error: bool = True): error codes in the output. Set to False for async task creation where we only care about HTTP success, not task execution. + command_name: The CLI command name (optional, for better context) """ + # Check HTTP status first if rsp.status_code != HTTPStatus.OK: - print_failed_message(rsp) + print_failed_message(rsp, command_name=command_name) raise typer.Exit(_exit_code_for(rsp)) - # Check for business-level errors even when HTTP status is 200 + # Check if output exists output = rsp.output if output is None: - print_failed_message(rsp) + # HTTP 200 but no output - this is unusual, treat as error + err_console.print( + f"[red]Error[/red] " + f"request_id: {getattr(rsp, 'request_id', 'N/A')}, " + f"HTTP 200 but response has no output data", + ) raise typer.Exit(_exit_code_for(rsp)) # Only check business-level errors if explicitly requested @@ -106,18 +355,26 @@ def ensure_ok(rsp, check_business_error: bool = True): # Some APIs return error info in output even with HTTP 200 if isinstance(output, dict): error_code = output.get("code") - message = output.get("message", "Unknown error") + message = output.get("message") else: error_code = getattr(output, "code", None) - message = getattr(output, "message", "Unknown error") - - if error_code and error_code != "": - err_console.print( - f"[red]Business Error[/red] request_id: {rsp.request_id}, " - f"status_code: {rsp.status_code}, " - f"code: {error_code}, " - f"message: {message}", + message = getattr(output, "message", None) + + # Only report if there's an actual error code + if error_code: + request_id = getattr(rsp, "request_id", "N/A") + + formatted_error = _format_error_parts( + request_id=request_id, + status_code=str(rsp.status_code), + error_code=error_code, + error_message=_get_cli_error_message( + error_code, + message or "API returned error code without message", + ), + command_name=command_name, ) + err_console.print(formatted_error) # reuse rsp with the business code for exit-code mapping class _BizRsp: @@ -158,8 +415,60 @@ def error(message: str, exit_code: int = 1) -> NoReturn: raise typer.Exit(exit_code) +def _handle_exception( + exception: Exception, + action: str, + output_console: Console, +) -> NoReturn: + """Print a friendly error message for *exception*, then exit with code 1. + + Args: + exception: The exception to handle. + action: The action that failed (e.g., "FC registration failed"). + output_console: The Rich console to print to. + """ + if _is_structured_error(exception): + error_code = _error_code_of(exception) + message = getattr(exception, "message", None) or str(exception) + + output_console.print( + f"[red]{action}[/red] " + f"(request_id: {_request_id_of(exception)}, code: {error_code})\n" + f" {_get_cli_error_message(error_code, message)}", + no_wrap=True, + ) + logger.debug( + f"{action} failed with {type(exception).__name__}", + exc_info=True, + ) + else: + # Handle unexpected exceptions with full context + output_console.print(f"[red]{action}:[/red] {exception}") + logger.debug( + f"{action} failed with unexpected exception", + exc_info=True, + ) + + if _verbose_errors: + output_console.print( + "".join( + traceback.format_exception( + type(exception), + exception, + exception.__traceback__, + ), + ), + ) + + raise typer.Exit(1) from exception + + def handle_sdk_error(action: str): - """Convert unexpected SDK exceptions into friendly CLI errors.""" + """Convert unexpected SDK exceptions into friendly CLI errors. + + Delegates to :func:`_handle_exception`, so command handlers get the same + reporting as the code that catches exceptions itself. + """ def decorator(command_function: CommandFunction) -> CommandFunction: @wraps(command_function) @@ -167,9 +476,10 @@ def wrapper(*args, **kwargs): try: return command_function(*args, **kwargs) except typer.Exit: + # Re-raise intentional exits without modification raise except Exception as exception: - error(f"{action}: {exception}") + _handle_exception(exception, action, err_console) return wrapper # type: ignore[return-value] diff --git a/dashscope/cli/deployments.py b/dashscope/cli/deployments.py index 6fffe1e..9b2e7a7 100644 --- a/dashscope/cli/deployments.py +++ b/dashscope/cli/deployments.py @@ -102,7 +102,24 @@ def _print_deployments(output): ): console.print("There is no deployed model!") return - for dep in output.deployments: + + # Support both dictionary and object formats + if isinstance(output, dict): + raw_deps = output.get("deployments", []) + from types import SimpleNamespace + + deployments = [ + SimpleNamespace(**d) if isinstance(d, dict) else d + for d in raw_deps + ] + else: + deployments = getattr(output, "deployments", []) + + if not deployments: + console.print("There is no deployed model!") + return + + for dep in deployments: console.print( f"Deployed_model: {dep.deployed_model}, " f"model: {dep.model_name}, " diff --git a/dashscope/cli/fine_tunes.py b/dashscope/cli/fine_tunes.py index d531752..9e28e73 100644 --- a/dashscope/cli/fine_tunes.py +++ b/dashscope/cli/fine_tunes.py @@ -115,11 +115,11 @@ def _stream_events(job_id: str): ) return - status = ( - rsp.output.get("status") - if isinstance(rsp.output, dict) - else getattr(rsp.output, "status", None) - ) + # Support both dictionary and object formats + if isinstance(rsp.output, dict): + status = rsp.output.get("status") + else: + status = getattr(rsp.output, "status", None) if status in ( TaskStatus.FAILED, TaskStatus.CANCELED, @@ -252,7 +252,11 @@ def create( if output is None: error("Fine-tune creation returned empty response") - job_id = getattr(output, "job_id", None) + job_id = ( + output.get("job_id") + if isinstance(output, dict) + else getattr(output, "job_id", None) + ) if not job_id: error( "Fine-tune creation succeeded but missing job_id in response. " diff --git a/dashscope/cli/generation.py b/dashscope/cli/generation.py index 2305dae..41f909f 100644 --- a/dashscope/cli/generation.py +++ b/dashscope/cli/generation.py @@ -9,7 +9,7 @@ from dashscope.aigc import Generation from dashscope.cli.common import ( - error, + err_console, extract_text, handle_sdk_error, print_failed_message, @@ -44,7 +44,9 @@ def _build_generation_kwargs( try: kwargs["messages"] = json.loads(messages) except json.JSONDecodeError as exc: - error("--messages must be a valid JSON string") + err_console.print( + "[red]Error:[/red] --messages must be a valid JSON string", + ) raise typer.Exit(1) from exc # Group simple parameters to reduce branches diff --git a/dashscope/client/base_api.py b/dashscope/client/base_api.py index 8ae1817..6fa44dc 100644 --- a/dashscope/client/base_api.py +++ b/dashscope/client/base_api.py @@ -22,6 +22,7 @@ HTTPMethod, ) from dashscope.common.error import InvalidParameter, InvalidTask, ModelRequired +from dashscope.common.error_registry import INTERNAL_ERROR from dashscope.common.logging import logger from dashscope.common.env import resolve_base_url from dashscope.common.utils import ( @@ -1623,12 +1624,25 @@ def _handle_response(cls, response: requests.Response): ): for is_error, status_code, data in cls._handle_stream(response): if is_error: + code = None + message = None + try: + error_data = json.loads(data) + code = error_data.get("code") or error_data.get( + "error_code", + ) + message = error_data.get("message") or error_data.get( + "error_message", + ) + except json.JSONDecodeError: + # Unparseable payload: keep the raw text as detail. + message = data yield DashScopeAPIResponse( request_id=request_id, status_code=status_code, output=None, - code="", - message="", + code=code or INTERNAL_ERROR.error_code, + message=message or INTERNAL_ERROR.format_msg(), ) # noqa E501 else: yield DashScopeAPIResponse( diff --git a/dashscope/common/constants.py b/dashscope/common/constants.py index 6fbecd9..f79634a 100644 --- a/dashscope/common/constants.py +++ b/dashscope/common/constants.py @@ -32,7 +32,6 @@ REFERENCE_URLS = "reference_urls" MEDIA_URLS = "media" TEXT_EMBEDDING_INPUT_KEY = "texts" -SERVICE_503_MESSAGE = "Service temporarily unavailable, possibly overloaded or not ready." # noqa E501 # pylint: disable=line-too-long WEBSOCKET_ERROR_CODE = 44 SSE_CONTENT_TYPE = "text/event-stream" DEPRECATED_MESSAGE = "history and auto_history are deprecated for qwen serial models and will be remove in future, use messages" # noqa E501 # pylint: disable=line-too-long diff --git a/dashscope/common/error.py b/dashscope/common/error.py index 2b49cc7..95c1735 100644 --- a/dashscope/common/error.py +++ b/dashscope/common/error.py @@ -144,3 +144,10 @@ class UploadFileException(DashScopeException): class TimeoutException(DashScopeException): pass + + +class InvalidBaseURL(DashScopeException): + """Raised when the base_address is invalid. + + This can happen when the scheme is missing or the URL is malformed. + """ diff --git a/dashscope/common/error_registry.py b/dashscope/common/error_registry.py new file mode 100644 index 0000000..0946e2e --- /dev/null +++ b/dashscope/common/error_registry.py @@ -0,0 +1,643 @@ +# -*- coding: utf-8 -*- +from __future__ import annotations +from dataclasses import dataclass, field +from typing import Dict, List + +__version__ = "1.0.5" +__commit__ = "ededf0f" +__snapshot__ = False + + +@dataclass(frozen=True) +class PublicError: + key: str + status_code: int + error_code: str + error_msg: str + anthropic_error_code: str + + def format_msg(self, substitutions: Dict[str, str] = None) -> str: + msg = self.error_msg + if substitutions: + for k, v in substitutions.items(): + msg = msg.replace("{" + k + "}", v) + return msg + + +@dataclass(frozen=True) +class SdkErrorDef: + name: str + message: str + solution: str + allow_retry: bool = False + vars: List[str] = field(default_factory=list) + + def format_message(self, variables: Dict[str, str] = None) -> str: + msg = self.message + if variables: + for k, v in variables.items(): + msg = msg.replace("{" + k + "}", v) + return msg + + +# -- Public Errors -------------------------------------------------- + +INVALID_REQUEST = PublicError( + key="invalid_request", + status_code=400, + error_code="BadRequestError", + error_msg=( + "The request is invalid. Please check the request and try " "again." + ), + anthropic_error_code="invalid_request_error", +) + +MISSING_PARAMETER = PublicError( + key="missing_parameter", + status_code=400, + error_code="BadRequestError", + error_msg="Missing required parameter: {parameter}.", + anthropic_error_code="invalid_request_error", +) + +CONTENT_POLICY_VIOLATION = PublicError( + key="content_policy_violation", + status_code=400, + error_code="BadRequestError", + error_msg="The request was rejected by content policy.", + anthropic_error_code="invalid_request_error", +) + +INVALID_URL = PublicError( + key="invalid_url", + status_code=400, + error_code="BadRequestError", + error_msg="The provided URL is invalid or cannot be accessed.", + anthropic_error_code="invalid_request_error", +) + +INVALID_FILE = PublicError( + key="invalid_file", + status_code=400, + error_code="BadRequestError", + error_msg="The provided file is invalid.", + anthropic_error_code="invalid_request_error", +) + +AUTH_FAILED = PublicError( + key="auth_failed", + status_code=401, + error_code="AuthenticationError", + error_msg=( + "Authentication failed. Please provide valid " + "authentication credentials." + ), + anthropic_error_code="authentication_error", +) + +INVALID_API_KEY = PublicError( + key="invalid_api_key", + status_code=401, + error_code="AuthenticationError", + error_msg="Incorrect API key provided.", + anthropic_error_code="authentication_error", +) + +PERMISSION_DENIED = PublicError( + key="permission_denied", + status_code=403, + error_code="PermissionDeniedError", + error_msg="You do not have permission to access this resource.", + anthropic_error_code="permission_error", +) + +RESOURCE_NOT_FOUND = PublicError( + key="resource_not_found", + status_code=404, + error_code="NotFoundError", + error_msg="The requested resource was not found: {resource}.", + anthropic_error_code="not_found_error", +) + +REQUEST_TOO_LARGE = PublicError( + key="request_too_large", + status_code=413, + error_code="RequestTooLargeError", + error_msg="The request exceeds the maximum allowed size of {limit}.", + anthropic_error_code="request_too_large", +) + +RATE_LIMIT_EXCEEDED = PublicError( + key="rate_limit_exceeded", + status_code=429, + error_code="RateLimitError", + error_msg="Rate limit exceeded. Please slow down your requests.", + anthropic_error_code="rate_limit_error", +) + +CONCURRENCY_LIMIT_EXCEEDED = PublicError( + key="concurrency_limit_exceeded", + status_code=429, + error_code="RateLimitError", + error_msg=( + "Too many concurrent requests. Please reduce concurrency " + "and try again." + ), + anthropic_error_code="rate_limit_error", +) + +INSUFFICIENT_QUOTA = PublicError( + key="insufficient_quota", + status_code=429, + error_code="RateLimitError", + error_msg=( + "You exceeded your current quota. Please check your plan " + "and billing details." + ), + anthropic_error_code="billing_error", +) + +INTERNAL_ERROR = PublicError( + key="internal_error", + status_code=500, + error_code="InternalServerError", + error_msg=( + "The server encountered an internal error. Please try again " "later." + ), + anthropic_error_code="api_error", +) + +SERVICE_UNAVAILABLE = PublicError( + key="service_unavailable", + status_code=503, + error_code="ServiceUnavailableError", + error_msg=( + "The service is temporarily unavailable. Please try again " "later." + ), + anthropic_error_code="overloaded_error", +) + +REQUEST_TIMEOUT = PublicError( + key="request_timeout", + status_code=504, + error_code="GatewayTimeoutError", + error_msg="The request timed out. Please try again later.", + anthropic_error_code="timeout_error", +) + + +# -- SDK Errors ------------------------------------------------- + +SDK_INVALID_API_KEY = SdkErrorDef( + name="sdk.InvalidApiKey", + message="Invalid API key configuration for client.", + solution=( + "Set the DASHSCOPE_API_KEY environment variable or pass a " + "valid api_key to AgenticRL(). You can obtain an API key at " + "https://modelstudio.console.alibabacloud.com/cn-beijing" + "?tab=api." + ), + allow_retry=False, + vars=[], +) + +SDK_AGENTIC_RL_FUNCTION_REGISTRATION_FAILED = SdkErrorDef( + name="sdk.agentic_rl.FunctionRegistrationFailed", + message="Function component registration failed: {inner_code}.", + solution=( + "Check the inner error code for details. Ensure the function " + "code package is valid and the deployment service is " + "reachable. Retry the register_functions() call." + ), + allow_retry=True, + vars=["inner_code"], +) + +SDK_AGENTIC_RL_DATASETS_UPLOAD_FAILED = SdkErrorDef( + name="sdk.agentic_rl.DatasetsUploadFailed", + message="Datasets upload failed: {inner_code}.", + solution=( + "Verify your dataset files exist and are readable. Check OSS " + "credentials and network connectivity. Retry the " + "upload_datasets() call." + ), + allow_retry=True, + vars=["inner_code"], +) + +SDK_AGENTIC_RL_DUPLICATE_FUNCTION_NAMES = SdkErrorDef( + name="sdk.agentic_rl.DuplicateFunctionNames", + message="Duplicate function names detected: {names}.", + solution=( + "Each registered function must have a unique name. Review " + "your RolloutFunctionComponent and " + "RewardFunctionComponent definitions and remove any " + "duplicate names before calling submit_job()." + ), + allow_retry=False, + vars=["names"], +) + +SDK_AGENTIC_RL_JOB_SUBMISSION_FAILED = SdkErrorDef( + name="sdk.agentic_rl.JobSubmissionFailed", + message="Job submission failed: {inner_code}.", + solution=( + "Check the inner error code for details. Ensure the model " + "name is valid and your account has sufficient quota. Retry " + "submit_job() after fixing the underlying issue." + ), + allow_retry=True, + vars=["inner_code"], +) + +SDK_AGENTIC_RL_WORKFLOW_FAILED = SdkErrorDef( + name="sdk.agentic_rl.WorkflowFailed", + message="RL tuning workflow failed: {inner_code}.", + solution=( + "This error wraps a failure in the run() workflow. Inspect " + "the inner error code to identify which step failed " + "(registration, upload, or submission), fix that issue, " + "and retry run()." + ), + allow_retry=True, + vars=["inner_code"], +) + +SDK_AGENTIC_RL_UNSUPPORTED_FUNCTION_TYPE = SdkErrorDef( + name="sdk.agentic_rl.UnsupportedFunctionType", + message="Unsupported function type: {functype}.", + solution=( + "Only ROLLOUT, REWARD, and GROUP_REWARD function types are " + "supported. Pass a valid FunctionType enum value to " + "test_functions()." + ), + allow_retry=False, + vars=["functype"], +) + +SDK_AGENTIC_RL_FUNCTION_TEST_FAILED = SdkErrorDef( + name="sdk.agentic_rl.FunctionTestFailed", + message="Function test failed: {inner_code}.", + solution=( + "Check the inner error code and the function logs for " + "details. Verify that the function code runs correctly with " + "the provided input_data. Fix the function and retry " + "test_functions()." + ), + allow_retry=True, + vars=["inner_code"], +) + +SDK_AGENTIC_RL_FUNCTION_TEST_TIMEOUT = SdkErrorDef( + name="sdk.agentic_rl.FunctionTestTimeout", + message="Function test timed out after {timeout} seconds.", + solution=( + "The function did not return within the timeout period. " + "Profile your function for slow operations (e.g. large " + "model inference, network calls) and optimize it. Consider " + "increasing the timeout if the function legitimately takes " + "longer." + ), + allow_retry=True, + vars=["timeout"], +) + +SDK_AGENTIC_RL_INPUT_ERROR = SdkErrorDef( + name="sdk.agentic_rl.InputError", + message="Invalid input data detected during validation.", + solution=( + "Check the input data passed to the function. Ensure all " + "required fields are present and have valid types. Refer to " + "the RolloutInput / RewardInput / GroupRewardInput schema " + "documentation." + ), + allow_retry=False, + vars=[], +) + +SDK_AGENTIC_RL_OUTPUT_ERROR = SdkErrorDef( + name="sdk.agentic_rl.OutputError", + message="Service response failed output validation checks.", + solution=( + "The function output did not pass validation. Ensure your " + "function returns a valid RolloutOutput / RewardOutput / " + "GroupRewardOutput object with all required fields " + "populated." + ), + allow_retry=False, + vars=[], +) + +SDK_AGENTIC_RL_BASE_CONNECTION_ERROR = SdkErrorDef( + name="sdk.agentic_rl.BaseConnectionError", + message="Connection-related error occurred.", + solution=( + "Check your network connectivity and ensure the target " + "service endpoint is reachable. Verify firewall rules and " + "proxy settings." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTIC_RL_OSS_CONNECTION_ERROR = SdkErrorDef( + name="sdk.agentic_rl.OSSConnectionError", + message="Connecting to OSS storage service failed.", + solution=( + "Verify that the OSS endpoint URL is correct and the OSS " + "service is accessible. Check your API key permissions for " + "OSS access. Ensure you are in a region that supports OSS " + "connectivity." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTIC_RL_OSS_UPLOAD_ERROR = SdkErrorDef( + name="sdk.agentic_rl.OSSUploadError", + message="File upload operation to OSS failed.", + solution=( + "Check that the file exists, is readable, and does not " + "exceed the upload size limit. Verify your OSS bucket " + "permissions allow write operations." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTIC_RL_DEPLOYMENT_ERROR = SdkErrorDef( + name="sdk.agentic_rl.DeploymentError", + message="Deployment-related error occurred.", + solution=( + "Check the deployment service status and logs. Ensure the " + "function code package and dependencies are valid. Retry " + "after the deployment service recovers." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTIC_RL_REGISTRATION_ERROR = SdkErrorDef( + name="sdk.agentic_rl.RegistrationError", + message="Function registration failed in the deployment system.", + solution=( + "The function could not be registered in the deployment " + "system. Verify the function name is unique and the code " + "package is valid. Check the inner error for " + "deployment-specific details." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTIC_RL_FUNCTION_LOAD_ERROR = SdkErrorDef( + name="sdk.agentic_rl.FunctionLoadError", + message="Loading a registered function into runtime failed.", + solution=( + "The function could not be loaded into the runtime after " + "registration. Check the error logs for import errors or " + "missing dependencies in the function layer." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTIC_RL_INSTANCE_WARMUP_ERROR = SdkErrorDef( + name="sdk.agentic_rl.InstanceWarmupError", + message="Function instance health check failed after deployment.", + solution=( + "The function instance health check failed after deployment. " + "Check the instance logs for startup errors. Ensure the " + "function initializes correctly and the health endpoint " + "responds within the timeout." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTIC_RL_INSTANCE_QUERY_ERROR = SdkErrorDef( + name="sdk.agentic_rl.InstanceQueryError", + message="Querying function instance status failed.", + solution=( + "Could not query the function instance status. The instance " + "may not be ready yet. Wait and retry, or check the " + "deployment logs for errors." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTIC_RL_FUNCTION_LAYER_ERROR = SdkErrorDef( + name="sdk.agentic_rl.FunctionLayerError", + message="Creating a layer of function failed.", + solution=( + "Failed to create the function dependency layer. Check that " + "the requirements.txt or setup.py is valid and all " + "dependencies are installable. Review the layer build logs " + "for details." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTIC_RL_DATASETS_ERROR = SdkErrorDef( + name="sdk.agentic_rl.DatasetsError", + message="Update datasets failed in the deployment system.", + solution=( + "Failed to update datasets in the deployment system. Verify " + "the dataset IDs are valid and the dataset files are " + "accessible. Retry after fixing the underlying issue." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTIC_RL_VALIDATION_ERROR = SdkErrorDef( + name="sdk.agentic_rl.ValidationError", + message="Data validation failed.", + solution=( + "One or more data fields failed validation. Check the error " + "message for the specific field and fix the value according " + "to the schema documentation." + ), + allow_retry=False, + vars=[], +) + +SDK_AGENTIC_RL_CONFIGURATION_ERROR = SdkErrorDef( + name="sdk.agentic_rl.ConfigurationError", + message="Invalid system configuration detected.", + solution=( + "Check your YAML configuration file or initialization " + "parameters. Ensure all required fields (model, functions, " + "datasets) are correctly specified. Refer to the AgenticRL " + "configuration documentation." + ), + allow_retry=False, + vars=[], +) + +SDK_AGENTIC_RL_BASE_PERMISSION_ERROR = SdkErrorDef( + name="sdk.agentic_rl.BasePermissionError", + message="Operation lacks required permissions.", + solution=( + "Your API key does not have the required permissions. " + "Ensure your account has access to AgenticRL, OSS, and " + "Function Compute services. Contact your administrator to " + "grant the necessary permissions." + ), + allow_retry=False, + vars=[], +) + +SDK_AGENTIC_RL_IO_ERROR_WITH_CODE = SdkErrorDef( + name="sdk.agentic_rl.IOErrorWithCode", + message="General I/O operation failure.", + solution=( + "A file or network I/O operation failed. Check file paths, " + "permissions, and network connectivity. Review the error " + "message for the specific operation that failed." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTIC_RL_ERROR = SdkErrorDef( + name="sdk.agentic_rl.Error", + message="Base client error occurred.", + solution=( + "An unexpected AgenticRL error occurred. Check the error " + "message and logs for details. If the issue persists, " + "submit a bug report at " + "https://github.com/dashscope/dashscope-sdk-python/issues." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTIC_RL_RUNTIME_ERROR_WITH_CODE = SdkErrorDef( + name="sdk.agentic_rl.RuntimeErrorWithCode", + message="Runtime error occurred in client.", + solution=( + "An unexpected runtime error occurred. Check the error " + "message and traceback for details. If the issue persists, " + "submit a bug report at " + "https://github.com/dashscope/dashscope-sdk-python/issues." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTIC_RL_VALUE_ERROR_WITH_CODE = SdkErrorDef( + name="sdk.agentic_rl.ValueErrorWithCode", + message="Invalid value encountered in client.", + solution=( + "An invalid value was encountered. Check the function " + "parameters and input data. Ensure all values conform to " + "the expected types and ranges." + ), + allow_retry=False, + vars=[], +) + +SDK_AGENTSTUDIO_API_CONNECTION_ERROR = SdkErrorDef( + name="sdk.agentstudio.APIConnectionError", + message="Failed to connect to the AgentStudio service.", + solution=( + "Check your network connectivity and ensure the " + "AgentStudio endpoint (base_url / region) is reachable. " + "Verify firewall and proxy settings." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTSTUDIO_API_TIMEOUT_ERROR = SdkErrorDef( + name="sdk.agentstudio.APITimeoutError", + message="The request to the AgentStudio service timed out.", + solution=( + "The request did not complete within the timeout period. " + "Increase the client timeout or retry the request." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTSTUDIO_STREAM_ERROR = SdkErrorDef( + name="sdk.agentstudio.StreamError", + message="The SSE stream encountered a fatal protocol error.", + solution=( + "Retry the streaming request. If the error persists, " + "report it at " + "https://github.com/dashscope/dashscope-sdk-python/issues." + ), + allow_retry=True, + vars=[], +) + +SDK_AGENTSTUDIO_STREAM_CLOSED_ERROR = SdkErrorDef( + name="sdk.agentstudio.StreamClosedError", + message="Attempted I/O on an already-closed stream.", + solution=( + "Do not read from a stream after it has been closed. " + "Consume the stream within its context manager or before " + "calling close()." + ), + allow_retry=False, + vars=[], +) + + +# -- Catalogs ------------------------------------------------------ +PUBLIC_ERRORS = [ + INVALID_REQUEST, + MISSING_PARAMETER, + CONTENT_POLICY_VIOLATION, + INVALID_URL, + INVALID_FILE, + AUTH_FAILED, + INVALID_API_KEY, + PERMISSION_DENIED, + RESOURCE_NOT_FOUND, + REQUEST_TOO_LARGE, + RATE_LIMIT_EXCEEDED, + CONCURRENCY_LIMIT_EXCEEDED, + INSUFFICIENT_QUOTA, + INTERNAL_ERROR, + SERVICE_UNAVAILABLE, + REQUEST_TIMEOUT, +] +INTERNAL_ERRORS = [ + SDK_INVALID_API_KEY, + SDK_AGENTIC_RL_FUNCTION_REGISTRATION_FAILED, + SDK_AGENTIC_RL_DATASETS_UPLOAD_FAILED, + SDK_AGENTIC_RL_DUPLICATE_FUNCTION_NAMES, + SDK_AGENTIC_RL_JOB_SUBMISSION_FAILED, + SDK_AGENTIC_RL_WORKFLOW_FAILED, + SDK_AGENTIC_RL_UNSUPPORTED_FUNCTION_TYPE, + SDK_AGENTIC_RL_FUNCTION_TEST_FAILED, + SDK_AGENTIC_RL_FUNCTION_TEST_TIMEOUT, + SDK_AGENTIC_RL_INPUT_ERROR, + SDK_AGENTIC_RL_OUTPUT_ERROR, + SDK_AGENTIC_RL_BASE_CONNECTION_ERROR, + SDK_AGENTIC_RL_OSS_CONNECTION_ERROR, + SDK_AGENTIC_RL_OSS_UPLOAD_ERROR, + SDK_AGENTIC_RL_DEPLOYMENT_ERROR, + SDK_AGENTIC_RL_REGISTRATION_ERROR, + SDK_AGENTIC_RL_FUNCTION_LOAD_ERROR, + SDK_AGENTIC_RL_INSTANCE_WARMUP_ERROR, + SDK_AGENTIC_RL_INSTANCE_QUERY_ERROR, + SDK_AGENTIC_RL_FUNCTION_LAYER_ERROR, + SDK_AGENTIC_RL_DATASETS_ERROR, + SDK_AGENTIC_RL_VALIDATION_ERROR, + SDK_AGENTIC_RL_CONFIGURATION_ERROR, + SDK_AGENTIC_RL_BASE_PERMISSION_ERROR, + SDK_AGENTIC_RL_IO_ERROR_WITH_CODE, + SDK_AGENTIC_RL_ERROR, + SDK_AGENTIC_RL_RUNTIME_ERROR_WITH_CODE, + SDK_AGENTIC_RL_VALUE_ERROR_WITH_CODE, + SDK_AGENTSTUDIO_API_CONNECTION_ERROR, + SDK_AGENTSTUDIO_API_TIMEOUT_ERROR, + SDK_AGENTSTUDIO_STREAM_ERROR, + SDK_AGENTSTUDIO_STREAM_CLOSED_ERROR, +] diff --git a/dashscope/common/utils.py b/dashscope/common/utils.py index 8c2d79f..74f43de 100644 --- a/dashscope/common/utils.py +++ b/dashscope/common/utils.py @@ -10,7 +10,7 @@ import uuid from dataclasses import dataclass from http import HTTPStatus -from typing import Dict +from typing import Dict, Tuple from urllib.parse import urlparse import aiohttp @@ -19,13 +19,30 @@ from dashscope.api_entities.dashscope_response import DashScopeAPIResponse from dashscope.common.api_key import get_default_api_key from dashscope.common.constants import SSE_CONTENT_TYPE +from dashscope.common.error_registry import INTERNAL_ERROR from dashscope.common.logging import logger from dashscope.version import __version__ _SDK_SESSION_ID = uuid.uuid4().hex -def is_validate_fine_tune_file(file_path): +def truncate_error_message(message: str, max_length: int = 200) -> str: + """Truncate error message for logging to avoid excessive log output. + + Args: + message: The error message to truncate. + max_length: Maximum length of the message. Defaults to 200. + + Returns: + Truncated message with '...' suffix if longer than max_length, + otherwise original message. + """ + if len(message) > max_length: + return message[:max_length] + "..." + return message + + +def is_validate_fine_tune_file(file_path: str) -> bool: with open(file_path, encoding="utf-8") as f: for line in f: try: @@ -35,7 +52,7 @@ def is_validate_fine_tune_file(file_path): return True -def _get_task_group_and_task(module_name): +def _get_task_group_and_task(module_name: str) -> Tuple[str, str]: """Get task_group and task name. get task_group and task name based on api file __name__ @@ -51,41 +68,36 @@ def _get_task_group_and_task(module_name): return task_group, task -def is_path(path: str): - """Check the input path is valid local path. +def is_path(path: str) -> bool: + """Check if the input is a valid local path. Args: - path_or_url (str): The path. + path: The path to check. Returns: - bool: If path return True, otherwise False. + True if it's a valid local path, False otherwise. """ url_parsed = urlparse(path) - if url_parsed.scheme in ("file", ""): - return os.path.exists(url_parsed.path) - else: - return False + return url_parsed.scheme in ("file", "") and os.path.exists( + url_parsed.path, + ) -def is_url(url: str): - """Check the input url is valid url. +def is_url(url: str) -> bool: + """Check if the input is a valid URL. Args: - url (str): The url + url: The URL to check. Returns: - bool: If is url return True, otherwise False. + True if it's a valid URL, False otherwise. """ url_parsed = urlparse(url) - # pylint: disable=simplifiable-if-statement - if url_parsed.scheme in ("http", "https", "oss"): - return True - else: - return False + return url_parsed.scheme in ("http", "https", "oss") def iter_over_async(ait): - loop = asyncio.get_event_loop_policy().new_event_loop() + loop = asyncio.new_event_loop() ait = ait.__aiter__() async def get_next(): @@ -109,16 +121,14 @@ def iter_thread(loop, message_queue): message_queue.put((True, e, None)) break finally: - try: - loop.close() - except Exception: - pass + loop.close() message_queue = queue.Queue() x = threading.Thread( target=iter_thread, args=(loop, message_queue), name="iter_async_thread", + daemon=True, ) x.start() while True: @@ -126,10 +136,14 @@ def iter_thread(loop, message_queue): if finished: if error is not None: yield DashScopeAPIResponse( - -1, - "", - "Unknown", - message=f"Error type: {type(error)}, message: {error}", + status_code=INTERNAL_ERROR.status_code, + request_id="", + code=INTERNAL_ERROR.error_code, + message=( + f"{INTERNAL_ERROR.format_msg()} " + f"(SDK Internal Error: " + f"{type(error).__name__}: {error})" + ), ) break yield obj # pylint: disable=no-else-break @@ -227,7 +241,7 @@ def default_headers(api_key: str = None, module: str = "") -> Dict[str, str]: return headers -def join_url(base_url, *args): +def join_url(base_url: str, *args: str) -> str: if not base_url.endswith("/"): base_url = base_url + "/" url = base_url @@ -237,58 +251,19 @@ def join_url(base_url, *args): return url[:-1] -async def _handle_aiohttp_response(response: aiohttp.ClientResponse): - request_id = "" - if response.status == HTTPStatus.OK: - json_content = await response.json() - if "request_id" in json_content: - request_id = json_content["request_id"] - return DashScopeAPIResponse( - request_id=request_id, - status_code=HTTPStatus.OK, - output=json_content, - ) - else: - if "application/json" in response.content_type: - error = await response.json() - msg = "" - if "message" in error: - msg = error["message"] - if "request_id" in error: - request_id = error["request_id"] - return DashScopeAPIResponse( - request_id=request_id, - status_code=response.status, - output=None, - code=error["code"], - message=msg, - ) - else: - msg = await response.read() - return DashScopeAPIResponse( - request_id=request_id, - status_code=response.status, - output=None, - code="Unknown", - message=msg, - ) - - @dataclass class SSEEvent: - id: str - eventType: str - data: str - - def __init__( # pylint: disable=redefined-builtin - self, - id: str, - type: str, - data: str, - ): - self.id = id - self.eventType = type - self.data = data + """Server-Sent Events event representation. + + Attributes: + id: Event ID from the 'id:' field. + eventType: Event type from the 'event:' field. + data: Event data from the 'data:' field. + """ + + id: str = "" + eventType: str = "" + data: str = "" def _handle_stream(response: requests.Response): @@ -325,20 +300,48 @@ def _handle_stream(response: requests.Response): def _handle_error_message(error, status_code, flattened_output, headers): - code = None msg = "" request_id = "" + + # Log original error information + original_code = error.get("code", error.get("error_code", "")) + original_message = error.get( + "message", + error.get("error_message", error.get("msg", "")), + ) + logger.error( + "Request failed: status=%s, code=%s, message=%s", + status_code, + original_code or "unknown", + original_message or "unknown", + ) + if flattened_output: error["status_code"] = status_code return error - if "message" in error: + + # Extract message, fallback to INTERNAL_ERROR.format_msg() if empty + if "message" in error and error["message"]: msg = error["message"] - if "msg" in error: + elif "msg" in error and error["msg"]: msg = error["msg"] - if "code" in error: + elif "error_message" in error and error["error_message"]: + msg = error["error_message"] + else: + msg = INTERNAL_ERROR.format_msg() + + # Extract code, fallback to INTERNAL_ERROR.error_code if empty + if "code" in error and error["code"]: code = error["code"] + elif "error_code" in error and error["error_code"]: + code = error["error_code"] + else: + code = INTERNAL_ERROR.error_code + + # Extract request_id if "request_id" in error: request_id = error["request_id"] + return DashScopeAPIResponse( request_id=request_id, status_code=status_code, @@ -373,22 +376,43 @@ def _handle_http_failed_response( flattened_output, headers, ) + # No valid error data in SSE response + error_message = "\n".join(msgs).strip() or INTERNAL_ERROR.format_msg() + error_code = INTERNAL_ERROR.error_code + logger.error( + "Request failed: status=%s, code=%s, message=%s", + response.status_code, + error_code, + truncate_error_message(error_message), + ) return DashScopeAPIResponse( request_id=request_id, status_code=response.status_code, - code="Unknown", - message=msgs, + code=error_code, + message=error_message, headers=headers, ) else: msg = response.content.decode("utf-8") + error_message = msg or INTERNAL_ERROR.format_msg() + error_code = INTERNAL_ERROR.error_code + logger.error( + "Request failed: status=%s, code=%s, message=%s", + response.status_code, + error_code, + truncate_error_message(error_message), + ) if flattened_output: - return {"status_code": response.status_code, "message": msg} # type: ignore[return-value] # pylint: disable=line-too-long # noqa: E501 + return { # type: ignore[return-value] + "status_code": response.status_code, + "code": error_code, + "message": error_message, + } return DashScopeAPIResponse( request_id=request_id, status_code=response.status_code, - code="Unknown", - message=msg, + code=error_code, + message=error_message, headers=headers, ) @@ -416,13 +440,33 @@ async def _handle_aio_stream(response): async def _handle_aiohttp_failed_response( - response: requests.Response, + response: aiohttp.ClientResponse, flattened_output: bool = False, ) -> DashScopeAPIResponse: request_id = "" headers = dict(response.headers) if "application/json" in response.content_type: error = await response.json() + # Pass through code, fallback to + # INTERNAL_ERROR.error_code if not available + error_code = ( + error.get("code") + or error.get("error_code") + or INTERNAL_ERROR.error_code + ) + # Pass through message, fallback to + # INTERNAL_ERROR.error_msg if not available + error_message = ( + error.get("message") + or error.get("error_message") + or INTERNAL_ERROR.format_msg() + ) + logger.error( + "Request failed: status=%s, code=%s, message=%s", + response.status, + error_code, + truncate_error_message(error_message), + ) return _handle_error_message( error, response.status, @@ -431,19 +475,34 @@ async def _handle_aiohttp_failed_response( ) elif SSE_CONTENT_TYPE in response.content_type: error = None + raw_data = [] async for _, _, data in _handle_aio_stream(response): - error = json.loads(data) + raw_data.append(data) + try: + error = json.loads(data) + except json.JSONDecodeError: + continue if error is None: + raw_content = "\n".join(raw_data).strip() if raw_data else "" + error_code = INTERNAL_ERROR.error_code + error_message = raw_content or INTERNAL_ERROR.format_msg() + logger.error( + "Request failed: status=%s, code=%s, message=%s", + response.status, + error_code, + truncate_error_message(error_message), + ) if flattened_output: return { # type: ignore[return-value] "status_code": response.status, - "message": "Empty SSE error response", + "code": error_code, + "message": error_message, } return DashScopeAPIResponse( request_id=request_id, status_code=response.status, - code="Unknown", - message="Empty SSE error response", + code=error_code, + message=error_message, headers=headers, ) return _handle_error_message( @@ -454,13 +513,25 @@ async def _handle_aiohttp_failed_response( ) else: msg = await response.text() + error_code = INTERNAL_ERROR.error_code + error_message = msg or INTERNAL_ERROR.format_msg() + logger.error( + "Request failed: status=%s, code=%s, message=%s", + response.status, + error_code, + truncate_error_message(error_message), + ) if flattened_output: - return {"status_code": response.status, "message": msg} # type: ignore[return-value] # pylint: disable=line-too-long # noqa: E501 + return { # type: ignore[return-value] + "status_code": response.status, + "code": error_code, + "message": error_message, + } return DashScopeAPIResponse( request_id=request_id, status_code=response.status, - code="Unknown", - message=msg, + code=error_code, + message=error_message, headers=headers, ) @@ -469,11 +540,10 @@ def _handle_http_response( response: requests.Response, flattened_output: bool = False, ): - response = _handle_http_stream_response(response, flattened_output) - _, output = next(response) - try: - next(response) - except StopIteration: + response_gen = _handle_http_stream_response(response, flattened_output) + _, output = next(response_gen) + # Consume remaining items to ensure generator completes + for _ in response_gen: pass return output @@ -514,19 +584,28 @@ def _handle_http_stream_response( usage=usage, headers=headers, ) - except json.JSONDecodeError as e: + except json.JSONDecodeError: + error_code = INTERNAL_ERROR.error_code + error_message = event.data or INTERNAL_ERROR.format_msg() + logger.error( + "Request failed: status=%s, code=%s, message=%s", + response.status_code, + error_code, + truncate_error_message(error_message), + ) if flattened_output: yield event.eventType, { "status_code": response.status_code, - "message": e.message, + "code": error_code, + "message": error_message, } else: yield event.eventType, DashScopeAPIResponse( request_id=request_id, - status_code=HTTPStatus.BAD_REQUEST, + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, output=None, - code="Unknown", - message=event.data, + code=error_code, + message=error_message, headers=headers, ) continue @@ -537,17 +616,39 @@ def _handle_http_stream_response( "message": event.data, } else: - msg = json.loads(event.data) - yield event.eventType, DashScopeAPIResponse( - request_id=request_id, - status_code=status_code, - output=None, - code=msg["code"] - if "code" in msg - else None, # noqa E501 - message=msg["message"] if "message" in msg else None, - headers=headers, - ) # noqa E501 + try: + msg = json.loads(event.data) + yield event.eventType, DashScopeAPIResponse( + request_id=request_id, + status_code=status_code, + output=None, + code=msg.get("code") + or msg.get("error_code") + or f"http_{status_code}", + message=msg.get("message") + or msg.get("error_message") + or f"HTTP {status_code} error", + headers=headers, + ) # noqa E501 + except json.JSONDecodeError: + error_code = INTERNAL_ERROR.error_code + error_message = ( + event.data or INTERNAL_ERROR.format_msg() + ) + logger.error( + "Request failed: status=%s, code=%s, message=%s", + status_code, + error_code, + truncate_error_message(error_message), + ) + yield event.eventType, DashScopeAPIResponse( + request_id=request_id, + status_code=status_code, + output=None, + code=error_code, + message=error_message, + headers=headers, + ) # pylint: disable=consider-using-in elif ( response.status_code == HTTPStatus.OK diff --git a/dashscope/finetune/agentic_rl.py b/dashscope/finetune/agentic_rl.py index 6eb707a..bd86f8d 100644 --- a/dashscope/finetune/agentic_rl.py +++ b/dashscope/finetune/agentic_rl.py @@ -4,7 +4,7 @@ # Copyright (c) Alibaba, Inc. and its affiliates. import time -from typing import Union, List, Optional, ClassVar, Dict, Any +from typing import Union, List, Optional, ClassVar, Dict, Any, Tuple from typing_extensions import Self from dashscope.client.base_api import CreateMixin @@ -41,17 +41,135 @@ generate_random_id, get_func_type_id, deep_remove_none, + HTTP_REQUEST_TIMEOUT, +) +from dashscope.common.error import ( + AuthenticationError, + InvalidParameter, + DashScopeException, +) +from dashscope.common.error_registry import ( + INVALID_API_KEY, + INVALID_REQUEST, + INTERNAL_ERROR, + PERMISSION_DENIED, + REQUEST_TIMEOUT, + SDK_INVALID_API_KEY, + SDK_AGENTIC_RL_CONFIGURATION_ERROR, + SDK_AGENTIC_RL_FUNCTION_REGISTRATION_FAILED, + SDK_AGENTIC_RL_DATASETS_UPLOAD_FAILED, + SDK_AGENTIC_RL_DUPLICATE_FUNCTION_NAMES, + SDK_AGENTIC_RL_JOB_SUBMISSION_FAILED, + SDK_AGENTIC_RL_WORKFLOW_FAILED, + SDK_AGENTIC_RL_UNSUPPORTED_FUNCTION_TYPE, + SDK_AGENTIC_RL_FUNCTION_TEST_FAILED, + SDK_AGENTIC_RL_FUNCTION_TEST_TIMEOUT, + SDK_AGENTIC_RL_VALIDATION_ERROR, ) from dashscope.finetune.reinforcement.common.errors import ( - RegistrationError, + BasePermissionError, + ConfigurationError, + InputError, + IOErrorWithCode, ValidationError, InstanceQueryError, - RuntimeErrorWithCode, ValueErrorWithCode, - DatasetsError, ) +def _get_public_error( + exc: Exception, + input_errors: Tuple[type, ...] = (), +): + """Map an internal exception to the appropriate public error. + + Per dashscope public-errors spec: + - BasePermissionError → PERMISSION_DENIED (403) + - ConfigurationError / ValidationError / InputError → INVALID_REQUEST (400) + - TimeoutError → REQUEST_TIMEOUT (504) + - All others → INTERNAL_ERROR (500) + + ``input_errors`` adds caller-specific types that also mean bad user input. + Flows that validate a payload before sending anything treat a generic + ``ValueError`` as a 400; the request paths cannot, because there the same + type may come from the server side. + """ + invalid_input_types = ( + ConfigurationError, + ValidationError, + InputError, + ) + input_errors + + if isinstance(exc, BasePermissionError): + return PERMISSION_DENIED + elif isinstance(exc, invalid_input_types): + return INVALID_REQUEST + elif isinstance(exc, TimeoutError): + return REQUEST_TIMEOUT + else: + return INTERNAL_ERROR + + +def _exc_message(exc: Exception) -> str: + """Extract clean message from an exception. + + Uses the ``.message`` attribute (set by AgenticRLError and friends) + to avoid embedding ``[error_code]``, ``name``, and ``(at timestamp)`` + wrappers when composing outer error messages. + + Falls back to ``str(exc)`` for plain exceptions. + """ + msg = getattr(exc, "message", None) + if msg: + return str(msg) + return str(exc) + + +def _log_internal_error( + error_def, + cause: Exception, + extra_vars: Optional[Dict[str, str]] = None, +) -> None: + """Record a failure under its internal ``sdk.agentic_rl.*`` code. + + Kept as a separate call so the five handlers stay one line each. + """ + inner_code = getattr(cause, "error_code", None) or "unknown" + variables = {"inner_code": inner_code} + if extra_vars: + variables.update(extra_vars) + + logger.error( + "[%s] %s | [%s] %s", + error_def.name, + error_def.format_message(variables), + inner_code, + cause, + exc_info=True, + # Without this every handler logs from this helper's line, so the + # record prefix can no longer tell the five call sites apart. + stacklevel=2, + ) + + +def _public_exception(public_error, cause: Exception) -> DashScopeException: + """Build the caller-facing exception for a classified internal failure. + + A 400 keeps the narrower ``InvalidParameter`` type; everything else becomes + a plain ``DashScopeException``. Both carry the public ``status_code`` and + ``error_code`` instead of the internal one. + """ + summary = f"{type(cause).__name__}: {_exc_message(cause)}" + message = f"{public_error.format_msg()} | Caused by: {summary}" + if public_error == INVALID_REQUEST: + exc = InvalidParameter(message) + else: + exc = DashScopeException(message) + exc.status_code = public_error.status_code + exc.error_code = public_error.error_code + return exc + + class AgenticRL(AgenticRLTuning, CreateMixin): SUB_PATH: ClassVar[str] = "fine-tunes" @@ -61,16 +179,37 @@ def __init__(self, api_key: str = None): try: set_api_key(api_key) except Exception as e: - raise ValueErrorWithCode( - "Invalid API key configuration", - error_code=3001, - ) from e + logger.error( + "[%s] %s | %s", + SDK_INVALID_API_KEY.name, + SDK_INVALID_API_KEY.format_message(), + e, + exc_info=True, + ) + exc = AuthenticationError(INVALID_API_KEY.format_msg()) + exc.status_code = INVALID_API_KEY.status_code + exc.error_code = INVALID_API_KEY.error_code + raise exc from e def init(self, config_path: Optional[str] = None, **kwargs) -> Self: """ Initialize an AgenticRL instance from a YAML configuration file. """ - self.tuning = TuningModel.load_from_yaml(config_path or "", **kwargs) + try: + self.tuning = TuningModel.load_from_yaml( + config_path or "", + **kwargs, + ) + except Exception as e: + _log_internal_error(SDK_AGENTIC_RL_CONFIGURATION_ERROR, e) + # ``load_from_yaml`` reports a missing file and a malformed one + # through the same IOErrorWithCode, and both are the caller's to + # fix. An I/O failure deeper in a request stays a 500. + public_error = _get_public_error( + e, + input_errors=(IOErrorWithCode,), + ) + raise _public_exception(public_error, e) from e return self @@ -84,7 +223,7 @@ async def register_functions( ] ] = None, lazy_load: Optional[bool] = True, - ) -> tuple[ + ) -> Tuple[ List[str], List[str], List[str], @@ -109,12 +248,13 @@ async def register_functions( ) logger.info("Function components registered") except Exception as e: - if hasattr(e, "error_code"): + if isinstance(e, DashScopeException): raise - raise RegistrationError( - "Function registration failed", - error_code=3002, - ) from e + _log_internal_error( + SDK_AGENTIC_RL_FUNCTION_REGISTRATION_FAILED, + e, + ) + raise _public_exception(_get_public_error(e), e) from e return ( rollout_entity_ids, @@ -130,7 +270,7 @@ async def upload_datasets( datasets: Optional[List[Dataset]] = None, training_files: Optional[Union[List[str], str]] = None, validation_files: Optional[Union[List[str], str]] = None, - ) -> tuple[List[str], List[str]]: + ) -> Tuple[List[str], List[str]]: if datasets: self.tuning.datasets = datasets @@ -144,14 +284,17 @@ async def upload_datasets( ) logger.info("Datasets uploaded") except Exception as e: - raise DatasetsError( - "Datasets upload failed", - error_code=3003, - ) from e + if isinstance(e, DashScopeException): + raise + _log_internal_error( + SDK_AGENTIC_RL_DATASETS_UPLOAD_FAILED, + e, + ) + raise _public_exception(_get_public_error(e), e) from e return uploaded_training_ids, uploaded_validation_ids - def submit_job( + def submit_job( # pylint: disable=too-many-branches self, model: Optional[str] = None, datasets: Optional[List[Dataset]] = None, @@ -206,11 +349,26 @@ def submit_job( ) # names of functions if not self.tuning.check_function_names(): - raise ValueErrorWithCode( - "Duplicate function names detected. All function names must " - "be unique.", - error_code=3004, + duplicates, _missing = self.tuning.find_function_name_problems() + if duplicates: + name_error = SDK_AGENTIC_RL_DUPLICATE_FUNCTION_NAMES + name_detail = name_error.format_message( + {"names": ", ".join(duplicates)}, + ) + else: + # The check also fails when a component carries no `name` at + # all, which is not a duplication problem. + name_error = SDK_AGENTIC_RL_VALIDATION_ERROR + name_detail = name_error.format_message() + logger.error( + "[%s] %s", + name_error.name, + name_detail, ) + exc = InvalidParameter(INVALID_REQUEST.format_msg()) + exc.status_code = INVALID_REQUEST.status_code + exc.error_code = INVALID_REQUEST.error_code + raise exc # datasets if datasets: @@ -267,12 +425,13 @@ def submit_job( **kwargs, ) except Exception as e: - if hasattr(e, "error_code"): + if isinstance(e, DashScopeException): raise - raise RuntimeErrorWithCode( - "Job submission failed", - error_code=3005, - ) from e + _log_internal_error( + SDK_AGENTIC_RL_JOB_SUBMISSION_FAILED, + e, + ) + raise _public_exception(_get_public_error(e), e) from e return FineTune(**resp) @@ -334,12 +493,13 @@ async def run( **kwargs, ) except Exception as e: - if hasattr(e, "error_code"): + # Log before the passthrough: the inner calls already convert + # their failures into DashScopeException, so waiting until after + # this check would leave the workflow failure unrecorded. + _log_internal_error(SDK_AGENTIC_RL_WORKFLOW_FAILED, e) + if isinstance(e, DashScopeException): raise - raise RuntimeErrorWithCode( - "RL tuning workflow failed", - error_code=3006, - ) from e + raise _public_exception(_get_public_error(e), e) from e @classmethod def cancel( @@ -437,6 +597,29 @@ def logs( **kwargs, ) + @staticmethod + def _validate_input(functype: FunctionType, input_data: Dict[str, Any]): + _FUNCTYPE_MODEL = { + FunctionType.ROLLOUT: RolloutInput, + FunctionType.REWARD: RewardInput, + FunctionType.GROUP_REWARD: GroupRewardInput, + } + model_cls = _FUNCTYPE_MODEL.get(functype) + if model_cls is None: + logger.error( + "[%s] %s | functype=%s", + SDK_AGENTIC_RL_UNSUPPORTED_FUNCTION_TYPE.name, + SDK_AGENTIC_RL_UNSUPPORTED_FUNCTION_TYPE.format_message( + {"functype": str(functype)}, + ), + functype, + ) + exc = InvalidParameter(INVALID_REQUEST.format_msg()) + exc.status_code = INVALID_REQUEST.status_code + exc.error_code = INVALID_REQUEST.error_code + raise exc + return model_cls.model_validate(input_data) + @classmethod async def test_functions( cls, @@ -469,17 +652,7 @@ async def test_functions( try: set_api_key(api_key) - if functype == FunctionType.ROLLOUT: - value = RolloutInput.model_validate(input_data) - elif functype == FunctionType.REWARD: - value = RewardInput.model_validate(input_data) - elif functype == FunctionType.GROUP_REWARD: - value = GroupRewardInput.model_validate(input_data) - else: - raise ValueErrorWithCode( - f"Unsupported function type: {functype}", - error_code=3007, - ) + value = cls._validate_input(functype, input_data) logger.info( f"Starting {str(functype)} verification", @@ -505,10 +678,25 @@ async def test_functions( return result except Exception as e: - raise ValidationError( - "Function test failed", - error_code=3008, - ) from e + if isinstance(e, (DashScopeException, InvalidParameter)): + raise + + # ValueError/TypeError raised by ``model_validate`` describe a + # payload the caller can fix, so they are a 400 here. + public_error = _get_public_error( + e, + input_errors=(ValueError, TypeError), + ) + + if public_error == REQUEST_TIMEOUT: + error_def = SDK_AGENTIC_RL_FUNCTION_TEST_TIMEOUT + extra_vars = {"timeout": str(HTTP_REQUEST_TIMEOUT)} + else: + error_def = SDK_AGENTIC_RL_FUNCTION_TEST_FAILED + extra_vars = None + + _log_internal_error(error_def, e, extra_vars) + raise _public_exception(public_error, e) from e @classmethod async def query_function_instance_logs( diff --git a/dashscope/finetune/reinforcement/common/errors.py b/dashscope/finetune/reinforcement/common/errors.py index 0d9f31d..11c877f 100644 --- a/dashscope/finetune/reinforcement/common/errors.py +++ b/dashscope/finetune/reinforcement/common/errors.py @@ -3,21 +3,54 @@ Custom exception hierarchy for the AgenticRL system """ +from __future__ import annotations + from datetime import datetime from typing import Optional, Dict +from dashscope.common.error_registry import ( + INTERNAL_ERRORS, + SDK_AGENTIC_RL_BASE_CONNECTION_ERROR, + SDK_AGENTIC_RL_BASE_PERMISSION_ERROR, + SDK_AGENTIC_RL_CONFIGURATION_ERROR, + SDK_AGENTIC_RL_DATASETS_ERROR, + SDK_AGENTIC_RL_DEPLOYMENT_ERROR, + SDK_AGENTIC_RL_ERROR, + SDK_AGENTIC_RL_FUNCTION_LAYER_ERROR, + SDK_AGENTIC_RL_FUNCTION_LOAD_ERROR, + SDK_AGENTIC_RL_INPUT_ERROR, + SDK_AGENTIC_RL_INSTANCE_QUERY_ERROR, + SDK_AGENTIC_RL_INSTANCE_WARMUP_ERROR, + SDK_AGENTIC_RL_IO_ERROR_WITH_CODE, + SDK_AGENTIC_RL_OSS_CONNECTION_ERROR, + SDK_AGENTIC_RL_OSS_UPLOAD_ERROR, + SDK_AGENTIC_RL_OUTPUT_ERROR, + SDK_AGENTIC_RL_REGISTRATION_ERROR, + SDK_AGENTIC_RL_RUNTIME_ERROR_WITH_CODE, + SDK_AGENTIC_RL_VALIDATION_ERROR, + SDK_AGENTIC_RL_VALUE_ERROR_WITH_CODE, +) + class _RootCauseMixin: """Mixin that provides root-cause traversal and formatting for exceptions that carry an error code.""" + MAX_CAUSE_DEPTH = 50 + @property def root_cause(self) -> "Exception": root: "Exception" = self # type: ignore[assignment] seen = {id(root)} - while root.__cause__ and id(root.__cause__) not in seen: + depth = 0 + while ( + root.__cause__ + and id(root.__cause__) not in seen + and depth < self.MAX_CAUSE_DEPTH + ): root = root.__cause__ seen.add(id(root)) + depth += 1 return root def _format_cause(self) -> str: @@ -31,18 +64,40 @@ def _format_cause(self) -> str: return f" (caused by: {type(root).__name__})" return "" + def _get_registry_message(self, error_code: str) -> str: + """Get the registry message for the given error code name. + + Returns empty string if not found or if the message contains + unfilled placeholders. + """ + for err_def in INTERNAL_ERRORS: + if err_def.name == error_code: + msg = err_def.format_message() + if err_def.vars and "{" in msg: + return "" + return msg + return "" + class AgenticRLError(_RootCauseMixin, Exception): """Base class for all Agentic RL exceptions.""" - def __init__(self, message: str, error_code: int = 1000): + def __init__( + self, + message: str, + error_code: str = SDK_AGENTIC_RL_ERROR.name, + ): super().__init__(message) self.error_code = error_code self.timestamp = datetime.now().isoformat() self.message = message def __str__(self): - base = f"[{self.error_code}] {self.message} (at {self.timestamp})" + registry_msg = self._get_registry_message(self.error_code) + if registry_msg: + base = f"{registry_msg} | {self.message}" + else: + base = self.message return f"{base}{self._format_cause()}" @@ -52,7 +107,7 @@ class IOErrorWithCode(AgenticRLError): def __init__( self, message: str, - error_code: int = 1800, + error_code: str = SDK_AGENTIC_RL_IO_ERROR_WITH_CODE.name, path: Optional[str] = None, operation: Optional[str] = None, ): @@ -65,26 +120,40 @@ class RuntimeErrorWithCode(_RootCauseMixin, RuntimeError): """Enhanced RuntimeError that supports error codes for better error categorization.""" - def __init__(self, message: str, error_code: int = 0): + def __init__( + self, + message: str, + error_code: str = SDK_AGENTIC_RL_RUNTIME_ERROR_WITH_CODE.name, + ): super().__init__(message) self.error_code = error_code self.message = message def __str__(self): - return f"[{self.error_code}] {self.message}{self._format_cause()}" + registry_msg = self._get_registry_message(self.error_code) + if registry_msg: + return f"{registry_msg} | {self.message}{self._format_cause()}" + return f"{self.message}{self._format_cause()}" class ValueErrorWithCode(_RootCauseMixin, ValueError): """Enhanced ValueError that supports error codes for better error categorization.""" - def __init__(self, message: str, error_code: int = 0): + def __init__( + self, + message: str, + error_code: str = SDK_AGENTIC_RL_VALUE_ERROR_WITH_CODE.name, + ): super().__init__(message) self.error_code = error_code self.message = message def __str__(self): - return f"[{self.error_code}] {self.message}{self._format_cause()}" + registry_msg = self._get_registry_message(self.error_code) + if registry_msg: + return f"{registry_msg} | {self.message}{self._format_cause()}" + return f"{self.message}{self._format_cause()}" class InputError(AgenticRLError): @@ -93,7 +162,7 @@ class InputError(AgenticRLError): def __init__( self, message: str, - error_code: int = 1100, + error_code: str = SDK_AGENTIC_RL_INPUT_ERROR.name, field: Optional[str] = None, ): super().__init__(message, error_code) @@ -106,7 +175,7 @@ class OutputError(AgenticRLError): def __init__( self, message: str, - error_code: int = 1200, + error_code: str = SDK_AGENTIC_RL_OUTPUT_ERROR.name, response: Optional[Dict] = None, ): super().__init__(message, error_code) @@ -119,7 +188,7 @@ class BaseConnectionError(AgenticRLError): def __init__( self, message: str, - error_code: int = 1300, + error_code: str = SDK_AGENTIC_RL_BASE_CONNECTION_ERROR.name, endpoint: Optional[str] = None, ): super().__init__(message, error_code) @@ -132,7 +201,7 @@ class OSSConnectionError(BaseConnectionError): def __init__( self, message: str, - error_code: int = 1310, + error_code: str = SDK_AGENTIC_RL_OSS_CONNECTION_ERROR.name, endpoint: str = None, ): super().__init__( @@ -148,13 +217,17 @@ class OSSUploadError(BaseConnectionError): def __init__( self, message: str, - error_code: int = 1320, + error_code: str = SDK_AGENTIC_RL_OSS_UPLOAD_ERROR.name, endpoint: str = None, bucket: Optional[str] = None, object_key: Optional[str] = None, file_size: Optional[int] = None, ): - super().__init__(f"OSS upload failed: {message}", error_code, endpoint) + super().__init__( + f"OSS upload failed: {message}", + error_code, + endpoint, + ) self.bucket = bucket self.object_key = object_key self.file_size = file_size @@ -166,7 +239,7 @@ class DeploymentError(AgenticRLError): def __init__( self, message: str, - error_code: int = 1400, + error_code: str = SDK_AGENTIC_RL_DEPLOYMENT_ERROR.name, resource_id: Optional[str] = None, ): super().__init__(message, error_code) @@ -179,7 +252,7 @@ class RegistrationError(DeploymentError): def __init__( self, message: str, - error_code: int = 1410, + error_code: str = SDK_AGENTIC_RL_REGISTRATION_ERROR.name, resource_id: Optional[str] = None, ): super().__init__( @@ -192,8 +265,15 @@ def __init__( class DatasetsError(DeploymentError): """Raised when update datasets fails in the deployment system""" - def __init__(self, message: str, error_code: int = 1460): - super().__init__(f"Datasets failed: {message}", error_code) + def __init__( + self, + message: str, + error_code: str = SDK_AGENTIC_RL_DATASETS_ERROR.name, + ): + super().__init__( + f"Datasets failed: {message}", + error_code, + ) class FunctionLoadError(DeploymentError): @@ -202,7 +282,7 @@ class FunctionLoadError(DeploymentError): def __init__( self, message: str, - error_code: int = 1420, + error_code: str = SDK_AGENTIC_RL_FUNCTION_LOAD_ERROR.name, entity_id: str = None, error_log: Optional[str] = None, ): @@ -221,7 +301,7 @@ class FunctionLayerError(DeploymentError): def __init__( self, message: str, - error_code: int = 1450, + error_code: str = SDK_AGENTIC_RL_FUNCTION_LAYER_ERROR.name, layer_name: str = None, error_log: Optional[str] = None, ): @@ -240,12 +320,15 @@ class InstanceWarmupError(DeploymentError): def __init__( self, message: str, - error_code: int = 1430, + error_code: str = SDK_AGENTIC_RL_INSTANCE_WARMUP_ERROR.name, instance_url: str = None, timeout: float = 0.0, retry_after: Optional[float] = None, ): - super().__init__(f"Instance warmup failed: {message}", error_code) + super().__init__( + f"Instance warmup failed: {message}", + error_code, + ) self.instance_url = instance_url self.timeout = timeout self.retry_after = retry_after @@ -257,14 +340,13 @@ class InstanceQueryError(DeploymentError): def __init__( self, message: str, - error_code: int = 1440, + error_code: str = SDK_AGENTIC_RL_INSTANCE_QUERY_ERROR.name, instance_id: str = None, query_attempts: int = 1, ): super().__init__( f"Instance query failed: {message}", error_code, - instance_id, ) self.instance_id = instance_id self.query_attempts = query_attempts @@ -276,11 +358,14 @@ class ValidationError(AgenticRLError): def __init__( self, message: str, - error_code: int = 1500, + error_code: str = SDK_AGENTIC_RL_VALIDATION_ERROR.name, invalid_data: Optional[Dict] = None, validation_rules: Optional[Dict] = None, ): - super().__init__(f"Validation failed: {message}", error_code) + super().__init__( + f"Validation failed: {message}", + error_code, + ) self.invalid_data = invalid_data self.validation_rules = validation_rules @@ -291,7 +376,7 @@ class ConfigurationError(ValidationError): def __init__( self, message: str, - error_code: int = 1510, + error_code: str = SDK_AGENTIC_RL_CONFIGURATION_ERROR.name, config_path: Optional[str] = None, ): super().__init__(message, error_code=error_code) @@ -304,7 +389,7 @@ class BasePermissionError(AgenticRLError): def __init__( self, message: str, - error_code: int = 1700, + error_code: str = SDK_AGENTIC_RL_BASE_PERMISSION_ERROR.name, operation: str = None, resource: str = None, ): diff --git a/dashscope/finetune/reinforcement/common/model.py b/dashscope/finetune/reinforcement/common/model.py index d0eb5a2..4a14003 100644 --- a/dashscope/finetune/reinforcement/common/model.py +++ b/dashscope/finetune/reinforcement/common/model.py @@ -22,6 +22,16 @@ ) import yaml +from dashscope.common.error_registry import ( + INTERNAL_ERROR, + SDK_AGENTIC_RL_INSTANCE_QUERY_ERROR, + SDK_AGENTIC_RL_INSTANCE_WARMUP_ERROR, + SDK_AGENTIC_RL_FUNCTION_LOAD_ERROR, + SDK_AGENTIC_RL_BASE_CONNECTION_ERROR, + SDK_AGENTIC_RL_REGISTRATION_ERROR, + SDK_AGENTIC_RL_FUNCTION_LAYER_ERROR, +) + # Local Application from dashscope.finetune.reinforcement.common.constants import ( FC_API_KEY, @@ -75,6 +85,7 @@ ) from dashscope.finetune.reinforcement.common.log import logger from dashscope.finetune.reinforcement.common.errors import ( + AgenticRLError, InputError, OutputError, OSSConnectionError, @@ -90,6 +101,21 @@ ) +def _exc_message(exc: Exception) -> str: + """Extract clean message from an exception. + + Uses the ``.message`` attribute (set by AgenticRLError and friends) + to avoid embedding ``[error_code]``, ``name``, and ``(at timestamp)`` + wrappers when composing outer error messages. + + Falls back to ``str(exc)`` for plain exceptions. + """ + msg = getattr(exc, "message", None) + if msg: + return str(msg) + return str(exc) + + class MountStorage(BaseModel): model_config = ConfigDict(extra="forbid") @@ -109,7 +135,7 @@ class Dataset(BaseModel): download_url: Optional[str] = None mount_storage: Optional[MountStorage] = None - async def upload_dataset(self) -> Optional[str]: + async def upload_dataset(self) -> str: if ( self.data_source_type == DataSourceType.FILE_ID and self.file_name is not None @@ -120,13 +146,21 @@ async def upload_dataset(self) -> Optional[str]: ) if file_id and isinstance(file_id, List) and len(file_id) > 0: self.file_id = file_id[0] + else: + raise OSSUploadError( + f"Empty upload result for {self.file_name}", + ) except Exception as e: raise OSSUploadError( "Failed to upload datasets", - error_code=2061, ) from e + if not self.file_id: + raise InputError( + f"Missing file_id after upload attempt: {self.file_name}", + field="file_id", + ) return self.file_id @@ -169,7 +203,6 @@ async def upload_datasets( except Exception as e: raise OSSUploadError( "Failed to upload datasets", - error_code=2062, ) from e return uploaded_training_ids, uploaded_validation_ids @@ -220,7 +253,6 @@ def load_from_dict(cls, data: dict, **kwargs) -> Self: except Exception as e: raise IOErrorWithCode( "Failed to load from dict", - error_code=1002, ) from e @classmethod @@ -235,7 +267,6 @@ def load_from_yaml(cls, file_path: str, **kwargs) -> Self: except Exception as e: raise IOErrorWithCode( f"Failed to load YAML file: {file_path}", - error_code=1001, path=file_path, ) from e @@ -272,7 +303,6 @@ def to_yaml(self, file_path: str, overwrite: bool = True) -> None: except Exception as e: raise IOErrorWithCode( "Failed to write file", - error_code=1003, ) from e @@ -341,7 +371,6 @@ async def get_oss(self, oss_id: Optional[str] = None) -> str: if not self.oss_signed_url: raise OSSConnectionError( f"Empty OSS URL received: {result}", - error_code=2001, ) logger.debug( @@ -355,7 +384,6 @@ async def get_oss(self, oss_id: Optional[str] = None) -> str: except Exception as e: raise OSSConnectionError( "Failed to obtain OSS URL", - error_code=2002, ) from e async def create_layer( @@ -401,7 +429,6 @@ async def create_layer( except Exception as e: raise FunctionLayerError( "Function layer create failed", - error_code=2013, ) from e return layer_code @@ -447,7 +474,6 @@ async def to_oss( except Exception as e: raise OSSUploadError( "Package upload failed", - error_code=2003, endpoint=url or "", ) from e @@ -484,7 +510,6 @@ async def get_layer(self, layer_code: str) -> str: if status != "SUCCESS": raise FunctionLayerError( f"Function layer create failed: {status}", - error_code=2014, ) return status @@ -492,22 +517,29 @@ async def get_layer(self, layer_code: str) -> str: except Exception as e: logger.warning( f"Load function layer failed | layer_code: {layer_code}, " - f"Error: {str(e)}", + f"Error: {_exc_message(e)}", ) return "SUCCESS" - def clean_temp_files(self, tmp_path: str) -> None: + def clean_temp_files(self, *tmp_paths: str) -> None: """Cleanup temporary deployment files.""" - try: - for f in [tmp_path]: - if os.path.exists(f): - if os.path.isfile(f): - os.remove(f) + failed_cleanups = [] + for tmp_path in tmp_paths: + if not tmp_path: + continue + try: + if os.path.exists(tmp_path): + if os.path.isfile(tmp_path): + os.remove(tmp_path) else: - shutil.rmtree(f) - except Exception as e: - logger.warning(f"Temp file cleanup failed: {str(e)}") + shutil.rmtree(tmp_path) + except Exception as e: + failed_cleanups.append((tmp_path, str(e))) + + if failed_cleanups: + msg = "; ".join(f"{p}: {err}" for p, err in failed_cleanups) + logger.error(f"Temp file cleanup failed: {msg}") def split_classpath(self): self.filepath, self.classname = get_filepath_classname(self.classpath) @@ -675,12 +707,13 @@ async def register( root = e while root.__cause__: root = root.__cause__ + _err = SDK_AGENTIC_RL_FUNCTION_LAYER_ERROR return ResponseFC( status=Status( task=StatusType.FAILED, - name="DeploymentError", - code=524, - message=f"Function layer deployment failed: {root}", + name=_err.name, + code=INTERNAL_ERROR.status_code, + message=(f"{_err.format_message()}: {_exc_message(root)}"), ), output={}, ) @@ -691,11 +724,13 @@ async def register( return ResponseFC( status=Status( task=StatusType.FAILED, - name="DeploymentError", - code=525, - message=f"Function deployment failed: {root}", + name=SDK_AGENTIC_RL_REGISTRATION_ERROR.name, + code=INTERNAL_ERROR.status_code, + message=( + f"{SDK_AGENTIC_RL_REGISTRATION_ERROR.format_message()}: " + f"{_exc_message(root)}" + ), ), - output={}, ) try: @@ -716,7 +751,6 @@ async def register( else: raise RegistrationError( f"Not exist type: {self.type.name}", - error_code=2011, ) result = await client_fc( @@ -730,7 +764,6 @@ async def register( if not self.entity_id: raise RegistrationError( f"Empty entity ID received: {result}", - error_code=2012, ) logger.info( @@ -753,12 +786,13 @@ async def register( root = e while root.__cause__: root = root.__cause__ + _err = SDK_AGENTIC_RL_BASE_CONNECTION_ERROR return ResponseFC( status=Status( task=StatusType.FAILED, - name="DeploymentError", - code=521, - message=f"Full deployment failed: {root}", + name=_err.name, + code=INTERNAL_ERROR.status_code, + message=(f"{_err.format_message()}: {_exc_message(root)}"), ), output={}, ) @@ -776,14 +810,12 @@ async def load( if not target_entity_id: raise ValueErrorWithCode( "No valid registration ID provided", - error_code=2021, ) if FC_LAYER_USED: if self.runtime.layer_code is None: raise ValueErrorWithCode( "layer_code is required when FC_LAYER_USED is enabled", - error_code=2022, ) await self.fcmodel.get_layer( layer_code=self.runtime.layer_code, @@ -804,7 +836,6 @@ async def load( if not self.instance_id: raise FunctionLoadError( f"Empty instance ID received: {result}", - error_code=2023, ) self.instance_url = result.get("output", {}).get("trigger_url", "") @@ -815,7 +846,6 @@ async def load( if (not self.instance_url) or (not self.instance_token): raise FunctionLoadError( "Missing instance URL or token", - error_code=2024, ) logger.info( @@ -830,16 +860,18 @@ async def load( logger.debug( f"Instance initialization failed | EntityID:" f" {target_entity_id}, " - f"Error: {str(e)}", + f"Error: {_exc_message(e)}", ) return ResponseFC( status=Status( task=StatusType.FAILED, - name="FunctionLoadError", - code=522, - message=f"Instance initialization failed: {str(e)}", + name=SDK_AGENTIC_RL_FUNCTION_LOAD_ERROR.name, + code=INTERNAL_ERROR.status_code, + message=( + f"{SDK_AGENTIC_RL_FUNCTION_LOAD_ERROR.format_message()}: " + f"{_exc_message(e)}" + ), ), - output={}, ) # Perform instance warmup if requested @@ -848,7 +880,6 @@ async def load( if not self.instance_url.startswith(("http://", "https://")): raise ValueErrorWithCode( "Invalid instance URL format", - error_code=2025, ) url = f"{self.instance_url.rstrip('/')}/health" @@ -862,7 +893,6 @@ async def load( if status != StatusType.HEALTH: raise InstanceWarmupError( f"Health check failed: {result}", - error_code=2026, instance_url=url, ) @@ -874,14 +904,17 @@ async def load( except Exception as e: logger.debug( f"Warmup failed | InstanceID: {self.instance_id}, " - f"Error: {str(e)}", + f"Error: {_exc_message(e)}", ) + _err = SDK_AGENTIC_RL_INSTANCE_WARMUP_ERROR return ResponseFC( status=Status( task=StatusType.FAILED, - name="InstanceWarmupError", - code=511, - message=f"Instance warmup failed: {str(e)}", + name=_err.name, + code=INTERNAL_ERROR.status_code, + message=( + f"{_err.format_message()}: {_exc_message(e)}" + ), ), output={"instance_id": self.instance_id}, ) @@ -907,7 +940,6 @@ async def query(cls, instance_id: str) -> ResponseFC: if not instance_id: raise InputError( "No instance ID available for query", - error_code=2031, ) url = f"{FC_QUERY_API}/{instance_id}" @@ -916,7 +948,6 @@ async def query(cls, instance_id: str) -> ResponseFC: if status == -1: raise InstanceQueryError( f"Invalid status received: {result}", - error_code=2032, ) logger.debug( @@ -927,14 +958,15 @@ async def query(cls, instance_id: str) -> ResponseFC: except Exception as e: logger.debug( f"Status query failed | InstanceID: {instance_id}, " - f"Error: {str(e)}", + f"Error: {_exc_message(e)}", ) + _err = SDK_AGENTIC_RL_INSTANCE_QUERY_ERROR return ResponseFC( status=Status( task=StatusType.FAILED, - name="InstanceQueryError", - code=523, - message=f"Status query failed: {str(e)}", + name=_err.name, + code=INTERNAL_ERROR.status_code, + message=(f"{_err.format_message()}: {_exc_message(e)}"), ), output={"instance_id": instance_id}, ) @@ -963,13 +995,11 @@ async def verify_function( if instance_id is None: raise ValueErrorWithCode( "instance_id is required for verification", - error_code=2041, ) result = await cls.query(instance_id) if result.status.task != StatusType.SUCCEEDED: raise InstanceQueryError( "Status query failed", - error_code=2042, ) instance_url = instance_url or result.output.get("output", {}).get( "trigger_url", @@ -982,7 +1012,6 @@ async def verify_function( if (not instance_url) or (not instance_token): raise OutputError( "No instance url/token provided", - error_code=2043, ) input_data_dict = input_data.model_dump( @@ -1014,13 +1043,16 @@ async def verify_function( else: raise ValidationError( "Unsupported input type", - error_code=2044, ) except Exception as e: + # AgenticRLError subclasses and timeouts already carry their own + # classification. Flattening them into ValidationError made a + # failed instance query or a request timeout look like a 400. + if isinstance(e, (AgenticRLError, TimeoutError)): + raise raise ValidationError( "Function verification failed", - error_code=2045, ) from e try: @@ -1039,7 +1071,6 @@ async def verify_function( error_msg = match.group(1) raise ValidationError( error_msg, - error_code=2046, ) validated = validator.model_validate(response) @@ -1053,9 +1084,13 @@ async def verify_function( return validated.model_dump() except Exception as e: + # The status branch above raises ValidationError carrying the + # reason the deployed function reported; re-wrapping it here would + # replace that reason with a generic message. + if isinstance(e, (AgenticRLError, TimeoutError)): + raise raise ValidationError( "Function output validation failed", - error_code=2047, ) from e @staticmethod @@ -1407,7 +1442,7 @@ class TuningModel(Models, BaseModel): async def register_functions( self, lazy_load: Optional[bool] = True, - ) -> tuple[ + ) -> Tuple[ List[str], List[str], List[str], @@ -1435,7 +1470,6 @@ async def register_functions( if not entity_id: raise RegistrationError( "Empty entity ID after registration", - error_code=2051, ) logger.debug( f"Registered new function component: " @@ -1444,7 +1478,6 @@ async def register_functions( else: raise RegistrationError( reg_result.status.message, - error_code=2052, ) if fc.type == FunctionType.ROLLOUT: @@ -1461,7 +1494,6 @@ async def register_functions( if not instance_id: raise FunctionLoadError( "Empty instance ID after load", - error_code=2053, ) logger.debug( f"Loaded function component instance: " @@ -1476,8 +1508,7 @@ async def register_functions( instance_group_reward_ids.append(instance_id) else: raise FunctionLoadError( - f"Load failed: {load_result}", - error_code=2054, + f"Load failed: {load_result.status.message}", ) except Exception as e: @@ -1485,7 +1516,6 @@ async def register_functions( raise raise RegistrationError( "Function component registration failed", - error_code=2055, ) from e return ( @@ -1501,7 +1531,7 @@ async def upload_datasets( self, training_files: Union[List[str], str] = None, validation_files: Union[List[str], str] = None, - ) -> tuple[List[str], List[str]]: + ) -> Tuple[List[str], List[str]]: """Register and validate training/validation datasets.""" uploaded_training_ids = [] uploaded_validation_ids = [] @@ -1542,7 +1572,6 @@ async def upload_datasets( except Exception as e: raise OSSUploadError( "Dataset registration failed", - error_code=2063, ) from e return uploaded_training_ids, uploaded_validation_ids @@ -1790,15 +1819,17 @@ def add_function_components( return self.functions - def check_function_names(self) -> bool: + def find_function_name_problems(self) -> Tuple[List[str], List[int]]: """ - Check for duplicate function component names. + Report naming problems across the function components. Returns: - True if all names are unique, False if duplicates found. + A ``(duplicate_names, missing_name_indexes)`` pair. Both lists are + empty when every component carries a unique ``name``. """ seen_names = {} - duplicate_found = False + duplicate_names: List[str] = [] + missing_name_indexes: List[int] = [] for index, fc in enumerate(self.functions): if not hasattr(fc, "name"): @@ -1806,7 +1837,7 @@ def check_function_names(self) -> bool: f"Function component at index {index} is missing a " f"'name' attribute", ) - duplicate_found = True + missing_name_indexes.append(index) continue name = fc.name @@ -1816,14 +1847,26 @@ def check_function_names(self) -> bool: f"Original at index {seen_names[name]}, duplicate at " f"index {index}", ) - duplicate_found = True + if name not in duplicate_names: + duplicate_names.append(name) else: seen_names[name] = index - if duplicate_found: + return duplicate_names, missing_name_indexes + + def check_function_names(self) -> bool: + """ + Check for duplicate function component names. + + Returns: + True if all names are unique, False if duplicates found. + """ + duplicate_names, missing_name = self.find_function_name_problems() + + if duplicate_names or missing_name: logger.error( - "Duplicate function names detected. All function names must " - "be unique.", + "Function name check failed. Every function component must " + "have a unique 'name' attribute.", ) return False diff --git a/dashscope/finetune/reinforcement/common/utils.py b/dashscope/finetune/reinforcement/common/utils.py index a3d58a5..573817d 100644 --- a/dashscope/finetune/reinforcement/common/utils.py +++ b/dashscope/finetune/reinforcement/common/utils.py @@ -92,7 +92,6 @@ async def _make_request() -> Dict[str, Any]: # Logical error – do not retry raise InputError( f"Unsupported method: {method}", - error_code=4001, ) # Treat server 5xx responses as transient failures and trigger a @@ -128,22 +127,19 @@ async def _make_request() -> Dict[str, Any]: except InputError: raise except asyncio.TimeoutError as e: - raise RuntimeErrorWithCode( + raise TimeoutError( f"Request timeout ({timeout}s)", - error_code=4003, ) from e except aiohttp.ClientError as e: raise RuntimeErrorWithCode( f"Client error: {e}", - error_code=4002, ) from e except Exception as e: raise RuntimeErrorWithCode( f"Unexpected error: {e}", - error_code=4004, ) from e - raise RuntimeErrorWithCode("Unexpected control flow", error_code=4005) + raise RuntimeErrorWithCode("Unexpected control flow") async def _handle_response(response) -> Dict[str, Any]: @@ -186,13 +182,12 @@ async def client_fc( def check_file(file: str) -> None: """Validate file existence and accessibility.""" if not os.path.exists(file): - raise InputError(f"File {file} not found", error_code=4011) + raise InputError(f"File {file} not found") if not os.path.isfile(file): - raise InputError(f"{file} is not a file", error_code=4012) + raise InputError(f"{file} is not a file") if not os.access(file, os.R_OK): raise InputError( f"No read access to file: {file}", - error_code=4013, ) @@ -393,7 +388,6 @@ def create_deployment_files( except Exception as e: raise RuntimeErrorWithCode( "Deployment file creation error", - error_code=4021, ) from e @@ -511,7 +505,6 @@ def _should_exclude(path: str, patterns: List[str]) -> Optional[str]: except Exception as e: raise RuntimeErrorWithCode( "Directory compression error", - error_code=4022, ) from e @@ -566,7 +559,6 @@ def _sync_upload_to_oss(signed_url: str, zip_path: str) -> int: except Exception as e: raise RuntimeErrorWithCode( "OSS upload error", - error_code=4023, ) from e @@ -599,7 +591,6 @@ async def upload_zip_to_oss_and_by_signed_url( if "403" in str(e): raise BasePermissionError( "OSS access denied (403)", - error_code=4024, ) from e raise @@ -646,7 +637,6 @@ async def to_bailian_data(files: List[FileSpec]) -> List[str]: f"max allowed " f"{DATASETS_FILE_SIZE_WARNING / (1024 * 1024):.0f}MB. " f"Adjust via env var DATASETS_FILE_SIZE_WARNING.", - error_code=4035, ) # Add file to form data @@ -668,7 +658,6 @@ async def to_bailian_data(files: List[FileSpec]) -> List[str]: if valid_file_count == 0: raise InputError( "No valid files found to upload. All files failed validation.", - error_code=4031, ) # Execute upload request @@ -686,7 +675,6 @@ async def to_bailian_data(files: List[FileSpec]) -> List[str]: if result.get("status", {}).get("code", 200) != 200: raise OutputError( f"File upload failed: {result}", - error_code=4032, ) data = result.get("data", {}) @@ -696,7 +684,6 @@ async def to_bailian_data(files: List[FileSpec]) -> List[str]: ) raise OutputError( f"Partial upload failed: {failed_files}", - error_code=4033, ) # Collect uploaded file IDs @@ -710,7 +697,6 @@ async def to_bailian_data(files: List[FileSpec]) -> List[str]: except Exception as e: raise OutputError( "File upload error", - error_code=4034, ) from e @@ -780,7 +766,6 @@ def set_api_key(api_key: Optional[str] = None) -> None: "DashScope API key is missing. " "Please provide 'api_key' argument or set the " "'DASHSCOPE_API_KEY' environment variable.", - error_code=4035, ) # 3. If env var exists, just log that we are using it (optional) @@ -811,13 +796,11 @@ def get_filepath_classname(full_path: str) -> Tuple[str, str]: raise InputError( f"Invalid format '{full_path}'. Expected " f"'path/to/file.py:ClassName'", - error_code=4041, ) if ":" in parts[1]: raise InputError( f"Invalid class name format '{parts[1]}'. " f"Class name cannot contain colon.", - error_code=4042, ) filepath, classname = parts[0].strip(), parts[1].strip() @@ -830,7 +813,6 @@ def get_filepath_classname(full_path: str) -> Tuple[str, str]: raise InputError( f"Invalid format '{full_path}'. Expected " f"'module.path.ClassName' or 'path/to/file.py:ClassName'", - error_code=4043, ) classname = parts[-1] module_path = ".".join(parts[:-1]) diff --git a/tests/unit/mock_request_base.py b/tests/unit/mock_request_base.py index c5c016b..ef3ee37 100644 --- a/tests/unit/mock_request_base.py +++ b/tests/unit/mock_request_base.py @@ -20,7 +20,7 @@ def setup_class(cls): super().setup_class() dashscope.base_http_api_url = "http://localhost:8089/api/v1/" dashscope.base_websocket_api_url = ( - "http://localhost:8089/api-ws/v1/inference" + "ws://localhost:8089/api-ws/v1/inference" ) dashscope.api_key = "default" dashscope.api_protocol = "http" diff --git a/tests/unit/test_agentic_rl_error_codes.py b/tests/unit/test_agentic_rl_error_codes.py new file mode 100644 index 0000000..fabea0c --- /dev/null +++ b/tests/unit/test_agentic_rl_error_codes.py @@ -0,0 +1,998 @@ +# -*- coding: utf-8 -*- +"""Tests for AgenticRL error handling with standard SDK exceptions. + +Validates that AgenticRL methods raise correct standard SDK exceptions +(AuthenticationError, InvalidParameter, DashScopeException) with proper +status_code and error_code attributes in error scenarios. +""" +from unittest.mock import patch, MagicMock, AsyncMock +import pytest + +from dashscope.common.error import ( + AuthenticationError, + InvalidParameter, + DashScopeException, +) +from dashscope.common.error_registry import ( + INTERNAL_ERROR, + INVALID_REQUEST, + PERMISSION_DENIED, + REQUEST_TIMEOUT, +) +from dashscope.finetune.agentic_rl import AgenticRL, _get_public_error +from dashscope.finetune.reinforcement import FunctionType +from dashscope.finetune.reinforcement import HTTP_REQUEST_TIMEOUT +from dashscope.finetune.reinforcement.common.errors import ( + BasePermissionError, + ConfigurationError, + InputError, + ValidationError, +) + + +class TestInitAuthenticationError: + """Test __init__ raises AuthenticationError for invalid API key.""" + + @patch("dashscope.finetune.agentic_rl.set_api_key") + def test_init_invalid_api_key_raises_authentication_error( + self, + mock_set_api_key, + ): + """__init__ should raise AuthenticationError when set_api_key fails.""" + mock_set_api_key.side_effect = ValueError("Invalid API key") + + with pytest.raises(AuthenticationError) as exc_info: + AgenticRL(api_key="invalid-key") + + assert exc_info.value.status_code == 401 + assert exc_info.value.error_code == "AuthenticationError" + assert "Incorrect API key" in str(exc_info.value) + + +class TestSubmitJobInvalidParameter: + """Test submit_job raises InvalidParameter for duplicate function names.""" + + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + @patch( + "dashscope.finetune.agentic_rl.generate_random_id", + return_value="abcd1234", + ) + def test_submit_job_duplicate_function_names_raises_invalid_parameter( + self, + _mock_random_id, + _mock_parent_init, + ): + """submit_job should raise InvalidParameter when + check_function_names returns False.""" + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.name = "test-job" + agent.tuning.model.name = "test-model" + agent.tuning.check_function_names.return_value = False + agent.tuning.find_function_name_problems.return_value = ( + ["rollout_fn"], + [], + ) + agent.tuning.datasets = [] + agent.tuning.combine_ids_runtimes.return_value = [] + agent.tuning.training.hyper_parameters = None + agent.tuning.training.resources = None + agent.tuning.training.type = "rl" + + with pytest.raises(InvalidParameter) as exc_info: + agent.submit_job() + + assert exc_info.value.status_code == 400 + assert exc_info.value.error_code == "BadRequestError" + assert "request is invalid" in str(exc_info.value).lower() + + +class TestRegisterFunctionsDashScopeException: + """Test register_functions raises DashScopeException on failure.""" + + @pytest.mark.asyncio + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + async def test_register_functions_failure_raises_dashscope_exception( + self, + _mock_parent_init, + ): + """register_functions should raise DashScopeException + when internal call fails.""" + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.register_functions = AsyncMock( + side_effect=RuntimeError("Registration failed"), + ) + + with pytest.raises(DashScopeException) as exc_info: + await agent.register_functions() + + assert exc_info.value.status_code == 500 + assert exc_info.value.error_code == "InternalServerError" + assert "internal error" in str(exc_info.value).lower() + + +class TestUploadDatasetsDashScopeException: + """Test upload_datasets raises DashScopeException on failure.""" + + @pytest.mark.asyncio + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + async def test_upload_datasets_failure_raises_dashscope_exception( + self, + _mock_parent_init, + ): + """upload_datasets should raise DashScopeException + when upload fails.""" + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.upload_datasets = AsyncMock( + side_effect=IOError("Upload failed"), + ) + + with pytest.raises(DashScopeException) as exc_info: + await agent.upload_datasets() + + assert exc_info.value.status_code == 500 + assert exc_info.value.error_code == "InternalServerError" + + +class TestSubmitJobCallFailure: + """Test submit_job raises DashScopeException when API call fails.""" + + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + @patch( + "dashscope.finetune.agentic_rl.generate_random_id", + return_value="abcd1234", + ) + @patch("dashscope.finetune.agentic_rl.CreateMixin.call") + def test_submit_job_call_failure_raises_dashscope_exception( + self, + mock_call, + _mock_random_id, + _mock_parent_init, + ): + """submit_job should raise DashScopeException + when super().call() fails.""" + mock_call.side_effect = RuntimeError("API call failed") + + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.name = "test-job" + agent.tuning.model.name = "test-model" + agent.tuning.check_function_names.return_value = True + agent.tuning.datasets = [] + agent.tuning.combine_ids_runtimes.return_value = [] + agent.tuning.training.hyper_parameters = None + agent.tuning.training.resources = None + agent.tuning.training.type = "rl" + + with pytest.raises(DashScopeException) as exc_info: + agent.submit_job() + + assert exc_info.value.status_code == 500 + assert exc_info.value.error_code == "InternalServerError" + + +class TestRunFailure: + """Test run raises DashScopeException on failure.""" + + @pytest.mark.asyncio + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + async def test_run_failure_raises_dashscope_exception( + self, + _mock_parent_init, + ): + """run should raise DashScopeException when any step fails.""" + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + object.__setattr__( + agent, + "register_functions", + AsyncMock( + side_effect=RuntimeError("Step failed"), + ), + ) + + with pytest.raises(DashScopeException) as exc_info: + await agent.run() + + assert exc_info.value.status_code == 500 + assert exc_info.value.error_code == "InternalServerError" + + +class TestTestFunctionsInvalidParameter: + """Test test_functions raises InvalidParameter for + unsupported types and validation failures.""" + + @pytest.mark.asyncio + @patch("dashscope.finetune.agentic_rl.set_api_key") + async def test_test_functions_unsupported_type_raises_invalid_parameter( + self, + _mock_set_api_key, + ): + """test_functions should raise InvalidParameter + for unsupported FunctionType.""" + with pytest.raises(InvalidParameter) as exc_info: + await AgenticRL.test_functions( + instance_id="inst-123", + functype=MagicMock(spec=FunctionType), # Unsupported type + input_data={}, + ) + + assert exc_info.value.status_code == 400 + assert exc_info.value.error_code == "BadRequestError" + + @pytest.mark.asyncio + @patch("dashscope.finetune.agentic_rl.set_api_key") + @patch("dashscope.finetune.agentic_rl.RolloutInput.model_validate") + async def test_test_functions_validation_failure_raises_invalid_parameter( + self, + mock_validate, + _mock_set_api_key, + ): + """test_functions should raise InvalidParameter + when validation fails.""" + mock_validate.side_effect = ValueError("Validation failed") + + with pytest.raises(InvalidParameter) as exc_info: + await AgenticRL.test_functions( + instance_id="inst-123", + functype=FunctionType.ROLLOUT, + input_data={"invalid": "data"}, + ) + + assert exc_info.value.status_code == 400 + assert exc_info.value.error_code == "BadRequestError" + + +class TestDashScopeExceptionPassthrough: + """Test that DashScopeException from API is passed through unchanged.""" + + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + @patch( + "dashscope.finetune.agentic_rl.generate_random_id", + return_value="abcd1234", + ) + @patch("dashscope.finetune.agentic_rl.CreateMixin.call") + def test_submit_job_passthrough_dashscope_exception( + self, + mock_call, + _mock_random_id, + _mock_parent_init, + ): + """submit_job should pass through DashScopeException + from API unchanged.""" + original_exc = DashScopeException("API error") + original_exc.status_code = 429 + original_exc.error_code = "RateLimitError" + original_exc.request_id = "req-123" + mock_call.side_effect = original_exc + + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.name = "test-job" + agent.tuning.model.name = "test-model" + agent.tuning.check_function_names.return_value = True + agent.tuning.datasets = [] + agent.tuning.combine_ids_runtimes.return_value = [] + agent.tuning.training.hyper_parameters = None + agent.tuning.training.resources = None + agent.tuning.training.type = "rl" + + with pytest.raises(DashScopeException) as exc_info: + agent.submit_job() + + # Should be the exact same exception object + assert exc_info.value is original_exc + assert exc_info.value.status_code == 429 + assert exc_info.value.error_code == "RateLimitError" + assert exc_info.value.request_id == "req-123" + + @pytest.mark.asyncio + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + async def test_register_functions_passthrough_dashscope_exception( + self, + _mock_parent_init, + ): + """register_functions should pass through + DashScopeException unchanged.""" + original_exc = DashScopeException("Service error") + original_exc.status_code = 503 + original_exc.error_code = "ServiceUnavailableError" + + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.register_functions = AsyncMock(side_effect=original_exc) + + with pytest.raises(DashScopeException) as exc_info: + await agent.register_functions() + + assert exc_info.value is original_exc + assert exc_info.value.status_code == 503 + assert exc_info.value.error_code == "ServiceUnavailableError" + + @pytest.mark.asyncio + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + async def test_run_passthrough_dashscope_exception( + self, + _mock_parent_init, + ): + """run should pass through DashScopeException + from inner calls unchanged.""" + original_exc = DashScopeException("Inner error") + original_exc.status_code = 400 + original_exc.error_code = "BadRequestError" + + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + object.__setattr__( + agent, + "register_functions", + AsyncMock(side_effect=original_exc), + ) + + with pytest.raises(DashScopeException) as exc_info: + await agent.run() + + assert exc_info.value is original_exc + assert exc_info.value.status_code == 400 + assert exc_info.value.error_code == "BadRequestError" + + +class TestExceptionChainPreserved: + """Verify that __cause__ is correctly preserved in all error scenarios.""" + + @patch("dashscope.finetune.agentic_rl.set_api_key") + def test_init_preserves_cause(self, mock_set_api_key): + """__init__ should preserve the original exception as __cause__.""" + original = ValueError("bad key") + mock_set_api_key.side_effect = original + + with pytest.raises(AuthenticationError) as exc_info: + AgenticRL(api_key="invalid") + + assert exc_info.value.__cause__ is original + + @pytest.mark.asyncio + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + async def test_register_functions_preserves_cause(self, _mock_parent_init): + """register_functions should preserve __cause__ + for non-DashScopeException.""" + original = IOError("network down") + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.register_functions = AsyncMock(side_effect=original) + + with pytest.raises(DashScopeException) as exc_info: + await agent.register_functions() + + assert exc_info.value.__cause__ is original + + @pytest.mark.asyncio + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + async def test_upload_datasets_preserves_cause(self, _mock_parent_init): + """upload_datasets should preserve __cause__.""" + original = OSError("disk full") + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.upload_datasets = AsyncMock(side_effect=original) + + with pytest.raises(DashScopeException) as exc_info: + await agent.upload_datasets() + + assert exc_info.value.__cause__ is original + + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + @patch( + "dashscope.finetune.agentic_rl.generate_random_id", + return_value="abcd1234", + ) + @patch("dashscope.finetune.agentic_rl.CreateMixin.call") + def test_submit_job_call_failure_preserves_cause( + self, + mock_call, + _mock_random_id, + _mock_parent_init, + ): + """submit_job should preserve __cause__ when API call fails.""" + original = TimeoutError("connection timeout") + mock_call.side_effect = original + + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.name = "test-job" + agent.tuning.model.name = "test-model" + agent.tuning.check_function_names.return_value = True + agent.tuning.datasets = [] + agent.tuning.combine_ids_runtimes.return_value = [] + agent.tuning.training.hyper_parameters = None + agent.tuning.training.resources = None + agent.tuning.training.type = "rl" + + with pytest.raises(DashScopeException) as exc_info: + agent.submit_job() + + assert exc_info.value.__cause__ is original + + @pytest.mark.asyncio + @patch("dashscope.finetune.agentic_rl.set_api_key") + async def test_test_functions_validation_preserves_cause( + self, + _mock_set_api_key, + ): + """test_functions should preserve __cause__ on validation failure.""" + original = TypeError("wrong type") + + with patch( + "dashscope.finetune.agentic_rl.RolloutInput.model_validate", + side_effect=original, + ): + with pytest.raises(InvalidParameter) as exc_info: + await AgenticRL.test_functions( + instance_id="inst-1", + functype=FunctionType.ROLLOUT, + input_data={"bad": "data"}, + ) + + assert exc_info.value.__cause__ is original + + +class TestInternalLogCodes: + """Verify each failure path logs its internal registry code.""" + + @patch("dashscope.finetune.agentic_rl.set_api_key") + @patch("dashscope.finetune.agentic_rl.logger") + def test_init_logs_invalid_api_key(self, mock_logger, mock_set_api_key): + """__init__ should log sdk.InvalidApiKey.""" + mock_set_api_key.side_effect = ValueError("bad") + + with pytest.raises(AuthenticationError): + AgenticRL(api_key="x") + + mock_logger.error.assert_called_once() + # Logger uses placeholder format: "[%s] %s | %s", + # name, message, exception + assert mock_logger.error.call_args[0][1] == "sdk.InvalidApiKey" + + @pytest.mark.asyncio + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + @patch("dashscope.finetune.agentic_rl.logger") + async def test_register_functions_logs_registration_failed( + self, + mock_logger, + _mock_parent_init, + ): + """register_functions should log + sdk.agentic_rl.FunctionRegistrationFailed.""" + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.register_functions = AsyncMock( + side_effect=RuntimeError("fail"), + ) + + with pytest.raises(DashScopeException): + await agent.register_functions() + + mock_logger.error.assert_called_once() + # Logger uses placeholder format: "[%s] %s | %s", + # name, message, exception + assert ( + mock_logger.error.call_args[0][1] + == "sdk.agentic_rl.FunctionRegistrationFailed" + ) + + @pytest.mark.asyncio + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + @patch("dashscope.finetune.agentic_rl.logger") + async def test_upload_datasets_logs_upload_failed( + self, + mock_logger, + _mock_parent_init, + ): + """upload_datasets should log + sdk.agentic_rl.DatasetsUploadFailed.""" + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.upload_datasets = AsyncMock(side_effect=IOError("fail")) + + with pytest.raises(DashScopeException): + await agent.upload_datasets() + + mock_logger.error.assert_called_once() + # Logger uses placeholder format: "[%s] %s | %s", + # name, message, exception + assert ( + mock_logger.error.call_args[0][1] + == "sdk.agentic_rl.DatasetsUploadFailed" + ) + + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + @patch( + "dashscope.finetune.agentic_rl.generate_random_id", + return_value="abcd1234", + ) + @patch("dashscope.finetune.agentic_rl.logger") + def test_submit_job_duplicate_names_logs_duplicate_names( + self, + mock_logger, + _mock_random_id, + _mock_parent_init, + ): + """submit_job duplicate names should log + sdk.agentic_rl.DuplicateFunctionNames.""" + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.name = "test-job" + agent.tuning.model.name = "test-model" + agent.tuning.check_function_names.return_value = False + agent.tuning.find_function_name_problems.return_value = ( + ["rollout_fn"], + [], + ) + agent.tuning.datasets = [] + agent.tuning.combine_ids_runtimes.return_value = [] + agent.tuning.training.hyper_parameters = None + agent.tuning.training.resources = None + agent.tuning.training.type = "rl" + + with pytest.raises(InvalidParameter): + agent.submit_job() + + mock_logger.error.assert_called_once() + # Logger uses placeholder format: "[%s] %s", name, message + assert ( + mock_logger.error.call_args[0][1] + == "sdk.agentic_rl.DuplicateFunctionNames" + ) + rendered = mock_logger.error.call_args[0][2] + assert "rollout_fn" in rendered + assert "check_function_names() failed" not in rendered + assert "{" not in rendered + + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + @patch( + "dashscope.finetune.agentic_rl.generate_random_id", + return_value="abcd1234", + ) + @patch("dashscope.finetune.agentic_rl.logger") + def test_submit_job_missing_name_is_not_reported_as_duplicate( + self, + mock_logger, + _mock_random_id, + _mock_parent_init, + ): + """check_function_names() also fails when a component carries no + `name` attribute; that must not be logged as a duplication.""" + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.name = "test-job" + agent.tuning.model.name = "test-model" + agent.tuning.check_function_names.return_value = False + agent.tuning.find_function_name_problems.return_value = ([], [2]) + agent.tuning.datasets = [] + agent.tuning.combine_ids_runtimes.return_value = [] + agent.tuning.training.hyper_parameters = None + agent.tuning.training.resources = None + agent.tuning.training.type = "rl" + + with pytest.raises(InvalidParameter) as exc_info: + agent.submit_job() + + assert exc_info.value.status_code == 400 + mock_logger.error.assert_called_once() + assert ( + mock_logger.error.call_args[0][1] + == "sdk.agentic_rl.ValidationError" + ) + assert "Duplicate" not in mock_logger.error.call_args[0][2] + + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + @patch( + "dashscope.finetune.agentic_rl.generate_random_id", + return_value="abcd1234", + ) + @patch("dashscope.finetune.agentic_rl.CreateMixin.call") + @patch("dashscope.finetune.agentic_rl.logger") + def test_submit_job_call_failure_logs_submission_failed( + self, + mock_logger, + mock_call, + _mock_random_id, + _mock_parent_init, + ): + """submit_job call failure should log + sdk.agentic_rl.JobSubmissionFailed.""" + mock_call.side_effect = RuntimeError("API down") + + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.name = "test-job" + agent.tuning.model.name = "test-model" + agent.tuning.check_function_names.return_value = True + agent.tuning.datasets = [] + agent.tuning.combine_ids_runtimes.return_value = [] + agent.tuning.training.hyper_parameters = None + agent.tuning.training.resources = None + agent.tuning.training.type = "rl" + + with pytest.raises(DashScopeException): + agent.submit_job() + + mock_logger.error.assert_called_once() + # Logger uses placeholder format: "[%s] %s | %s", + # name, message, exception + assert ( + mock_logger.error.call_args[0][1] + == "sdk.agentic_rl.JobSubmissionFailed" + ) + + @pytest.mark.asyncio + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + @patch("dashscope.finetune.agentic_rl.logger") + async def test_run_logs_workflow_failed( + self, + mock_logger, + _mock_parent_init, + ): + """run should log sdk.agentic_rl.WorkflowFailed on failure.""" + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + object.__setattr__( + agent, + "register_functions", + AsyncMock(side_effect=RuntimeError("step failed")), + ) + + with pytest.raises(DashScopeException): + await agent.run() + + mock_logger.error.assert_called_once() + # Logger uses placeholder format: "[%s] %s | %s", + # name, message, exception + assert ( + mock_logger.error.call_args[0][1] + == "sdk.agentic_rl.WorkflowFailed" + ) + + @pytest.mark.asyncio + @patch("dashscope.finetune.agentic_rl.set_api_key") + @patch("dashscope.finetune.agentic_rl.logger") + async def test_test_functions_unsupported_type_logs_unsupported_type( + self, + mock_logger, + _mock_set_api_key, + ): + """test_functions unsupported type should log + sdk.agentic_rl.UnsupportedFunctionType.""" + with pytest.raises(InvalidParameter): + await AgenticRL.test_functions( + instance_id="inst-1", + functype=MagicMock(spec=FunctionType), + input_data={}, + ) + + # Check all logged messages for the error name + error_names = [call[0][1] for call in mock_logger.error.call_args_list] + assert "sdk.agentic_rl.UnsupportedFunctionType" in error_names + assert "sdk.agentic_rl.FunctionTestFailed" not in error_names + + @pytest.mark.asyncio + @patch("dashscope.finetune.agentic_rl.set_api_key") + @patch("dashscope.finetune.agentic_rl.logger") + async def test_test_functions_validation_failure_logs_test_failed( + self, + mock_logger, + _mock_set_api_key, + ): + """test_functions validation failure should log + sdk.agentic_rl.FunctionTestFailed.""" + with patch( + "dashscope.finetune.agentic_rl.RolloutInput.model_validate", + side_effect=ValueError("bad input"), + ): + with pytest.raises(InvalidParameter): + await AgenticRL.test_functions( + instance_id="inst-1", + functype=FunctionType.ROLLOUT, + input_data={"bad": "data"}, + ) + + mock_logger.error.assert_called_once() + # Logger uses placeholder format: "[%s] %s | %s", + # name, message, exception + assert ( + mock_logger.error.call_args[0][1] + == "sdk.agentic_rl.FunctionTestFailed" + ) + + @pytest.mark.asyncio + @patch("dashscope.finetune.agentic_rl.set_api_key") + @patch("dashscope.finetune.agentic_rl.logger") + async def test_test_functions_timeout_logs_configured_timeout( + self, + mock_logger, + _mock_set_api_key, + ): + """The {timeout} variable must carry the configured request timeout + rather than the literal "unknown".""" + with patch( + "dashscope.finetune.agentic_rl.RolloutInput.model_validate", + return_value=MagicMock(), + ): + with patch( + "dashscope.finetune.agentic_rl." + "AgenticRLFunctionComponent.verify_function", + side_effect=TimeoutError("too slow"), + ): + with pytest.raises(DashScopeException) as exc_info: + await AgenticRL.test_functions( + instance_id="inst-1", + functype=FunctionType.ROLLOUT, + input_data={}, + ) + + assert exc_info.value.status_code == 504 + rendered = mock_logger.error.call_args[0][2] + assert ( + mock_logger.error.call_args[0][1] + == "sdk.agentic_rl.FunctionTestTimeout" + ) + assert f"after {HTTP_REQUEST_TIMEOUT} seconds" in rendered + assert "unknown seconds" not in rendered + assert "{" not in rendered + + +class TestPassthroughAttributeCompleteness: + """Verify that passthrough DashScopeException preserves ALL attributes.""" + + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + @patch( + "dashscope.finetune.agentic_rl.generate_random_id", + return_value="abcd1234", + ) + @patch("dashscope.finetune.agentic_rl.CreateMixin.call") + def test_submit_job_passthrough_preserves_all_attributes( + self, + mock_call, + _mock_random_id, + _mock_parent_init, + ): + """Passthrough should preserve status_code, + error_code, request_id, and message.""" + original = DashScopeException("Rate limited by server") + original.status_code = 429 + original.error_code = "RateLimitError" + original.request_id = "req-abc-123" + mock_call.side_effect = original + + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.name = "test-job" + agent.tuning.model.name = "test-model" + agent.tuning.check_function_names.return_value = True + agent.tuning.datasets = [] + agent.tuning.combine_ids_runtimes.return_value = [] + agent.tuning.training.hyper_parameters = None + agent.tuning.training.resources = None + agent.tuning.training.type = "rl" + + with pytest.raises(DashScopeException) as exc_info: + agent.submit_job() + + assert exc_info.value is original + assert exc_info.value.status_code == 429 + assert exc_info.value.error_code == "RateLimitError" + assert exc_info.value.request_id == "req-abc-123" + assert "Rate limited" in str(exc_info.value) + + +class TestMultipleUnderlyingExceptionTypes: + """Verify correct behavior with various underlying exception types.""" + + @pytest.mark.asyncio + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + @pytest.mark.parametrize( + "underlying_exc, expected_status, expected_code", + [ + (IOError("disk error"), 500, "InternalServerError"), + (TimeoutError("timed out"), 504, "GatewayTimeoutError"), + ( + ConnectionError("connection refused"), + 500, + "InternalServerError", + ), + (PermissionError("access denied"), 500, "InternalServerError"), + (MemoryError("out of memory"), 500, "InternalServerError"), + ], + ) + async def test_register_functions_handles_various_exceptions( + self, + _mock_parent_init, + underlying_exc, + expected_status, + expected_code, + ): + """register_functions should convert various + exception types to DashScopeException.""" + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.register_functions = AsyncMock(side_effect=underlying_exc) + + with pytest.raises(DashScopeException) as exc_info: + await agent.register_functions() + + assert exc_info.value.status_code == expected_status + assert exc_info.value.error_code == expected_code + assert exc_info.value.__cause__ is underlying_exc + + @pytest.mark.asyncio + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + @pytest.mark.parametrize( + "underlying_exc", + [ + OSError("os error"), + RuntimeError("runtime error"), + SystemError("system error"), + ], + ) + async def test_upload_datasets_handles_various_exceptions( + self, + _mock_parent_init, + underlying_exc, + ): + """upload_datasets should convert various + exception types to DashScopeException.""" + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + agent.tuning.upload_datasets = AsyncMock(side_effect=underlying_exc) + + with pytest.raises(DashScopeException) as exc_info: + await agent.upload_datasets() + + assert exc_info.value.status_code == 500 + assert exc_info.value.error_code == "InternalServerError" + assert exc_info.value.__cause__ is underlying_exc + + +class TestPublicErrorMapping: + """Lock the exception -> public-error mapping table. + + ``test_functions`` used to carry a second copy of this table and the two + drifted apart; both now route through ``_get_public_error``. + """ + + @pytest.mark.parametrize( + "underlying_exc, expected_error", + [ + (BasePermissionError("denied"), PERMISSION_DENIED), + (ConfigurationError("bad config"), INVALID_REQUEST), + (ValidationError("bad payload"), INVALID_REQUEST), + (InputError("bad field"), INVALID_REQUEST), + (TimeoutError("too slow"), REQUEST_TIMEOUT), + (OSError("io"), INTERNAL_ERROR), + (RuntimeError("boom"), INTERNAL_ERROR), + (ValueError("plain"), INTERNAL_ERROR), + (TypeError("plain"), INTERNAL_ERROR), + ], + ) + def test_default_mapping(self, underlying_exc, expected_error): + assert _get_public_error(underlying_exc) is expected_error + + @pytest.mark.parametrize( + "underlying_exc", + [ValueError("bad payload"), TypeError("bad payload")], + ) + def test_input_errors_opt_in_to_400(self, underlying_exc): + """A flow that validates a payload before sending it can classify a + generic value/type failure as the caller's fault.""" + mapped = _get_public_error( + underlying_exc, + input_errors=(ValueError, TypeError), + ) + assert mapped is INVALID_REQUEST + + def test_input_errors_do_not_override_permission(self): + mapped = _get_public_error( + BasePermissionError("denied"), + input_errors=(BasePermissionError,), + ) + assert mapped is PERMISSION_DENIED + + def test_input_errors_do_not_swallow_timeout(self): + mapped = _get_public_error( + TimeoutError("too slow"), + input_errors=(ValueError, TypeError), + ) + assert mapped is REQUEST_TIMEOUT + + @pytest.mark.asyncio + @patch("dashscope.finetune.agentic_rl.set_api_key") + @patch("dashscope.finetune.agentic_rl.RolloutInput.model_validate") + @pytest.mark.parametrize( + "underlying_exc, expected_status, expected_code", + [ + (ValueError("bad payload"), 400, "BadRequestError"), + (TypeError("bad payload"), 400, "BadRequestError"), + (TimeoutError("too slow"), 504, "GatewayTimeoutError"), + (OSError("io"), 500, "InternalServerError"), + ], + ) + async def test_functions_uses_the_shared_mapping( + self, + mock_validate, + _mock_set_api_key, + underlying_exc, + expected_status, + expected_code, + ): + mock_validate.side_effect = underlying_exc + + with pytest.raises(DashScopeException) as exc_info: + await AgenticRL.test_functions( + instance_id="inst-1", + functype=FunctionType.ROLLOUT, + input_data={}, + ) + + assert exc_info.value.status_code == expected_status + assert exc_info.value.error_code == expected_code diff --git a/tests/unit/test_agentic_rl_init_errors.py b/tests/unit/test_agentic_rl_init_errors.py new file mode 100644 index 0000000..d721a3f --- /dev/null +++ b/tests/unit/test_agentic_rl_init_errors.py @@ -0,0 +1,309 @@ +# -*- coding: utf-8 -*- +"""Tests for AgenticRL.init() error reporting and the shared error helpers. + +``init()`` used to let ``IOErrorWithCode`` escape untouched, so a caller who +passed a bad config path got an exception with no ``status_code`` and no public +error code. The log-and-classify sequence was also copy-pasted into five other +methods; these tests pin the extracted helpers so the call sites can stay short. +""" +import logging +from unittest.mock import MagicMock, patch + +import pytest + +from dashscope.common.error import DashScopeException, InvalidParameter +from dashscope.common.error_registry import ( + INTERNAL_ERROR, + INVALID_REQUEST, + PERMISSION_DENIED, + REQUEST_TIMEOUT, +) +from dashscope.finetune import agentic_rl +from dashscope.finetune.agentic_rl import ( + AgenticRL, + _log_internal_error, + _public_exception, +) +from dashscope.finetune.reinforcement import FunctionType, HTTP_REQUEST_TIMEOUT +from dashscope.finetune.reinforcement.common.errors import ( + IOErrorWithCode, + RuntimeErrorWithCode, +) + +_MODULE = "dashscope.finetune.agentic_rl" + + +def _agent(): + """Build an AgenticRL without running ``__init__`` (which needs a key).""" + return AgenticRL.__new__(AgenticRL) + + +class TestInitReportsConfigProblemsAsBadRequest: + def test_missing_config_file_has_a_status_code(self): + agent = _agent() + + with pytest.raises(DashScopeException) as exc_info: + agent.init("/nonexistent/agentic-rl-config.yaml") + + assert exc_info.value.status_code == 400 + assert exc_info.value.error_code == INVALID_REQUEST.error_code + + def test_missing_config_file_is_not_an_internal_error(self): + """A path the caller got wrong must not look like a server failure.""" + agent = _agent() + + with pytest.raises(DashScopeException) as exc_info: + agent.init("/nonexistent/agentic-rl-config.yaml") + + assert exc_info.value.status_code != INTERNAL_ERROR.status_code + + def test_missing_config_file_is_an_invalid_parameter(self): + agent = _agent() + + with pytest.raises(InvalidParameter): + agent.init("/nonexistent/agentic-rl-config.yaml") + + def test_message_names_the_file_that_failed(self, tmp_path): + missing = str(tmp_path / "gone.yaml") + agent = _agent() + + with pytest.raises(DashScopeException) as exc_info: + agent.init(missing) + + assert missing in str(exc_info.value) + + def test_original_io_error_is_chained(self): + agent = _agent() + + with pytest.raises(DashScopeException) as exc_info: + agent.init("/nonexistent/agentic-rl-config.yaml") + + assert isinstance(exc_info.value.__cause__, IOErrorWithCode) + + def test_malformed_config_is_also_a_bad_request(self, tmp_path): + """``load_from_dict`` reports unknown keys through IOErrorWithCode.""" + config = tmp_path / "bad.yaml" + config.write_text("not_a_real_field: 1\n", encoding="utf-8") + agent = _agent() + + with pytest.raises(DashScopeException) as exc_info: + agent.init(str(config)) + + assert exc_info.value.status_code == 400 + assert exc_info.value.error_code == INVALID_REQUEST.error_code + + def test_empty_config_path_is_a_bad_request(self): + """``init()`` with no argument must not surface an unwrapped IOError.""" + agent = _agent() + + with pytest.raises(DashScopeException) as exc_info: + agent.init() + + assert exc_info.value.status_code == 400 + + @patch(f"{_MODULE}.logger") + def test_failure_is_logged_as_a_configuration_error(self, mock_logger): + agent = _agent() + + with pytest.raises(DashScopeException): + agent.init("/nonexistent/agentic-rl-config.yaml") + + mock_logger.error.assert_called_once() + assert ( + mock_logger.error.call_args[0][1] + == "sdk.agentic_rl.ConfigurationError" + ) + + @patch(f"{_MODULE}.logger") + def test_log_message_has_no_unfilled_placeholder(self, mock_logger): + agent = _agent() + + with pytest.raises(DashScopeException): + agent.init("/nonexistent/agentic-rl-config.yaml") + + rendered = mock_logger.error.call_args[0][2] + assert "{" not in rendered + assert rendered == "Invalid system configuration detected." + + +class _Collect(logging.Handler): + """Collects records straight off the SDK logger. + + ``caplog`` attaches at the root, and the SDK logger does not propagate, so + it sees nothing here. + """ + + def __init__(self, sink): + super().__init__(level=logging.ERROR) + self._sink = sink + + def emit(self, record): + self._sink.append(record) + + +class TestLogInternalError: + def test_keeps_the_positional_log_shape(self): + error_def = MagicMock() + error_def.name = "sdk.agentic_rl.SomeFailure" + error_def.format_message.return_value = "rendered message" + cause = RuntimeErrorWithCode("boom") + + _log_internal_error(error_def, cause) + + args, kwargs = error_def.format_message.call_args + assert error_def.name == "sdk.agentic_rl.SomeFailure" + assert args == ({"inner_code": "sdk.agentic_rl.RuntimeErrorWithCode"},) + assert kwargs == {} + + def test_inner_code_falls_back_to_unknown(self): + mock_logger = MagicMock() + with patch(f"{_MODULE}.logger", mock_logger): + _log_internal_error(MagicMock(name="def"), ValueError("boom")) + + assert mock_logger.error.call_args[0][3] == "unknown" + + def test_extra_vars_reach_the_rendered_message(self): + error_def = MagicMock() + error_def.format_message.return_value = "timed out after 300 seconds" + cause = RuntimeErrorWithCode("boom") + + with patch(f"{_MODULE}.logger"): + _log_internal_error(error_def, cause, {"timeout": "300"}) + + variables = error_def.format_message.call_args[0][0] + assert variables["timeout"] == "300" + assert "inner_code" in variables + + def test_cause_is_logged_with_a_traceback(self): + mock_logger = MagicMock() + cause = RuntimeErrorWithCode("boom") + + with patch(f"{_MODULE}.logger", mock_logger): + _log_internal_error(MagicMock(), cause) + + assert mock_logger.error.call_args[0][4] is cause + assert mock_logger.error.call_args.kwargs["exc_info"] is True + + def test_record_is_attributed_to_the_caller(self): + """Every handler logs through this helper, so without the stacklevel + the record's location would be the same line for all five.""" + error_def = MagicMock() + error_def.name = "sdk.agentic_rl.SomeFailure" + error_def.format_message.return_value = "rendered" + records = [] + handler = _Collect(records) + + agentic_rl.logger.addHandler(handler) + try: + _log_internal_error(error_def, RuntimeErrorWithCode("boom")) + finally: + agentic_rl.logger.removeHandler(handler) + + assert records[-1].funcName == ( + "test_record_is_attributed_to_the_caller" + ) + + +class TestPublicException: + def test_bad_request_keeps_the_narrower_type(self): + exc = _public_exception(INVALID_REQUEST, ValueError("bad payload")) + + assert isinstance(exc, InvalidParameter) + assert exc.status_code == 400 + assert exc.error_code == INVALID_REQUEST.error_code + + @pytest.mark.parametrize( + "public_error", + [INTERNAL_ERROR, PERMISSION_DENIED, REQUEST_TIMEOUT], + ) + def test_other_statuses_are_plain_dashscope_exceptions(self, public_error): + exc = _public_exception(public_error, RuntimeError("boom")) + + assert type(exc) is DashScopeException + assert exc.status_code == public_error.status_code + assert exc.error_code == public_error.error_code + + def test_message_keeps_the_cause_summary(self): + exc = _public_exception( + INTERNAL_ERROR, + RuntimeErrorWithCode("upload exploded"), + ) + + assert "RuntimeErrorWithCode" in str(exc) + assert "upload exploded" in str(exc) + + def test_message_prefers_the_clean_message_attribute(self): + """``str()`` on an AgenticRLError carries the registry prefix and the + ``(caused by ...)`` tail; the caller-facing text should not.""" + cause = IOErrorWithCode( + "Failed to load YAML file: /tmp/x.yaml", + path="/tmp/x.yaml", + ) + + exc = _public_exception(INVALID_REQUEST, cause) + + assert "I/O error: Failed to load YAML file" in str(exc) + assert "General I/O operation failure" not in str(exc) + + +class TestFunctionTestLogMessages: + """Guards the rendered log text after the helper extraction.""" + + async def _run(self, error): + """Drive test_functions into ``error``; the caller asserts on it.""" + with patch(f"{_MODULE}.set_api_key"), patch( + f"{_MODULE}.RolloutInput.model_validate", + return_value=MagicMock( + model_dump=MagicMock(return_value={}), + ), + ), patch( + f"{_MODULE}.AgenticRLFunctionComponent.verify_function", + side_effect=error, + ): + await AgenticRL.test_functions( + "inst-1", + FunctionType.ROLLOUT, + {"prompt": "hi"}, + ) + + @pytest.mark.asyncio + async def test_timeout_renders_the_timeout_message(self): + error = TimeoutError("too slow") + + with patch(f"{_MODULE}.logger") as mock_logger: + with pytest.raises(DashScopeException): + await self._run(error) + + assert ( + mock_logger.error.call_args[0][1] + == "sdk.agentic_rl.FunctionTestTimeout" + ) + rendered = mock_logger.error.call_args[0][2] + assert rendered == ( + f"Function test timed out after {HTTP_REQUEST_TIMEOUT} seconds." + ) + + @pytest.mark.asyncio + async def test_timeout_maps_to_504(self): + with pytest.raises(DashScopeException) as exc_info: + await self._run(TimeoutError("too slow")) + + assert exc_info.value.status_code == REQUEST_TIMEOUT.status_code + + @pytest.mark.asyncio + async def test_failure_renders_the_inner_code(self): + error = RuntimeErrorWithCode("deploy broke") + + with patch(f"{_MODULE}.logger") as mock_logger: + with pytest.raises(DashScopeException): + await self._run(error) + + assert ( + mock_logger.error.call_args[0][1] + == "sdk.agentic_rl.FunctionTestFailed" + ) + rendered = mock_logger.error.call_args[0][2] + assert rendered == ( + "Function test failed: sdk.agentic_rl.RuntimeErrorWithCode." + ) + assert "{" not in rendered diff --git a/tests/unit/test_agentic_rl_model.py b/tests/unit/test_agentic_rl_model.py index 0559079..d80e562 100644 --- a/tests/unit/test_agentic_rl_model.py +++ b/tests/unit/test_agentic_rl_model.py @@ -243,7 +243,7 @@ async def test_register_functions_failure(self, agentic_rl_tuning): await agentic_rl_tuning.tuning.register_functions() assert "Registration failed" in str(exc_info.value) - assert exc_info.value.error_code == 2052 + assert exc_info.value.error_code == "sdk.agentic_rl.RegistrationError" # pylint: disable=redefined-outer-name @pytest.mark.asyncio diff --git a/tests/unit/test_agentic_rl_submit_job.py b/tests/unit/test_agentic_rl_submit_job.py index 9fc79ff..b1ce499 100644 --- a/tests/unit/test_agentic_rl_submit_job.py +++ b/tests/unit/test_agentic_rl_submit_job.py @@ -507,15 +507,13 @@ def test_init_with_valid_api_key(self): def test_init_with_invalid_api_key(self): from dashscope.finetune.agentic_rl import AgenticRL - from dashscope.finetune.reinforcement.common.errors import ( - ValueErrorWithCode, - ) + from dashscope.common.error import DashScopeException with patch( "dashscope.finetune.agentic_rl.set_api_key", side_effect=Exception("Invalid key"), ): - with pytest.raises(ValueErrorWithCode): + with pytest.raises(DashScopeException): AgenticRL(api_key="invalid") def test_init_without_api_key(self): @@ -603,9 +601,7 @@ async def test_register_functions_success(self): @pytest.mark.asyncio async def test_register_functions_failure(self): from dashscope.finetune.agentic_rl import AgenticRL - from dashscope.finetune.reinforcement.common.errors import ( - RegistrationError, - ) + from dashscope.common.error import DashScopeException client = AgenticRL() client.tuning = MagicMock() @@ -614,7 +610,7 @@ async def test_register_functions_failure(self): ) client.tuning.functions = [] - with pytest.raises(RegistrationError): + with pytest.raises(DashScopeException): await client.register_functions() @pytest.mark.asyncio @@ -634,9 +630,7 @@ async def test_upload_datasets_success(self): @pytest.mark.asyncio async def test_upload_datasets_failure(self): from dashscope.finetune.agentic_rl import AgenticRL - from dashscope.finetune.reinforcement.common.errors import ( - DatasetsError, - ) + from dashscope.common.error import DashScopeException client = AgenticRL() client.tuning = MagicMock() @@ -645,7 +639,7 @@ async def test_upload_datasets_failure(self): ) client.tuning.datasets = [] - with pytest.raises(DatasetsError): + with pytest.raises(DashScopeException): await client.upload_datasets() @@ -692,9 +686,7 @@ def test_submit_job_builds_correct_request(self): def test_submit_job_duplicate_function_names(self): from dashscope.finetune.agentic_rl import AgenticRL - from dashscope.finetune.reinforcement.common.errors import ( - ValueErrorWithCode, - ) + from dashscope.common.error import DashScopeException client = AgenticRL() client.tuning = MagicMock() @@ -710,8 +702,11 @@ def test_submit_job_duplicate_function_names(self): client.tuning.check_function_names = MagicMock( return_value=False, ) + client.tuning.find_function_name_problems = MagicMock( + return_value=(["rollout_fn"], []), + ) - with pytest.raises(ValueErrorWithCode, match="Duplicate"): + with pytest.raises(DashScopeException, match="request is invalid"): client.submit_job() @@ -846,9 +841,7 @@ async def test_run_full_workflow( @pytest.mark.asyncio async def test_run_register_failure_raises(self): from dashscope.finetune.agentic_rl import AgenticRL - from dashscope.finetune.reinforcement.common.errors import ( - RuntimeErrorWithCode, - ) + from dashscope.common.error import DashScopeException with patch.object( AgenticRL, @@ -859,15 +852,13 @@ async def test_run_register_failure_raises(self): client = AgenticRL() client.tuning = MagicMock() - with pytest.raises(RuntimeErrorWithCode): + with pytest.raises(DashScopeException): await client.run(model="qwen3.5-9b") @pytest.mark.asyncio async def test_run_upload_failure_raises(self): from dashscope.finetune.agentic_rl import AgenticRL - from dashscope.finetune.reinforcement.common.errors import ( - RuntimeErrorWithCode, - ) + from dashscope.common.error import DashScopeException with patch.object( AgenticRL, @@ -882,15 +873,13 @@ async def test_run_upload_failure_raises(self): client = AgenticRL() client.tuning = MagicMock() - with pytest.raises(RuntimeErrorWithCode): + with pytest.raises(DashScopeException): await client.run(model="qwen3.5-9b") @pytest.mark.asyncio async def test_run_submit_failure_raises(self): from dashscope.finetune.agentic_rl import AgenticRL - from dashscope.finetune.reinforcement.common.errors import ( - RuntimeErrorWithCode, - ) + from dashscope.common.error import DashScopeException with patch.object( AgenticRL, @@ -908,7 +897,7 @@ async def test_run_submit_failure_raises(self): client = AgenticRL() client.tuning = MagicMock() - with pytest.raises(RuntimeErrorWithCode): + with pytest.raises(DashScopeException): await client.run(model="qwen3.5-9b") @pytest.mark.asyncio diff --git a/tests/unit/test_agentic_rl_utils.py b/tests/unit/test_agentic_rl_utils.py index defdbf3..f0baa66 100644 --- a/tests/unit/test_agentic_rl_utils.py +++ b/tests/unit/test_agentic_rl_utils.py @@ -300,7 +300,10 @@ async def test_client_error_raises_with_root_cause(self): retry_times=1, ) - assert exc_info.value.error_code == 4002 + assert ( + exc_info.value.error_code + == "sdk.agentic_rl.RuntimeErrorWithCode" + ) root = exc_info.value.__cause__ assert isinstance(root, aiohttp.ClientConnectionError) assert "Cannot write to transport" in str(root) @@ -313,7 +316,7 @@ async def test_timeout_raises_with_root_cause(self): ".ClientSession", return_value=_mock_session(asyncio.TimeoutError()), ): - with pytest.raises(RuntimeErrorWithCode) as exc_info: + with pytest.raises(TimeoutError) as exc_info: await async_http_request( method="POST", url="https://example.com/api", @@ -322,7 +325,6 @@ async def test_timeout_raises_with_root_cause(self): retry_times=1, ) - assert exc_info.value.error_code == 4003 assert isinstance( exc_info.value.__cause__, asyncio.TimeoutError, diff --git a/tests/unit/test_agentstudio_protocol.py b/tests/unit/test_agentstudio_protocol.py index 70bdafa..330e49e 100644 --- a/tests/unit/test_agentstudio_protocol.py +++ b/tests/unit/test_agentstudio_protocol.py @@ -4,8 +4,7 @@ These three contracts were agreed on with the backend team: 1. Error envelope uses ``error.code`` / ``error.message`` (the - documented shape). Legacy ``error_code`` / ``error_message`` is - still tolerated for compatibility. + documented shape). 2. Wire format is snake_case throughout — both request bodies emitted by the SDK and response bodies returned by the server. The only defensive translation is ``requestId`` → ``request_id`` because @@ -19,7 +18,11 @@ import pytest from dashscope.agentstudio import exceptions -from dashscope.agentstudio.transport import is_error_payload, unwrap +from dashscope.agentstudio.transport import ( + build_headers, + is_error_payload, + unwrap, +) from dashscope.agentstudio.types import ( user_custom_tool_result, user_define_outcome, @@ -40,29 +43,73 @@ def test_error_uses_nested_code_and_message(): "request_id": "req_001", } err = exceptions.from_response(status_code=400, body=body) - assert isinstance(err, exceptions.InvalidRequestError) + assert isinstance(err, exceptions.APIStatusError) assert err.code == "invalid_request_error" assert err.message == "bad arg" assert err.request_id == "req_001" -def test_error_legacy_underscored_fields_still_parsed(): - body = { - "type": "error", - "error": {"error_code": "rate_limit_error", "error_message": "slow"}, - } - err = exceptions.from_response(status_code=429, body=body) - assert isinstance(err, exceptions.RateLimitError) - assert err.code == "rate_limit_error" +def test_permission_error_code_classifies(): + """The unified ``permission_error`` code classifies correctly.""" + body = {"type": "error", "error": {"code": "permission_error"}} + err = exceptions.from_response(status_code=403, body=body) + assert isinstance(err, exceptions.APIStatusError) + assert err.code == "permission_error" + + +def test_missing_code_falls_back_to_api_error(): + """A HTTP response without a recognizable code resolves to the generic + ``api_error`` — we no longer guess a public code from the status number.""" + err = exceptions.from_response(status_code=404, body=None) + assert isinstance(err, exceptions.APIStatusError) + assert err.code == "api_error" + assert err.message == "HTTP 404" + + +def test_missing_code_never_guesses_from_status(): + """Regardless of the status number, an omitted server code yields the same + generic ``api_error`` with a bare HTTP status message.""" + for status in (400, 401, 403, 404, 429, 500, 503, 504): + err = exceptions.from_response(status_code=status, body=None) + assert err.code == "api_error" + assert err.message == f"HTTP {status}" + + +def test_flat_top_level_code_is_classified(): + """The classic flat DashScope envelope carries ``code``/``message`` at the + top level (no ``error`` wrapper); a recognized code still classifies.""" + body = {"code": "not_found_error", "message": "gone", "request_id": "r_1"} + err = exceptions.from_response(status_code=404, body=body) + assert isinstance(err, exceptions.APIStatusError) + assert err.code == "not_found_error" + assert err.message == "gone" + assert err.request_id == "r_1" + + +def test_flat_top_level_unknown_code_is_preserved(): + """A flat code (e.g. ``InvalidParameter``) is preserved as-is from the + server response, along with the server message.""" + body = {"code": "InvalidParameter", "message": "Model not exist."} + err = exceptions.from_response(status_code=400, body=body) + assert isinstance(err, exceptions.APIStatusError) + assert err.code == "InvalidParameter" + assert err.message == "Model not exist." -def test_is_error_payload_detects_both_shapes(): +def test_is_error_payload_detects_error_shapes(): + # Explicit type flag. assert is_error_payload( {"type": "error", "error": {"code": "x", "message": "y"}}, ) + # An error object with code/message, no explicit type flag. + assert is_error_payload({"error": {"code": "x", "message": "y"}}) + # Pre-release shape. Must be detected too: the transport only raises when + # this returns True, so missing it turns an error into a fake success. assert is_error_payload( - {"type": "error", "error": {"error_code": "x", "error_message": "y"}}, + {"error": {"error_code": "x", "error_message": "y"}}, ) + # A normal resource payload is not an error. + assert not is_error_payload({"id": "agt_1", "request_id": "r"}) # --------------------------------------------------------------------------- @@ -281,7 +328,9 @@ def test_session_stats_and_usage_fields(): def test_from_response_spring_default(): - """Spring Boot default error page is coerced to the right error type.""" + """A Spring Boot default error page carries no machine code, so it falls + back to the generic ``api_error`` (its ``error`` text becomes the message). + """ body = { "timestamp": "...", "status": 404, @@ -289,7 +338,103 @@ def test_from_response_spring_default(): "path": "/api/v1/agentstudio/agents", } err = exceptions.from_response(status_code=404, body=body) + assert isinstance(err, exceptions.APIStatusError) assert isinstance(err, exceptions.NotFoundError) + assert err.code == "api_error" + assert err.message == "Not Found" + + +def test_sdk_originated_codes_use_registry_namespace(): + """Errors raised without a server response carry the registry's + ``sdk.agentstudio.*`` code, not a public API code.""" + assert exceptions.APIConnectionError.code == ( + "sdk.agentstudio.APIConnectionError" + ) + assert exceptions.APITimeoutError.code == "sdk.agentstudio.APITimeoutError" + assert exceptions.StreamError.code == "sdk.agentstudio.StreamError" + assert exceptions.StreamClosedError.code == ( + "sdk.agentstudio.StreamClosedError" + ) + # Timeout is a specialization of connection failure. + assert issubclass( + exceptions.APITimeoutError, + exceptions.APIConnectionError, + ) + + +def test_prerelease_error_code_shape_is_accepted(): + """The pre-release backend sends error_code/error_message instead of + code/message; both must resolve identically.""" + body = {"type": "error", "error": {"error_code": "rate_limit_error"}} + err = exceptions.from_response(status_code=429, body=body) + assert isinstance(err, exceptions.RateLimitError) + assert err.code == "rate_limit_error" + + +def test_camel_case_request_id_is_accepted(): + """unwrap() already translates requestId on the success path, so the + error path must accept the same spelling.""" + body = { + "error": {"code": "api_error", "message": "boom"}, + "requestId": "req_camel", + } + err = exceptions.from_response(status_code=500, body=body) + assert err.request_id == "req_camel" + # snake_case still wins when both are present. + both = { + "error": {"code": "api_error"}, + "request_id": "req_snake", + "requestId": "req_camel", + } + assert ( + exceptions.from_response( + status_code=500, + body=both, + ).request_id + == "req_snake" + ) + + +def test_recognized_code_outranks_status(): + """When the server's code and its status disagree, the code decides the + class -- the status is only a fallback for unrecognized codes.""" + body = {"error": {"code": "rate_limit_error", "message": "slow down"}} + err = exceptions.from_response(status_code=400, body=body) + assert isinstance(err, exceptions.RateLimitError) + assert not isinstance(err, exceptions.InvalidRequestError) + assert err.status_code == 400 + + +def test_request_timeout_code_classifies_as_internal_server_error(): + """The registry's ``request_timeout`` key spelling classifies the same as + ``timeout_error`` -- InternalServerError -- regardless of the paired + status.""" + for status in (408, 504): + body = {"type": "error", "error": {"code": "request_timeout"}} + err = exceptions.from_response(status_code=status, body=body) + assert isinstance(err, exceptions.InternalServerError), status + assert err.code == "request_timeout" + + +def test_bare_408_classifies_as_internal_server_error(): + """408 is retried and then surfaced; without a server code it must not + degrade to the bare APIStatusError base class.""" + err = exceptions.from_response(status_code=408, body=None) + assert isinstance(err, exceptions.InternalServerError) + assert err.code == "api_error" + + +def test_missing_api_key_raises_authentication_error(): + """A missing api_key is a local configuration error. It must stay + catchable as ``except AuthenticationError`` and report 401, matching + the registry's AUTH_FAILED entry.""" + with pytest.raises(exceptions.AuthenticationError) as exc_info: + build_headers(api_key=None, uid=None, user_agent="ua") + assert exc_info.value.code == "authentication_error" + assert exc_info.value.status_code == 401 + + with pytest.raises(exceptions.AuthenticationError): + build_headers(api_key="", uid=None, user_agent="ua") # --------------------------------------------------------------------------- @@ -557,6 +702,40 @@ def test_permission_policy_validation(): # pylint: enable=too-many-function-args +def test_exception_classes_are_reexported_from_package_root(): + """`from dashscope.agentstudio import NotFoundError` must keep working. + + The classification-by-code refactor dropped these re-exports, which turned + every documented ``except NotFoundError`` into an import-time failure. The + other tests here import from the ``exceptions`` submodule, so only this one + catches a repeat. + """ + from dashscope import agentstudio + + names = [ + "AgentStudioError", + "APIStatusError", + "APIConnectionError", + "APITimeoutError", + "StreamError", + "StreamClosedError", + "InvalidRequestError", + "AuthenticationError", + "PermissionDeniedError", + "NotFoundError", + "ConflictError", + "RateLimitError", + "OverloadedError", + "InternalServerError", + ] + for name in names: + assert name in agentstudio.__all__, name + assert getattr(agentstudio, name) is getattr( + agentstudio.exceptions, + name, + ), name + + def test_approval_errors_map_by_http_status(): """Approval error codes are not enumerated as separate exception types; they map by HTTP status, with the reason in .code/.message.""" diff --git a/tests/unit/test_async_custom_session.py b/tests/unit/test_async_custom_session.py index 032e805..8ad1690 100644 --- a/tests/unit/test_async_custom_session.py +++ b/tests/unit/test_async_custom_session.py @@ -84,7 +84,10 @@ class TestAsyncSessionUsage: async def test_custom_aio_session_is_used_for_request(self): """测试自定义 aio_session 被实际用于请求""" # 创建 mock session - mock_session = AsyncMock() + # spec is required: HttpRequest detects an aiohttp session by + # class name / isinstance, so a bare AsyncMock is misfiled as a + # sync requests.Session and aio_call() hits the network for real. + mock_session = AsyncMock(spec=aiohttp.ClientSession) mock_response = AsyncMock() mock_response.status = 200 mock_response.headers = {"content-type": "application/json"} @@ -429,7 +432,7 @@ class TestAsyncSessionWithDifferentMethods: @pytest.mark.asyncio async def test_custom_aio_session_with_post_request(self): """测试 POST 请求使用自定义 session""" - mock_session = AsyncMock() + mock_session = AsyncMock(spec=aiohttp.ClientSession) mock_response = AsyncMock() mock_response.status = 200 mock_response.__aenter__ = AsyncMock(return_value=mock_response) @@ -476,7 +479,7 @@ async def mock_handle_response(_response): @pytest.mark.asyncio async def test_custom_aio_session_with_get_request(self): """测试 GET 请求使用自定义 session""" - mock_session = AsyncMock() + mock_session = AsyncMock(spec=aiohttp.ClientSession) mock_response = AsyncMock() mock_response.status = 200 mock_response.__aenter__ = AsyncMock(return_value=mock_response) @@ -596,7 +599,7 @@ class TestAsyncSessionLifecycle: @pytest.mark.asyncio async def test_multiple_requests_with_same_custom_session(self): """测试使用同一个自定义 session 进行多次请求""" - mock_session = AsyncMock() + mock_session = AsyncMock(spec=aiohttp.ClientSession) mock_response = AsyncMock() mock_response.status = 200 mock_response.__aenter__ = AsyncMock(return_value=mock_response) diff --git a/tests/unit/test_cli_common.py b/tests/unit/test_cli_common.py index f28f737..ffcb7d1 100644 --- a/tests/unit/test_cli_common.py +++ b/tests/unit/test_cli_common.py @@ -1,11 +1,361 @@ # -*- coding: utf-8 -*- -"""Unit tests for CLI exit-code classification in dashscope.cli.common.""" +"""Tests for dashscope/cli/common.py error handling and exit-code tiers.""" +from http import HTTPStatus from types import SimpleNamespace +from unittest.mock import Mock +import pytest +import typer from typer.testing import CliRunner -from dashscope.cli.common import _exit_code_for from dashscope.cli import embeddings +from dashscope.cli.common import ( + _exit_code_for, + _handle_exception, + err_console, + print_failed_message, + ensure_ok, + handle_sdk_error, + set_verbose_errors, +) +from dashscope.api_entities.dashscope_response import DashScopeAPIResponse +from dashscope.common.error import AuthenticationError +from dashscope.finetune.reinforcement.common.errors import OutputError + + +class TestPrintFailedMessage: + """Test print_failed_message with various response scenarios.""" + + def test_complete_response(self, capsys): + """Test with all fields present.""" + rsp = DashScopeAPIResponse( + status_code=500, + request_id="req_123", + code="ServerError", + message="Internal server error", + ) + print_failed_message(rsp) + captured = capsys.readouterr() + assert "req_123" in captured.err + assert "500" in captured.err + assert "ServerError" in captured.err + assert "Internal server error" in captured.err + + def test_missing_request_id(self, capsys): + """Test when request_id is missing.""" + rsp = Mock() + rsp.status_code = 400 + rsp.code = "BadRequest" + rsp.message = "Invalid parameter" + # Simulate missing request_id attribute + del rsp.request_id + + print_failed_message(rsp) + captured = capsys.readouterr() + # Missing request_id should not be displayed (empty fields are omitted) + assert "request_id:" not in captured.err + # Error code is kept in original camelCase format + assert "BadRequest" in captured.err + assert "400" in captured.err + + def test_empty_code_and_message(self, capsys): + """Test when code and message are empty strings.""" + rsp = DashScopeAPIResponse( + status_code=503, + request_id="req_456", + code="", + message="", + ) + print_failed_message(rsp) + captured = capsys.readouterr() + # Should not show empty code/message fields + # Use word boundary check: "code: " with space after colon + assert ", code: " not in captured.err + assert ", message: " not in captured.err + assert "req_456" in captured.err + + def test_none_attributes(self, capsys): + """Test when attributes are None.""" + rsp = Mock() + rsp.status_code = 502 + rsp.request_id = None + rsp.code = None + rsp.message = None + + print_failed_message(rsp) + captured = capsys.readouterr() + # None values should not be displayed + assert ", request_id:" not in captured.err + assert ", code: " not in captured.err + assert ", message: " not in captured.err + assert "502" in captured.err + + +class TestEnsureOk: + """Test ensure_ok with various response scenarios.""" + + def test_successful_response(self): + """Test with successful HTTP 200 and no business error.""" + rsp = DashScopeAPIResponse( + status_code=HTTPStatus.OK, + request_id="req_ok", + code="", + message="", + output={"result": "success"}, + ) + result = ensure_ok(rsp) + assert result == {"result": "success"} + + def test_http_error(self, capsys): + """Test with HTTP error status.""" + rsp = DashScopeAPIResponse( + status_code=404, + request_id="req_404", + code="NotFound", + message="Resource not found", + ) + + with pytest.raises(typer.Exit): + ensure_ok(rsp) + + captured = capsys.readouterr() + # Should only print once (not duplicated) + # Error message now uses "Request failed" instead of "Failed" + assert captured.err.count("Request failed") == 1 + + def test_business_error_in_dict_output(self, capsys): + """Test with HTTP 200 but business error in dict output.""" + rsp = DashScopeAPIResponse( + status_code=HTTPStatus.OK, + request_id="req_biz_err", + code="", + message="", + output={"code": "InvalidParameter", "message": "Model not found"}, + ) + + with pytest.raises(typer.Exit): + ensure_ok(rsp) + + captured = capsys.readouterr() + # Rich may wrap long lines and add extra spaces, normalize whitespace + normalized_err = " ".join(captured.err.split()) + # Error code is kept in original camelCase format + assert "InvalidParameter" in normalized_err + assert "Model not found" in normalized_err + + def test_business_error_without_message(self, capsys): + """Test business error without message field.""" + rsp = DashScopeAPIResponse( + status_code=HTTPStatus.OK, + request_id="req_no_msg", + output={"code": "SomeError"}, # No message field + ) + + with pytest.raises(typer.Exit): + ensure_ok(rsp) + + captured = capsys.readouterr() + # Should show improved fallback message + # (normalize all whitespace for Rich formatting) + normalized_err = " ".join(captured.err.split()) + assert "API returned error code without message" in normalized_err + + def test_none_output(self, capsys): + """Test when output is None despite HTTP 200.""" + rsp = DashScopeAPIResponse( + status_code=HTTPStatus.OK, + request_id="req_null", + output=None, + ) + + with pytest.raises(typer.Exit): + ensure_ok(rsp) + + captured = capsys.readouterr() + assert "no output data" in captured.err + + def test_skip_business_error_check(self): + """Test with check_business_error=False.""" + rsp = DashScopeAPIResponse( + status_code=HTTPStatus.OK, + output={ + "code": "AsyncTaskPending", + "message": "Task is processing", + }, + ) + + # Should not raise even though there's a code in output + result = ensure_ok(rsp, check_business_error=False) + assert result == { + "code": "AsyncTaskPending", + "message": "Task is processing", + } + + def test_object_output_with_error(self, capsys): + """Test with object output containing error fields.""" + mock_output = Mock() + mock_output.code = "ObjectError" + mock_output.message = "Object-level error" + + rsp = DashScopeAPIResponse( + status_code=HTTPStatus.OK, + request_id="req_obj", + output=mock_output, + ) + + with pytest.raises(typer.Exit): + ensure_ok(rsp) + + captured = capsys.readouterr() + assert "ObjectError" in captured.err + + +class TestHandleSdkError: + """Test handle_sdk_error decorator.""" + + def test_dashscope_exception_handling(self, capsys): + """Test handling of DashScopeException.""" + + @handle_sdk_error("Test action") + def failing_function(): + # Create exception properly using __init__ with positional args + exc = AuthenticationError() + exc.request_id = "req_auth" + exc.code = "AuthFailed" + exc.message = "Invalid API key" + raise exc + + with pytest.raises(typer.Exit): + failing_function() + + captured = capsys.readouterr() + assert "Test action" in captured.err + assert "req_auth" in captured.err + # Error code is kept in original camelCase format (exception type name) + assert "AuthenticationError" in captured.err + + def test_generic_exception_handling(self, capsys): + """Test handling of generic exceptions.""" + + @handle_sdk_error("Generic test") + def generic_failing_function(): + raise ValueError("Something went wrong") + + with pytest.raises(typer.Exit): + generic_failing_function() + + captured = capsys.readouterr() + assert "Generic test" in captured.err + assert "Something went wrong" in captured.err + + def test_typer_exit_passthrough(self): + """Test that typer.Exit is re-raised without modification.""" + + @handle_sdk_error("Should not catch this") + def intentional_exit(): + raise typer.Exit(code=2) + + with pytest.raises(typer.Exit) as exc_info: + intentional_exit() + + assert exc_info.value.exit_code == 2 + + def test_successful_function_passthrough(self): + """Test that successful functions work normally.""" + + @handle_sdk_error("Success test") + def success_function(): + return "success" + + result = success_function() + assert result == "success" + + +class TestHandleException: + """Test _handle_exception error code, request id, and verbose traceback.""" + + @pytest.fixture(autouse=True) + def _reset_verbose(self): + yield + set_verbose_errors(False) + + @staticmethod + def _fail(exc, action="Call failed"): + """Report *exc* the way a real handler does: raised, then caught.""" + try: + raise exc + except Exception as raised: + _handle_exception(raised, action, err_console) + + def test_registry_error_code_is_reported(self, capsys): + """RL exceptions subclass plain Exception but carry error_code.""" + exc = OutputError("job rejected", response={"request_id": "req-rl"}) + + with pytest.raises(typer.Exit) as exc_info: + self._fail(exc, "Run failed") + + captured = capsys.readouterr() + assert exc_info.value.exit_code == 1 + assert "sdk.agentic_rl.OutputError" in captured.err + assert "req-rl" in captured.err + assert "job rejected" in captured.err + + def test_request_id_read_from_response_dict(self, capsys): + """request_id is attached to .response, not set as an attribute.""" + exc = OutputError("bad output") + assert getattr(exc, "request_id", None) is None + exc.response = {"request_id": "req-from-resp"} + + with pytest.raises(typer.Exit): + self._fail(exc, "Run failed") + + assert "req-from-resp" in capsys.readouterr().err + + def test_core_exception_uses_class_name(self, capsys): + """Core DashScopeException subclasses carry no error_code.""" + exc = AuthenticationError() + exc.request_id = "req-core" + + with pytest.raises(typer.Exit): + self._fail(exc) + + captured = capsys.readouterr() + assert "AuthenticationError" in captured.err + assert "req-core" in captured.err + + def test_plain_exception_has_no_code_block(self, capsys): + with pytest.raises(typer.Exit): + self._fail(ValueError("boom")) + + captured = capsys.readouterr() + assert "boom" in captured.err + assert "code:" not in captured.err + + def test_verbose_prints_traceback(self, capsys): + set_verbose_errors(True) + + with pytest.raises(typer.Exit): + self._fail(ValueError("boom")) + + assert "Traceback" in capsys.readouterr().err + + def test_without_verbose_no_traceback(self, capsys): + with pytest.raises(typer.Exit): + self._fail(ValueError("boom")) + + assert "Traceback" not in capsys.readouterr().err + + def test_decorator_shares_structured_reporting(self, capsys): + @handle_sdk_error("Submit") + def failing(): + raise OutputError("nope", response={"request_id": "req-deco"}) + + with pytest.raises(typer.Exit): + failing() + + captured = capsys.readouterr() + assert "sdk.agentic_rl.OutputError" in captured.err + assert "req-deco" in captured.err # --------------------------------------------------------------------------- diff --git a/tests/unit/test_cli_main.py b/tests/unit/test_cli_main.py index 22f9c34..df65981 100644 --- a/tests/unit/test_cli_main.py +++ b/tests/unit/test_cli_main.py @@ -65,7 +65,7 @@ def mock_list(**kwargs): combined_output = captured_output.out + captured_output.err assert exception_info.value.code == 1 - assert "No api key provided." in combined_output + assert "No api key provided" in combined_output assert "Traceback" not in combined_output def test_top_level_help_shows_global_api_key(self): diff --git a/tests/unit/test_error_detail_preserved.py b/tests/unit/test_error_detail_preserved.py new file mode 100644 index 0000000..d47e15d --- /dev/null +++ b/tests/unit/test_error_detail_preserved.py @@ -0,0 +1,239 @@ +# -*- coding: utf-8 -*- +"""Error paths must keep the detail that explains the failure. + +Four places reduced a specific exception to a generic message: the agentstudio +transports wrapped httpx timeouts, whose ``str()`` is usually empty; +``iter_over_async`` logged the real exception but reported only the registry +text; and the WebSocket connect/handshake handlers dropped the host, port and +server reason. The 503 detection also matched a bare "503" anywhere in the +message, which fires on addresses such as ``host:5030``. + +The agentstudio SSE iterators had the opposite gap: a mid-stream transport +failure reached the caller as a bare httpx exception (often with an empty +message) instead of a ``StreamError`` naming the cause. +""" +from types import SimpleNamespace +from unittest import mock +from unittest.mock import AsyncMock, MagicMock + +import httpx +import pytest + +from dashscope.agentstudio import exceptions +from dashscope.agentstudio.exceptions import describe_exception +from dashscope.agentstudio.streaming import AsyncEventStream, EventStream +from dashscope.agentstudio.transport import ( + AsyncTransport, + SyncTransport, +) +from dashscope.api_entities.websocket_request import ( + _internal_error_message, + _is_service_unavailable, +) +from dashscope.common.error_registry import INTERNAL_ERROR +from dashscope.common.utils import iter_over_async + + +class TestDescribeException: + def test_messageless_exception_falls_back_to_type_name(self): + assert describe_exception(httpx.ReadTimeout("")) == "ReadTimeout" + + def test_whitespace_only_message_is_treated_as_empty(self): + assert describe_exception(httpx.ReadTimeout(" ")) == "ReadTimeout" + + def test_message_is_prefixed_with_type_name(self): + assert describe_exception(ValueError("bad")) == "ValueError: bad" + + +def _transport_kwargs(client): + return { + "base_url": "https://example.com", + "api_key": "key", + "workspace": None, + "uid": None, + "user_agent": "ua", + "timeout": 1.0, + "max_retries": 0, + "http_client": client, + } + + +class TestSyncTransportKeepsDetail: + def _transport(self, side_effect): + client = MagicMock() + client.send.side_effect = side_effect + return SyncTransport(**_transport_kwargs(client)) + + def test_timeout_message_is_not_empty(self): + transport = self._transport(httpx.ReadTimeout("")) + + with pytest.raises(exceptions.APITimeoutError) as exc_info: + transport.request("GET", "/v1/agents") + + assert exc_info.value.message.strip() + assert "ReadTimeout" in exc_info.value.message + + def test_timeout_keeps_its_cause(self): + original = httpx.ReadTimeout("") + transport = self._transport(original) + + with pytest.raises(exceptions.APITimeoutError) as exc_info: + transport.request("GET", "/v1/agents") + + assert exc_info.value.__cause__ is original + + def test_connection_error_message_is_not_empty(self): + transport = self._transport(httpx.ConnectError("")) + + with pytest.raises(exceptions.APIConnectionError) as exc_info: + transport.request("GET", "/v1/agents") + + assert "ConnectError" in exc_info.value.message + + +class TestAsyncTransportKeepsDetail: + def _transport(self, side_effect): + client = AsyncMock() + client.build_request = MagicMock() + client.send.side_effect = side_effect + return AsyncTransport(**_transport_kwargs(client)) + + @pytest.mark.asyncio + async def test_timeout_message_is_not_empty(self): + transport = self._transport(httpx.ReadTimeout("")) + + with pytest.raises(exceptions.APITimeoutError) as exc_info: + await transport.request("GET", "/v1/agents") + + assert "ReadTimeout" in exc_info.value.message + + @pytest.mark.asyncio + async def test_connection_error_message_is_not_empty(self): + transport = self._transport(httpx.ConnectError("")) + + with pytest.raises(exceptions.APIConnectionError) as exc_info: + await transport.request("GET", "/v1/agents") + + assert "ConnectError" in exc_info.value.message + + +class TestStreamErrorWrapsTransportFailures: + """A mid-stream transport failure must surface as StreamError carrying + the cause, not as a bare httpx exception with an empty message.""" + + def test_sync_mid_stream_failure_becomes_stream_error(self): + original = httpx.ReadError("") + stream = EventStream(response=MagicMock(spec=httpx.Response)) + + def produce(): + yield SimpleNamespace(data='{"type":"ok"}') + raise original + + with mock.patch.object(stream, "_event_source") as es: + es.iter_sse.side_effect = produce + it = iter(stream) + assert next(it) == {"type": "ok"} + with pytest.raises(exceptions.StreamError) as exc_info: + next(it) + + assert exc_info.value.code == "sdk.agentstudio.StreamError" + assert "ReadError" in exc_info.value.message + assert exc_info.value.__cause__ is original + + def test_sync_agentstudio_error_passes_through_unwrapped(self): + original = exceptions.StreamClosedError("stream already closed") + stream = EventStream(response=MagicMock(spec=httpx.Response)) + + def produce(): + raise original + yield # noqa: W0101, pylint: disable=unreachable + + with mock.patch.object(stream, "_event_source") as es: + es.iter_sse.side_effect = produce + with pytest.raises(exceptions.StreamClosedError) as exc_info: + next(iter(stream)) + + assert exc_info.value is original + + @pytest.mark.asyncio + async def test_async_mid_stream_failure_becomes_stream_error(self): + original = httpx.ReadError("") + stream = AsyncEventStream(response=AsyncMock(spec=httpx.Response)) + + async def produce(): + yield SimpleNamespace(data='{"type":"ok"}') + raise original + + with mock.patch.object(stream, "_event_source") as es: + es.aiter_sse.side_effect = produce + ait = stream.__aiter__() + assert await ait.__anext__() == {"type": "ok"} + with pytest.raises(exceptions.StreamError) as exc_info: + await ait.__anext__() + + assert exc_info.value.code == "sdk.agentstudio.StreamError" + assert "ReadError" in exc_info.value.message + assert exc_info.value.__cause__ is original + + +class TestIterOverAsyncReportsTheFailure: + def test_message_names_the_underlying_exception(self): + async def failing(): + yield 1 + raise RuntimeError("generator exploded") + + yielded = list(iter_over_async(failing())) + + assert yielded[0] == 1 + response = yielded[-1] + assert response.status_code == INTERNAL_ERROR.status_code + assert response.code == INTERNAL_ERROR.error_code + assert "RuntimeError" in response.message + assert "generator exploded" in response.message + + def test_registry_text_is_still_present(self): + async def failing(): + yield 1 + raise ValueError("bad") + + response = list(iter_over_async(failing()))[-1] + + assert response.message.startswith(INTERNAL_ERROR.format_msg()) + + +class TestInternalErrorMessage: + def test_includes_registry_text_and_exception_detail(self): + message = _internal_error_message(OSError(22, "Invalid argument")) + + assert message.startswith(INTERNAL_ERROR.error_msg) + assert "OSError" in message + assert "Invalid argument" in message + + def test_explicit_detail_wins_over_str(self): + message = _internal_error_message( + OSError("ignored"), + "handshake rejected: model not found", + ) + + assert "handshake rejected: model not found" in message + assert "ignored" not in message + + def test_empty_detail_falls_back_to_the_exception(self): + message = _internal_error_message(ValueError("real reason"), "") + + assert "real reason" in message + + +class TestServiceUnavailableDetection: + def test_explicit_phrase_is_treated_as_503(self): + assert _is_service_unavailable(OSError("Service Unavailable")) + + def test_port_containing_503_is_not_treated_as_503(self): + assert not _is_service_unavailable( + OSError("Cannot connect to host 10.50.30.1:5030"), + ) + + def test_plain_connection_refusal_is_not_treated_as_503(self): + assert not _is_service_unavailable( + OSError("Cannot connect to host dashscope.aliyuncs.com:443"), + ) diff --git a/tests/unit/test_error_registry_p0_regressions.py b/tests/unit/test_error_registry_p0_regressions.py new file mode 100644 index 0000000..8263e5e --- /dev/null +++ b/tests/unit/test_error_registry_p0_regressions.py @@ -0,0 +1,258 @@ +# -*- coding: utf-8 -*- +# pylint: disable=protected-access +"""Regression tests for the two P0 defects fixed on dev/errors. + +P0-1: ``AioHttpRequest._handle_response`` yielded the result of the + ``async def`` ``_handle_aiohttp_failed_response`` without awaiting + it, so every ``Aio*`` API handed callers a bare coroutine instead + of a ``DashScopeAPIResponse`` on any non-2xx status. + +P0-2: six FAILED-response sites in ``reinforcement/common/model.py`` read + ``SdkErrorDef.external``, a field deleted together with + ``ClientErrorDef``. Every real function-component deployment + failure therefore raised ``AttributeError`` instead of returning a + structured ``ResponseFC``. +""" +import asyncio +import warnings +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from dashscope.api_entities.aiohttp_request import AioHttpRequest +from dashscope.api_entities.dashscope_response import DashScopeAPIResponse +from dashscope.common.error_registry import ( + INTERNAL_ERROR, + SDK_AGENTIC_RL_BASE_CONNECTION_ERROR, + SDK_AGENTIC_RL_FUNCTION_LAYER_ERROR, + SDK_AGENTIC_RL_FUNCTION_LOAD_ERROR, + SDK_AGENTIC_RL_INSTANCE_QUERY_ERROR, + SDK_AGENTIC_RL_INSTANCE_WARMUP_ERROR, + SDK_AGENTIC_RL_REGISTRATION_ERROR, +) +from dashscope.finetune.reinforcement.common.errors import FunctionLayerError +from dashscope.finetune.reinforcement.common.model import ( + AgenticRLFunctionComponent, +) +from dashscope.finetune.reinforcement.common.model_types import ( + FunctionType, + StatusType, +) + + +class _MockErrorResponse: + """Minimal stand-in for an aiohttp.ClientResponse carrying an error.""" + + def __init__(self, status, content_type, payload=None, text=""): + self.status = status + self.content_type = content_type + self.headers = {"Content-Type": content_type} + self._payload = payload + self._text = text + + async def json(self): + return self._payload + + async def text(self): + return self._text + + +def _make_request(stream=False): + return AioHttpRequest( + url="https://dashscope.aliyuncs.com/api/v1/x", + api_key="sk-test", + http_method="POST", + stream=stream, + ) + + +async def _collect(request, response): + return [rsp async for rsp in request._handle_response(response)] + + +class TestAioHttpErrorResponseIsAwaited: + """P0-1: non-2xx responses must yield a real response object.""" + + def test_json_error_yields_response_not_coroutine(self): + response = _MockErrorResponse( + status=400, + content_type="application/json", + payload={ + "code": "InvalidParameter", + "message": "Model not exist.", + "request_id": "req-1", + }, + ) + results = asyncio.run(_collect(_make_request(), response)) + + assert len(results) == 1 + rsp = results[0] + assert isinstance(rsp, DashScopeAPIResponse) + assert not asyncio.iscoroutine(rsp) + assert rsp.status_code == 400 + assert rsp.code == "InvalidParameter" + assert rsp.message == "Model not exist." + assert rsp.request_id == "req-1" + + def test_json_error_without_code_falls_back(self): + response = _MockErrorResponse( + status=500, + content_type="application/json", + payload={"request_id": "req-2"}, + ) + rsp = asyncio.run(_collect(_make_request(), response))[0] + + assert isinstance(rsp, DashScopeAPIResponse) + assert rsp.code == INTERNAL_ERROR.error_code + assert rsp.message == INTERNAL_ERROR.format_msg() + + def test_non_json_error_yields_response(self): + response = _MockErrorResponse( + status=502, + content_type="text/html", + text="Bad Gateway", + ) + rsp = asyncio.run(_collect(_make_request(), response))[0] + + assert isinstance(rsp, DashScopeAPIResponse) + assert rsp.status_code == 502 + assert rsp.code == INTERNAL_ERROR.error_code + assert rsp.message == "Bad Gateway" + + def test_no_unawaited_coroutine_warning(self): + response = _MockErrorResponse( + status=429, + content_type="application/json", + payload={"code": "Throttling", "message": "slow down"}, + ) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + asyncio.run(_collect(_make_request(), response)) + + assert not [ + w for w in caught if "never awaited" in str(w.message) + ], "error response coroutine was not awaited" + + def test_registry_has_no_external_field(self): + """Guard the assumption behind the P0-2 fix.""" + for err_def in ( + SDK_AGENTIC_RL_REGISTRATION_ERROR, + SDK_AGENTIC_RL_FUNCTION_LAYER_ERROR, + SDK_AGENTIC_RL_BASE_CONNECTION_ERROR, + SDK_AGENTIC_RL_FUNCTION_LOAD_ERROR, + SDK_AGENTIC_RL_INSTANCE_WARMUP_ERROR, + SDK_AGENTIC_RL_INSTANCE_QUERY_ERROR, + ): + assert not hasattr(err_def, "external") + + +def _make_component(**kwargs): + return AgenticRLFunctionComponent( + type=FunctionType.ROLLOUT, + name="test_fn", + **kwargs, + ) + + +def _assert_failed(rsp, expected_name): + """A FAILED ResponseFC carrying the real diagnostic, not AttributeError.""" + assert rsp.status.task == StatusType.FAILED + assert rsp.status.name == expected_name + assert rsp.status.code == INTERNAL_ERROR.status_code + assert "{" not in rsp.status.message, "unfilled placeholder in message" + assert rsp.status.message + + +class TestFunctionComponentFailurePaths: + """P0-2: every deployment-failure path returns a structured response.""" + + def test_query_with_empty_instance_id(self): + rsp = asyncio.run(AgenticRLFunctionComponent.query("")) + + _assert_failed(rsp, SDK_AGENTIC_RL_INSTANCE_QUERY_ERROR.name) + assert "No instance ID available" in rsp.status.message + assert rsp.output == {"instance_id": ""} + + def test_load_without_entity_id(self): + rsp = asyncio.run(_make_component().load()) + + _assert_failed(rsp, SDK_AGENTIC_RL_FUNCTION_LOAD_ERROR.name) + assert "No valid registration ID provided" in rsp.status.message + + def test_register_generic_failure(self): + component = _make_component() + component.fcmodel = MagicMock() + component.fcmodel.generate_id.side_effect = RuntimeError("oss down") + + rsp = asyncio.run(component.register()) + + _assert_failed(rsp, SDK_AGENTIC_RL_REGISTRATION_ERROR.name) + assert "oss down" in rsp.status.message + + def test_register_function_layer_failure(self): + component = _make_component() + component.fcmodel = MagicMock() + component.fcmodel.generate_id.side_effect = FunctionLayerError( + "layer build failed", + ) + + rsp = asyncio.run(component.register()) + + _assert_failed(rsp, SDK_AGENTIC_RL_FUNCTION_LAYER_ERROR.name) + assert "layer build failed" in rsp.status.message + + def test_register_fc_call_failure(self): + component = _make_component() + fcmodel = MagicMock() + fcmodel.classpath = "" + fcmodel.get_oss = AsyncMock() + fcmodel.to_oss = AsyncMock() + fcmodel.oss_id = "oss-id-1" + fcmodel.filepath = "/pkg/rollout.py" + fcmodel.classname = "RolloutFn" + fcmodel.oss_signed_url = "https://oss/x.zip" + component.fcmodel = fcmodel + + with patch( + "dashscope.finetune.reinforcement.common.model.client_fc", + new=AsyncMock(side_effect=ConnectionError("fc unreachable")), + ), patch( + "dashscope.finetune.reinforcement.common.model.FC_LAYER_USED", + False, + ): + rsp = asyncio.run(component.register()) + + _assert_failed(rsp, SDK_AGENTIC_RL_BASE_CONNECTION_ERROR.name) + assert "fc unreachable" in rsp.status.message + + def test_load_warmup_failure(self): + component = _make_component() + component.entity_id = "entity-1" + component.instance_id = "inst-1" + component.instance_url = "ftp://not-http" + component.instance_token = "tok" + + with patch( + "dashscope.finetune.reinforcement.common.model.client_fc", + new=AsyncMock( + return_value={ + "output": { + "instanceId": "inst-1", + "trigger_url": "ftp://not-http", + "trigger_token": "tok", + }, + }, + ), + ), patch( + "dashscope.finetune.reinforcement.common.model.FC_LAYER_USED", + False, + ): + rsp = asyncio.run(component.load(warmup=True)) + + _assert_failed(rsp, SDK_AGENTIC_RL_INSTANCE_WARMUP_ERROR.name) + assert "Invalid instance URL format" in rsp.status.message + assert rsp.output == {"instance_id": "inst-1"} + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/tests/unit/test_error_registry_placeholders.py b/tests/unit/test_error_registry_placeholders.py new file mode 100644 index 0000000..dc11b95 --- /dev/null +++ b/tests/unit/test_error_registry_placeholders.py @@ -0,0 +1,97 @@ +# -*- coding: utf-8 -*- +"""Registry placeholder integrity and P1-7 regressions. + +``error_registry.py`` is generated by an external tool and must not be edited, +so the guarantees consumers depend on are asserted here instead. The central +risk is a call site rendering a template without supplying its ``vars``, which +leaks a literal ``{inner_code}`` into a user-visible message. +""" +import re + +import pytest + +from dashscope.common.error_registry import ( + INTERNAL_ERRORS, + PUBLIC_ERRORS, +) +from dashscope.finetune.reinforcement.common.errors import ( + AgenticRLError, + RuntimeErrorWithCode, + ValueErrorWithCode, +) + +_PLACEHOLDER = re.compile(r"\{(\w+)\}") + +_EXCEPTION_CLASSES = [AgenticRLError, RuntimeErrorWithCode, ValueErrorWithCode] + + +class TestRegistryIntegrity: + def test_internal_error_names_are_unique(self): + names = [d.name for d in INTERNAL_ERRORS] + assert len(names) == len(set(names)) + + def test_public_error_keys_are_unique(self): + keys = [d.key for d in PUBLIC_ERRORS] + assert len(keys) == len(set(keys)) + + @pytest.mark.parametrize("err_def", INTERNAL_ERRORS, ids=lambda d: d.name) + def test_declared_vars_match_message_placeholders(self, err_def): + assert set(err_def.vars) == set(_PLACEHOLDER.findall(err_def.message)) + + @pytest.mark.parametrize("err_def", INTERNAL_ERRORS, ids=lambda d: d.name) + def test_unfilled_template_keeps_its_placeholder(self, err_def): + """Locks the assumption the consumer-side guard relies on: rendering a + templated definition with no arguments leaves ``{var}`` in place.""" + rendered = err_def.format_message() + for var in err_def.vars: + assert "{" + var + "}" in rendered + if not err_def.vars: + assert "{" not in rendered + + @pytest.mark.parametrize("err_def", INTERNAL_ERRORS, ids=lambda d: d.name) + def test_fully_filled_template_has_no_placeholder(self, err_def): + filled = {var: "VALUE" for var in err_def.vars} + assert "{" not in err_def.format_message(filled) + + @pytest.mark.parametrize("err_def", INTERNAL_ERRORS, ids=lambda d: d.name) + def test_solution_is_not_empty(self, err_def): + assert err_def.solution.strip() + + def test_templated_public_errors_render_clean(self): + substitutions = { + "missing_parameter": {"parameter": "model"}, + "resource_not_found": {"resource": "qwen-plus"}, + "request_too_large": {"limit": "10MB"}, + } + for err_def in PUBLIC_ERRORS: + expected = substitutions.get(err_def.key) + if expected is None: + continue + assert "{" not in err_def.format_msg(expected) + + +class TestExceptionStrNeverLeaksPlaceholders: + @pytest.mark.parametrize("exc_cls", _EXCEPTION_CLASSES) + @pytest.mark.parametrize("err_def", INTERNAL_ERRORS, ids=lambda d: d.name) + def test_str_has_no_unfilled_placeholder(self, exc_cls, err_def): + exc = exc_cls("job rejected", error_code=err_def.name) + assert "{" not in str(exc) + + def test_templated_code_falls_back_to_own_message(self): + exc = AgenticRLError( + "job rejected", + error_code="sdk.agentic_rl.WorkflowFailed", + ) + assert str(exc) == "job rejected" + + def test_vars_free_code_still_uses_registry_message(self): + exc = AgenticRLError( + "job rejected", + error_code="sdk.agentic_rl.OutputError", + ) + assert str(exc).startswith("Service response failed output") + assert "job rejected" in str(exc) + + def test_unknown_code_falls_back_to_own_message(self): + exc = AgenticRLError("job rejected", error_code="sdk.not_a_real_code") + assert str(exc) == "job rejected" diff --git a/tests/unit/test_error_status_classification.py b/tests/unit/test_error_status_classification.py new file mode 100644 index 0000000..c89343d --- /dev/null +++ b/tests/unit/test_error_status_classification.py @@ -0,0 +1,146 @@ +# -*- coding: utf-8 -*- +# pylint: disable=protected-access +"""Failures must be reported with the status they actually mean. + +Three classification defects: a 403 WebSocket handshake was reported as a 401 +AuthenticationError, telling the caller to rotate a valid API key; the +``run()`` workflow log was unreachable because every inner failure arrives +already wrapped as a DashScopeException and was re-raised before logging; and +an async call silently ignored a caller-supplied session it could not reuse. +""" +from http import HTTPStatus +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from dashscope.api_entities import http_request as http_request_module +from dashscope.api_entities.http_request import HttpRequest +from dashscope.api_entities.websocket_request import _handshake_error +from dashscope.common.constants import HTTPMethod +from dashscope.common.error import DashScopeException +from dashscope.common.error_registry import ( + AUTH_FAILED, + INTERNAL_ERROR, + PERMISSION_DENIED, + SERVICE_UNAVAILABLE, +) +from dashscope.finetune.agentic_rl import AgenticRL + + +class TestHandshakeStatusClassification: + @pytest.mark.parametrize( + "status, expected", + [ + (HTTPStatus.UNAUTHORIZED, AUTH_FAILED), + (HTTPStatus.FORBIDDEN, PERMISSION_DENIED), + (HTTPStatus.SERVICE_UNAVAILABLE, SERVICE_UNAVAILABLE), + (HTTPStatus.INTERNAL_SERVER_ERROR, INTERNAL_ERROR), + ], + ) + def test_classified_statuses(self, status, expected): + assert _handshake_error(status) is expected + + def test_forbidden_is_not_reported_as_an_auth_failure(self): + """A 403 must not carry a 401 status or the AuthenticationError + code, or the caller is told to replace credentials that work.""" + denied = _handshake_error(HTTPStatus.FORBIDDEN) + + assert denied.status_code == 403 + assert denied.error_code == "PermissionDeniedError" + + @pytest.mark.parametrize("status", [HTTPStatus.BAD_REQUEST, 418]) + def test_unclassified_status_returns_none(self, status): + """None means the caller re-raises instead of inventing a response.""" + assert _handshake_error(status) is None + + +class TestRunLogsWorkflowFailure: + @pytest.mark.asyncio + @patch( + "dashscope.finetune.agentic_rl.AgenticRLTuning.__init__", + return_value=None, + ) + @patch("dashscope.finetune.agentic_rl.logger") + async def test_passthrough_failure_is_still_logged( + self, + mock_logger, + _mock_parent_init, + ): + """Inner calls always hand run() a DashScopeException, so logging + after the passthrough check meant never logging at all.""" + original = DashScopeException("Inner error") + original.status_code = 400 + original.error_code = "BadRequestError" + + agent = AgenticRL.__new__(AgenticRL) + object.__setattr__(agent, "tuning", MagicMock()) + object.__setattr__( + agent, + "register_functions", + AsyncMock(side_effect=original), + ) + + with pytest.raises(DashScopeException) as exc_info: + await agent.run() + + assert exc_info.value is original + assert ( + mock_logger.error.call_args[0][1] + == "sdk.agentic_rl.WorkflowFailed" + ) + rendered = mock_logger.error.call_args[0][2] + assert "BadRequestError" in rendered + assert "{" not in rendered + + +class TestAsyncCallReportsIgnoredSession: + @pytest.mark.asyncio + async def test_warns_when_the_supplied_session_cannot_be_reused(self): + """A session that is not an aiohttp.ClientSession is stored for sync + calls only; an async call must say it is falling back rather than + leaving a custom connector silently unapplied.""" + request = HttpRequest( + url="https://example.com/api", + api_key="fake-api-key", + http_method=HTTPMethod.POST, + stream=False, + session=object(), + ) + + with patch.object( + http_request_module, + "get_shared_aio_session", + new=AsyncMock(side_effect=RuntimeError("stop here")), + ): + with patch.object(http_request_module, "logger") as mock_logger: + with pytest.raises(RuntimeError): + async for _ in request._handle_aio_request(): + pass + + assert any( + "ignoring the supplied" in call.args[0] + and "object" in call.args[1:] + for call in mock_logger.warning.call_args_list + if call.args + ) + + @pytest.mark.asyncio + async def test_no_warning_without_a_supplied_session(self): + request = HttpRequest( + url="https://example.com/api", + api_key="fake-api-key", + http_method=HTTPMethod.POST, + stream=False, + ) + + with patch.object( + http_request_module, + "get_shared_aio_session", + new=AsyncMock(side_effect=RuntimeError("stop here")), + ): + with patch.object(http_request_module, "logger") as mock_logger: + with pytest.raises(RuntimeError): + async for _ in request._handle_aio_request(): + pass + + assert not mock_logger.warning.call_args_list diff --git a/tests/unit/test_recognition_explicit_params.py b/tests/unit/test_recognition_explicit_params.py index 5edcd96..7d28836 100644 --- a/tests/unit/test_recognition_explicit_params.py +++ b/tests/unit/test_recognition_explicit_params.py @@ -3,6 +3,10 @@ """Test explicit parameters for Recognition class.""" # pylint: disable=protected-access +import threading +from contextlib import contextmanager +from unittest.mock import patch + from dashscope.audio.asr.recognition import Recognition, RecognitionCallback @@ -22,6 +26,31 @@ def on_close(self): pass +@contextmanager +def _stubbed_worker(recognition): + """Let ``start()`` run without its worker opening a real WebSocket. + + The worker has to stay alive until released: ``start()`` checks + ``is_alive()`` immediately after spawning it and raises ``InvalidTask`` + if the thread has already finished. + """ + release = threading.Event() + try: + with patch.object( + Recognition, + "_Recognition__receive_worker", + lambda self: release.wait(10), + ): + yield recognition + finally: + recognition._running = False + if recognition._silence_timer is not None: + recognition._silence_timer.cancel() + release.set() + if recognition._worker is not None: + recognition._worker.join(timeout=5) + + class TestRecognitionExplicitParams: """Test explicit parameters in Recognition class.""" @@ -106,30 +135,16 @@ def test_start_with_explicit_params(self): sample_rate=16000, ) - # Mock the thread and timer to avoid actual execution recognition._running = False recognition._callback = MockRecognitionCallback() - # Call start with explicit parameters - # Note: This will fail at thread creation, but we can verify params - try: + with _stubbed_worker(recognition): recognition.start( phrase_id="test_phrase", disfluency_removal_enabled=True, diarization_enabled=True, speaker_count=2, ) - except Exception: - # Expected to fail at thread creation - pass - finally: - # Clean up to stop any background threads - recognition._running = False - if ( - recognition._worker is not None - and recognition._worker.is_alive() - ): - recognition._worker.join(timeout=1) # Verify parameters are updated in _kwargs assert recognition._kwargs.get("disfluency_removal_enabled") is True @@ -151,26 +166,14 @@ def test_param_override_in_start(self): assert recognition._kwargs["disfluency_removal_enabled"] is False assert recognition._kwargs["speaker_count"] == 1 - # Mock the thread and timer to avoid actual execution recognition._running = False # Override parameters in start - try: + with _stubbed_worker(recognition): recognition.start( disfluency_removal_enabled=True, speaker_count=3, ) - except Exception: - # Expected to fail at thread creation - pass - finally: - # Clean up to stop any background threads - recognition._running = False - if ( - recognition._worker is not None - and recognition._worker.is_alive() - ): - recognition._worker.join(timeout=1) # Verify parameters are overridden assert recognition._kwargs.get("disfluency_removal_enabled") is True diff --git a/tests/unit/test_sse_parse_error_consistency.py b/tests/unit/test_sse_parse_error_consistency.py new file mode 100644 index 0000000..edd7fa8 --- /dev/null +++ b/tests/unit/test_sse_parse_error_consistency.py @@ -0,0 +1,111 @@ +# -*- coding: utf-8 -*- +# pylint: disable=protected-access +"""All four SSE parse-failure paths must report the same error. + +A body the SDK cannot parse is a server-side defect, not a bad request, so +every path reports 500 InternalServerError and carries the offending payload +in ``.message``. These paths had drifted apart: two reported 400 +BadRequestError (one of them claiming a parameter named "response data" was +missing) and one logged the payload but discarded it from the response. +""" +from http import HTTPStatus +from types import SimpleNamespace + +import pytest + +from dashscope.api_entities.aiohttp_request import AioHttpRequest +from dashscope.api_entities.http_request import HttpRequest +from dashscope.common.error_registry import INTERNAL_ERROR +from dashscope.common.utils import _handle_http_stream_response + +MALFORMED = '{"output": truncated' + +# The stream helpers default their status_code to 400 before any status: line +# arrives, so a parse branch that echoed it would report 400 by accident. +SSE_LINES = [b"event:result", b"data:" + MALFORMED.encode()] + + +class _FakeSyncResponse: + status_code = HTTPStatus.OK + headers = {"content-type": "text/event-stream"} + + def iter_lines(self): + return iter(SSE_LINES) + + +class _FakeAsyncContent: + def __aiter__(self): + async def _gen(): + for line in SSE_LINES: + yield line + + return _gen() + + +class _FakeAioResponse: + status = HTTPStatus.OK + content_type = "text/event-stream" + headers = {"Content-Type": "text/event-stream"} + + def __init__(self): + self.content = _FakeAsyncContent() + + +def _assert_unified(rsp): + assert ( + rsp.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + ), "an unparsable server body must not be reported as a client error" + assert rsp.code == INTERNAL_ERROR.error_code + assert MALFORMED in rsp.message, "the raw payload must reach the caller" + assert "response data" not in rsp.message + + +def test_sync_stream_parse_failure(): + fake_self = SimpleNamespace( + stream=True, + encryption=None, + flattened_output=False, + ) + rsps = list(HttpRequest._handle_response(fake_self, _FakeSyncResponse())) + assert len(rsps) == 1 + _assert_unified(rsps[0]) + + +def test_utils_stream_parse_failure(): + """The baseline the other three paths are aligned to.""" + _, rsp = next( + _handle_http_stream_response(_FakeSyncResponse()), + ) + _assert_unified(rsp) + + +@pytest.mark.asyncio +async def test_aio_stream_parse_failure_in_http_request(): + fake_self = SimpleNamespace(stream=True, encryption=None) + rsps = [ + rsp + async for rsp in HttpRequest._handle_aio_response( + fake_self, + _FakeAioResponse(), + ) + ] + assert len(rsps) == 1 + _assert_unified(rsps[0]) + + +@pytest.mark.asyncio +async def test_aio_stream_parse_failure_in_aiohttp_request(): + fake_self = SimpleNamespace( + stream=True, + # _handle_stream is unbound here and never touches self. + _handle_stream=lambda rsp: AioHttpRequest._handle_stream(None, rsp), + ) + rsps = [ + rsp + async for rsp in AioHttpRequest._handle_response( + fake_self, + _FakeAioResponse(), + ) + ] + assert len(rsps) == 1 + _assert_unified(rsps[0]) diff --git a/tests/unit/test_utf8_encoding.py b/tests/unit/test_utf8_encoding.py index 32dcaf2..b390add 100644 --- a/tests/unit/test_utf8_encoding.py +++ b/tests/unit/test_utf8_encoding.py @@ -59,7 +59,7 @@ def _make_mock_sync_session(): class _MockAioResponse: status = 200 content_type = "application/json" - headers = {"Content-Type": "application/json"} + headers = {"Content-Type": "application/json; charset=utf-8"} async def json(self): return {"output": {"text": "ok"}, "request_id": "test-123"} @@ -91,6 +91,18 @@ async def request( self.captured["headers"] = headers return _MockAioResponse() + # pylint: disable=unused-argument + async def post( + self, + url, + data=None, + headers=None, + timeout=None, + ): + self.captured["data"] = data + self.captured["headers"] = headers + return _MockAioResponse() + async def close(self): pass @@ -126,15 +138,6 @@ def test_stream_overrides_accept(self): ) assert "text/event-stream" in req.headers["Accept"] - def test_get_has_no_content_type(self): - req = HttpRequest( - url="https://example.com/api/v1/test", - api_key="test-key", - http_method=HTTPMethod.GET, - stream=False, - ) - assert "Content-Type" not in req.headers - class TestAioHttpRequestUtf8Headers: def test_content_type_has_charset(self): diff --git a/tests/unit/test_verify_function_error_classification.py b/tests/unit/test_verify_function_error_classification.py new file mode 100644 index 0000000..f841cf2 --- /dev/null +++ b/tests/unit/test_verify_function_error_classification.py @@ -0,0 +1,271 @@ +# -*- coding: utf-8 -*- +"""verify_function must not flatten service failures into a 400. + +Both of its handlers wrapped every exception in ``ValidationError``, so a failed +instance query, a missing trigger URL or a request timeout all reached +``AgenticRL.test_functions`` as "invalid input" and were reported to the user as +a 400. The reason the deployed function itself reported was discarded the same +way, replaced by a generic "Function output validation failed". +""" +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from dashscope.common.error import DashScopeException, InvalidParameter +from dashscope.finetune.agentic_rl import AgenticRL +from dashscope.finetune.reinforcement import FunctionType +from dashscope.finetune.reinforcement.common.errors import ( + InstanceQueryError, + OutputError, + ValidationError, +) +from dashscope.finetune.reinforcement.common.model import ( + AgenticRLFunctionComponent, + RolloutInput, +) +from dashscope.finetune.reinforcement.common.model_types import StatusType + +_MODEL = "dashscope.finetune.reinforcement.common.model" + + +def _query_result(task, output=None): + result = MagicMock() + result.status.task = task + result.output = output if output is not None else {} + return result + + +def _instance_output(url="https://fc.example.com", token="tok"): + return {"output": {"trigger_url": url, "trigger_token": token}} + + +class TestVerificationFailuresKeepTheirType: + """The handler around instance lookup and the test request.""" + + @pytest.mark.asyncio + async def test_failed_instance_query_is_not_a_400(self): + with patch.object( + AgenticRLFunctionComponent, + "query", + new=AsyncMock( + return_value=_query_result(StatusType.FAILED), + ), + ): + with pytest.raises(InstanceQueryError): + await AgenticRLFunctionComponent.verify_function( + MagicMock(spec=RolloutInput), + "inst-1", + ) + + @pytest.mark.asyncio + async def test_missing_trigger_url_is_not_a_400(self): + with patch.object( + AgenticRLFunctionComponent, + "query", + new=AsyncMock( + return_value=_query_result( + StatusType.SUCCEEDED, + _instance_output(url=""), + ), + ), + ): + with pytest.raises(OutputError): + await AgenticRLFunctionComponent.verify_function( + MagicMock(spec=RolloutInput), + "inst-1", + ) + + @pytest.mark.asyncio + async def test_request_timeout_stays_a_timeout(self): + """A timeout flattened into ValidationError could never reach the 504 + branch in test_functions.""" + with patch.object( + AgenticRLFunctionComponent, + "query", + new=AsyncMock( + return_value=_query_result( + StatusType.SUCCEEDED, + _instance_output(), + ), + ), + ): + with patch( + f"{_MODEL}.client_fc", + new=AsyncMock( + side_effect=TimeoutError("Request timeout (600s)"), + ), + ): + with pytest.raises(TimeoutError) as exc_info: + await AgenticRLFunctionComponent.verify_function( + MagicMock(spec=RolloutInput), + "inst-1", + ) + + assert "600s" in str(exc_info.value) + + @pytest.mark.asyncio + async def test_unexpected_failure_is_still_wrapped(self): + """The wrap is only removed for failures that classify themselves.""" + with patch.object( + AgenticRLFunctionComponent, + "query", + new=AsyncMock(side_effect=RuntimeError("bug")), + ): + with pytest.raises(ValidationError) as exc_info: + await AgenticRLFunctionComponent.verify_function( + MagicMock(spec=RolloutInput), + "inst-1", + ) + + assert "Function verification failed" in exc_info.value.message + + +class TestFunctionReportedReasonSurvives: + """The handler around response validation.""" + + @pytest.mark.asyncio + async def test_deployed_function_reason_is_kept(self): + response = { + "status": {"code": 500, "message": "model not deployed"}, + } + with patch.object( + AgenticRLFunctionComponent, + "query", + new=AsyncMock( + return_value=_query_result( + StatusType.SUCCEEDED, + _instance_output(), + ), + ), + ): + with patch( + f"{_MODEL}.client_fc", + new=AsyncMock(return_value=response), + ): + with pytest.raises(ValidationError) as exc_info: + await AgenticRLFunctionComponent.verify_function( + MagicMock(spec=RolloutInput), + "inst-1", + ) + + assert "model not deployed" in exc_info.value.message + + +class TestTestFunctionsReportsServiceFailuresAs5xx: + """End to end: what the user actually sees.""" + + @pytest.mark.asyncio + @patch("dashscope.finetune.agentic_rl.set_api_key") + @pytest.mark.parametrize( + "underlying, expected_status, expected_code, expected_type", + [ + ( + InstanceQueryError("Status query failed"), + 500, + "InternalServerError", + DashScopeException, + ), + ( + OutputError("No instance url/token provided"), + 500, + "InternalServerError", + DashScopeException, + ), + ( + TimeoutError("Request timeout (600s)"), + 504, + "GatewayTimeoutError", + DashScopeException, + ), + ( + ValidationError("model not deployed"), + 400, + "BadRequestError", + InvalidParameter, + ), + ], + ) + async def test_classification( + self, + _mock_set_api_key, + underlying, + expected_status, + expected_code, + expected_type, + ): + with patch( + "dashscope.finetune.agentic_rl.RolloutInput.model_validate", + return_value=MagicMock(), + ): + with patch( + "dashscope.finetune.agentic_rl." + "AgenticRLFunctionComponent.verify_function", + new=AsyncMock(side_effect=underlying), + ): + with pytest.raises(expected_type) as exc_info: + await AgenticRL.test_functions( + instance_id="inst-1", + functype=FunctionType.ROLLOUT, + input_data={}, + ) + + assert exc_info.value.status_code == expected_status + assert exc_info.value.error_code == expected_code + assert exc_info.value.__cause__ is underlying + + @pytest.mark.asyncio + @patch("dashscope.finetune.agentic_rl.set_api_key") + @patch("dashscope.finetune.agentic_rl.logger") + async def test_timeout_logs_the_timeout_code( + self, + mock_logger, + _mock_set_api_key, + ): + """The FunctionTestTimeout registry entry was unreachable before.""" + with patch( + "dashscope.finetune.agentic_rl.RolloutInput.model_validate", + return_value=MagicMock(), + ): + with patch( + "dashscope.finetune.agentic_rl." + "AgenticRLFunctionComponent.verify_function", + new=AsyncMock( + side_effect=TimeoutError("Request timeout (600s)"), + ), + ): + with pytest.raises(DashScopeException): + await AgenticRL.test_functions( + instance_id="inst-1", + functype=FunctionType.ROLLOUT, + input_data={}, + ) + + assert ( + mock_logger.error.call_args[0][1] + == "sdk.agentic_rl.FunctionTestTimeout" + ) + + @pytest.mark.asyncio + @patch("dashscope.finetune.agentic_rl.set_api_key") + async def test_service_failure_message_names_the_cause( + self, + _mock_set_api_key, + ): + underlying = InstanceQueryError("Status query failed") + with patch( + "dashscope.finetune.agentic_rl.RolloutInput.model_validate", + return_value=MagicMock(), + ): + with patch( + "dashscope.finetune.agentic_rl." + "AgenticRLFunctionComponent.verify_function", + new=AsyncMock(side_effect=underlying), + ): + with pytest.raises(DashScopeException) as exc_info: + await AgenticRL.test_functions( + instance_id="inst-1", + functype=FunctionType.ROLLOUT, + input_data={}, + ) + + assert "InstanceQueryError" in str(exc_info.value)