Repository navigation
fix(api): map CircuitBreakerOpen to UNAVAILABLE with retry_after - #6141
Open
akushonkamen wants to merge 1 commit into
Open
akushonkamen wants to merge 1 commit into
akushonkamen wants to merge 1 commit into
Conversation
An open embedding circuit breaker surfaced by the search/find routes fell through map_exception (no isinstance branch, no fallback marker match) and reached general_error_handler as None, so clients got an opaque 500 INTERNAL instead of a retryable UNAVAILABLE. volcengine#6045 already routed upstream 429s (RESOURCE_EXHAUSTED) and litellm connection failures (UNAVAILABLE) on main; this adds the missing third case: CircuitBreakerOpen now maps to UnavailableError("embedding provider") carrying the annotated retry_after so clients can back off. Fixes volcengine#6139 Signed-off-by: akushonkamen <akushonkamen@163.com>
Contributor
Author
|
Hi — a quick note on the failing CI check, since everything else is green:
i.e. the CI integration environment's Volcengine credentials are currently rejected — this is not touched by this PR:
Happy to re-run once the CI credentials are fixed, and of course glad to address any review feedback on the change itself. |
11 of 21 tasks
This branch has not been deployed
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
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
本 PR 修复 embedding 熔断器打开(
CircuitBreakerOpen)经 search / find 路由外泄时被映射成不透明 500 INTERNAL 的问题:客户端现在会收到可重试的 503 UNAVAILABLE,并携带retry_after以便退避。Human Involvement
Related Issue
Fixes #6139
Type of Change
Changes Made
根因:
map_exception(openviking/server/error_mapping.py:477起)没有CircuitBreakerOpen的isinstance分支,且熔断消息(如 "Circuit breaker is OPEN, retry after 13s")不含 upstream marker / HTTP 状态码 / 兜底子串可命中,最终走到函数末尾的 fallbackreturn None(error_mapping.py:617)。app.py的general_error_handler(openviking/server/app.py:565)对None落到默认 500 INTERNAL。CircuitBreakerOpen唯一携带 HTTP 语义的外溢路径是 embedding 资源_execute打上retry_after后 re-raise(openviking/config/embedding.py:243-245);另两处捕获点(openviking/storage/collection_schemas.py:755、openviking/storage/queuefs/semantic_processor.py:546)均为内部 requeue,不外泄到 API 层。修法:
map_exception在GitConcurrentCommitError分支之后新增isinstance(exc, CircuitBreakerOpen)分支(error_mapping.py:509-516),镜像既有 AGFSUnavailableError分支风格:构造UnavailableError("embedding provider", reason=str(exc))(code=UNAVAILABLE → HTTP 503),并把 embedding 路径标注的retry_after补进details,供客户端退避。测试:
tests/server/test_error_mapping.py新增 4 个回归测试——熔断带 / 不带retry_after、文本 429 → RESOURCE_EXHAUSTED、litellmAPIConnectionError→ UNAVAILABLE(后两者钉住 #6045 已在 main 修好的路径,防回归)。Testing
本地验证(克隆自
origin/main=84cc2b4,与工单 triage 基线一致;uv sync --frozen --extra test后全部实跑):uv run --no-sync python -c "map_exception(CircuitBreakerOpen('Circuit breaker is OPEN, retry after 13s'))"→None(复现 500 根因)。'Error code: 429'→RESOURCE_EXHAUSTED;litellmAPIConnectionError(status_code=500)→UNAVAILABLE,均已在 main 绿(与 fix(api): 修正输入错误与模型失败的 HTTP 状态码 #6045 triage 一致)。uv run --no-sync python -m pytest tests/server/test_error_mapping.py -q -k "circuit or litellm or text_429"→ 2 failed 2 passed(熔断 2 例红、429/litellm 2 例绿)。timeout 300 uv run --no-sync python -m pytest tests/server/test_error_mapping.py -q→ 40 passed, 8 warnings(= 36 既有 + 4 新增)。uv run --no-sync ruff check openviking/server/error_mapping.py tests/server/test_error_mapping.py→ All checks passed。ruff format --check报 1 处 reformat,经git show HEAD:tests/...对纯净文件复验为 HEAD 既有 drift(test_git_auth_failure_remains_a_client_error尾段),本 PR 新增行零 format diff,未顺带重排(保持最小 diff)。Checklist
Additional Notes
Signed-off-by: akushonkamen <akushonkamen@163.com>,Conventional Commits 格式fix(api):。gh search prs 6139全状态为空、issue comments 无重复实现。