Repository navigation
Dev/errors - #162
Draft
luk384090-cloud wants to merge 8 commits into
Draft
Dev/errors#162luk384090-cloud wants to merge 8 commits into
luk384090-cloud wants to merge 8 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request enhances error handling, response validation, and compatibility across the DashScope CLI and SDK, including mapping SDK errors to CLI-friendly codes and adding unit tests. The review feedback highlights several critical issues where the new validation logic strictly assumes dictionary outputs, which will cause runtime crashes or failures when API responses are objects. It is recommended to update these checks to dynamically support both dictionary and object formats.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
lzsweb
reviewed
Jul 15, 2026
lzsweb
reviewed
Jul 16, 2026
lzsweb
reviewed
Jul 16, 2026
lzsweb
reviewed
Jul 16, 2026
lzsweb
reviewed
Jul 21, 2026
lzsweb
reviewed
Aug 5, 2026
lzsweb
reviewed
Aug 6, 2026
lzsweb
reviewed
Aug 6, 2026
foleydang
force-pushed
the
dev/errors
branch
2 times, most recently
from
August 7, 2026 11:16
97af92c to
40b34b6
Compare
lzsweb
reviewed
Aug 19, 2026
|
|
||
| class PermissionDeniedError(APIStatusError): | ||
| code = "permission_denied_error" | ||
| code = "permission_error" |
Collaborator
There was a problem hiding this comment.
这个py里面还遗留比较多的，没有定义类的。
class APIStatusError(AgentStudioError):
"""Raised when the server returns a non-2xx status."""
code = "api_status_error"
class InvalidRequestError(APIStatusError):
code = "invalid_request_error"
class AuthenticationError(APIStatusError):
code = "authentication_error"
class PermissionDeniedError(APIStatusError):
code = "permission_error"
class NotFoundError(APIStatusError):
code = "not_found_error"
class ConflictError(APIStatusError):
code = "conflict_error"
class RateLimitError(APIStatusError):
code = "rate_limit_error"
class OverloadedError(APIStatusError):
code = "overloaded_error"
class InternalServerError(APIStatusError):
code = "api_error"
lzsweb
reviewed
Aug 20, 2026
Every failure path used to surface `code="Unknown"` with `status_code=-1`, leaving callers unable to tell a bad request from a server fault. This routes all of them through a single registry that separates the public code the caller sees (`BadRequestError`, `RateLimitError`, ...) from the internal one used for diagnosis (`sdk.agentic_rl.FunctionTestTimeout`, ...). Public/internal split: - Add `dashscope/common/error_registry.py` holding the public errors and the internal `sdk.*` definitions, each with a message, a solution and its substitution variables. The file is generator output; all fixes here land on the consumer side. - AgentStudio and AgenticRL classify by server code only, so a status can no longer be contradicted by a message substring. Classification fixes: - `verify_function` no longer flattens `AgenticRLError` subclasses and timeouts into `ValidationError`, which made a failed instance query or a request timeout look like a 400. It also stopped discarding the reason the deployed function itself reported. - Add `TimeoutErrorWithCode` deriving from the builtin `TimeoutError`, so a wrapped `asyncio.TimeoutError` still reaches the 504 branch on every supported Python version. Previously it became a 500 and the `sdk.agentic_rl.FunctionTestTimeout` code was unreachable. - `AgenticRL.init()` wrapped nothing, so a bad `config_path` leaked a raw `IOErrorWithCode` with no `status_code` at all. A missing or malformed config is the caller's to fix and now reports 400. - A WebSocket handshake 403 was reported as 401, sending callers to rotate an API key that was in fact valid. - `AgenticRL.run()` logged only after passing `DashScopeException` straight through, which left most workflow failures unrecorded. - The service-unavailable probe matched the bare substring "503", so an address such as `10.50.30.1:5030` was misread as an outage. Detail preservation: - Transport, WebSocket and `iter_over_async` failures keep the originating exception instead of replacing it with a generic 500 message. `httpx` timeouts usually carry no message at all, so those fall back to the type name. - Unparsable SSE bodies report 500 and retain the payload. - Passing a `requests.Session` to an async call silently ignored it; it now warns, since only an `aiohttp.ClientSession` can be reused there. The log-and-classify sequence duplicated across five `AgenticRL` methods is extracted into `_log_internal_error` / `_public_exception`, and the resulting record is attributed to the calling handler rather than to the helper. Tests: 1132 passed, 6 skipped.
The pre-commit workflow failed on `add-trailing-comma`, `black` and `pylint`, and the 3.9 unit-test job failed on one new test. pylint: - `_handle_response` was at 52/50 statements. The SSE parse-failure branch it gained was a near-copy of the one in `call()`, so both now share `_sse_parse_failure_response`, which brings it back under the limit instead of widening it. - Four test modules drive private request handlers on purpose; they now carry the module-level `protected-access` disable the other 18 test modules use. - `test_error_detail_preserved` had a bare `yield` after `raise` purely to make the closure an async generator, which reads as unreachable code. It now yields first and raises after, matching the neighbouring test. - `test_agentstudio_protocol` used `import dashscope.agentstudio as ...`. Python 3.9: - `test_record_is_attributed_to_the_caller` read `caplog.records`, which was empty because the SDK logger does not propagate and caplog attaches at the root. It now attaches a collecting handler to the module's own logger, so the stacklevel attribution is verified on every supported version. Tests: 1174 passed, 6 skipped. All pre-commit hooks pass.
lzsweb
reviewed
Sep 14, 2026
|
|
||
| class InternalServerError(APIStatusError): | ||
| code = "api_error" | ||
| code = INTERNAL_ERROR.anthropic_error_code |
| self, | ||
| message: str, | ||
| error_code: int = 1420, | ||
| error_code: str = "sdk.agentic_rl.FunctionLoadError", |
lzsweb
marked this pull request as draft
September 17, 2026 07:42
added 2 commits
September 17, 2026 16:07
Resolve conflicts in dashscope/finetune/agentic_rl.py: - imports: union of both sides (add 'time' from main, keep 'Tuple' from dev/errors) - test_functions except block: keep the public-error mapping from dev/errors - keep function-instance log query/delete helpers added on main
… merge Merging main brought in the teacher_model/PG_OPD branch into submit_job, pushing it to 13 branches (pylint limit 12). Add an inline 'pylint: disable=too-many-branches' on submit_job, matching the existing convention used across the SDK (transport.py, generation.py, etc.).
2 of 3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
[Describe what this PR does and why]
Related Issue: Fixes #[issue_number] or Relates to #[issue_number]
Security Considerations: [Check if API keys or sensitive credentials are exposed in code/logs]
Type of Change
Component(s) Affected
Checklist
Testing
[How to test these changes]
Additional Notes
[Optional: any other context]