Skip to content

fix(api): map CircuitBreakerOpen to UNAVAILABLE with retry_after - #6141

Open
akushonkamen wants to merge 1 commit into
volcengine:mainfrom
akushonkamen:fix/6139-circuit-breaker-unavailable
Open

akushonkamen wants to merge 1 commit into
volcengine:mainfrom
akushonkamen:fix/6139-circuit-breaker-unavailable

Conversation

@akushonkamen

Copy link
Copy Markdown
Contributor

Description

本 PR 修复 embedding 熔断器打开(CircuitBreakerOpen)经 search / find 路由外泄时被映射成不透明 500 INTERNAL 的问题:客户端现在会收到可重试的 503 UNAVAILABLE,并携带 retry_after 以便退避。

Human Involvement

  • A human participated in the implementation or review loop
  • This PR was generated entirely by AI agents without human participation in the loop

AI 披露:本修复由 AI agent 定位根因、实现并验证;认领评论(含与 #6045 的划界)同样由 AI 生成。

Related Issue

Fixes #6139

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

根因:map_exception(openviking/server/error_mapping.py:477 起)没有 CircuitBreakerOpen 的 isinstance 分支,且熔断消息(如 "Circuit breaker is OPEN, retry after 13s")不含 upstream marker / HTTP 状态码 / 兜底子串可命中,最终走到函数末尾的 fallback return 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),镜像既有 AGFS UnavailableError 分支风格:构造 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、litellm APIConnectionError → UNAVAILABLE(后两者钉住 #6045 已在 main 修好的路径,防回归)。

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

本地验证(克隆自 origin/main = 84cc2b4,与工单 triage 基线一致;uv sync --frozen --extra test 后全部实跑):

  1. 红态探针(改前):uv run --no-sync python -c "map_exception(CircuitBreakerOpen('Circuit breaker is OPEN, retry after 13s'))" → None(复现 500 根因)。
  2. 基线探针:文本 'Error code: 429' → RESOURCE_EXHAUSTED;litellm APIConnectionError(status_code=500) → UNAVAILABLE,均已在 main 绿(与 fix(api): 修正输入错误与模型失败的 HTTP 状态码 #6045 triage 一致)。
  3. 测试级红:stash 修复后 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 例绿)。
  4. 修复后同命令 → 4 passed。
  5. 全文件:timeout 300 uv run --no-sync python -m pytest tests/server/test_error_mapping.py -q → 40 passed, 8 warnings(= 36 既有 + 4 新增)。
  6. lint: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

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Additional Notes

  • Commit 带 DCO:Signed-off-by: akushonkamen <akushonkamen@163.com>,Conventional Commits 格式 fix(api):。
  • 查重:issue timeline 中 akushonkamen 0 个关联事件、gh search prs 6139 全状态为空、issue comments 无重复实现。

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>
@akushonkamen

Copy link
Copy Markdown
Contributor Author

Hi — a quick note on the failing CI check, since everything else is green:

API & CLI Integration Tests (ubuntu-24.04) failed for environmental reasons unrelated to this PR. All 5 failed tests (filesystem/test_fs_cp.py, test_fs_read_write.py, …) fail with the same external-service error:

PROCESSING_ERROR: Failed to generate embedding: Volcengine embedding failed:
Error code: 401 - AuthenticationError: The API key format is incorrect

i.e. the CI integration environment's Volcengine credentials are currently rejected — this is not touched by this PR:

  • Same failure on unrelated branches: the identical 5 tests fail with the identical 401 signature on other PR branches today (e.g. run 38754191585 on fix/6129-embedding-token-estimate), and this workflow shows failures across 5+ unrelated PR branches on 2026-10-15 — while the main run at 13:07 UTC succeeded, suggesting PR-branch runs are hitting an invalid/missing API key.
  • Out of scope of the diff: this PR only changes openviking/server/error_mapping.py (maps CircuitBreakerOpen → UNAVAILABLE with retry_after) and its unit tests in tests/server/test_error_mapping.py — no filesystem or embedding code paths.
  • Local evidence (all green): python -m pytest tests/server/test_error_mapping.py → 40 passed (includes the 4 new tests added by this PR); the other PR-triggered checks on this PR (check-deps, plugin-tests, plugin-versions) all passed.

Happy to re-run once the CI credentials are fixed, and of course glad to address any review feedback on the change itself.

@ZaynJarvis ZaynJarvis added the kernel Core server, runtime, storage, retrieval, SDK, and CLI. label Oct 16, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kernel Core server, runtime, storage, retrieval, SDK, and CLI.

Projects

Status: Backlog

2 participants