fix - Concurrency limit failed not return json

[jianglan89]

No description provided.

[LLLLKKKK]

AI Code Review — PR #843 (v1)

fix - Concurrency limit failed not return json

规模: 2 files, +62/-5 | Review 类型: 全量

---

修复并发限制触发时 chat_completion 端点返回非 JSON 响应的问题。将 generate_request_id 包裹在 try/except 中，与 inference/embedding 端点保持一致。新增 3 个测试用例。

P2

1. try/except 范围可以更精确 — 建议只包裹 increment()。

---

LGTM ready to ci — P0=0, P1=0。

[LLLLKKKK]

🤖 AI Code Review — PR #843

PR 概述

Title: fix - Concurrency limit failed not return json
Author: jianglan89
规模: 2(GitHub) files, +62/-5

核心目标

修复 chat_completion 接口在并发限制触发时未返回 JSON 格式错误响应的问题。当 ConcurrencyController.increment() 抛出 ConcurrencyException 时，原代码未捕获异常，导致客户端收到非 JSON 的错误响应（可能是 500 HTML 页面），而非结构化的 JSON 错误信息。

---

改动逻辑拆解

GitHub 开源仓库变更（主要代码）

1. frontend_server.py — chat_completion 异常捕获

在 chat_completion 方法中，将 self._global_controller.increment() 和 generate_request_id() 调用包裹在 try/except 块中。异常时调用 self._handle_exception(request.model_dump(exclude_none=True), e) 返回结构化 JSON 错误响应。

这与 inference 和 embedding 方法中已有的异常处理模式完全一致：

• inference（第 222-237 行）：已有 try/except 包裹 increment()
• embedding（第 134-144 行）：已有 try/except 包裹 increment()
• chat_completion（本次修复）：之前缺少此保护

2. frontend_server_test.py — 新增 3 个测试用例

• test_inference_increment_failure_returns_json：验证 inference 路径
• test_embedding_increment_failure_returns_json：验证 embedding 路径
• test_chat_completion_increment_failure_returns_json：验证 chat_completion 路径

测试通过设置 max_concurrency=0 的 ConcurrencyController 使 increment() 始终抛出 ConcurrencyException，验证三个入口均返回 status_code=500 的 JSON 响应（包含 error_code 和 message 字段）。

---

Checklist 检查结果

通用原则

软件工程原则

检查项	结果
SRP	✅ PR 只修复一个问题
OCP	✅ 不涉及
LSP	✅ 不涉及
ISP	✅ 不涉及
DIP	✅ 不涉及
DRY	✅ 异常处理模式与 inference/embedding 一致，复用 _handle_exception
KISS	✅ 修复简洁直接
YAGNI	✅ 无多余设计

架构审视

检查项	结果
抽象边界	✅
依赖方向	✅
状态完整性	✅ 异常时未调用 increment() 成功，无需 decrement()
错误语义	✅ 返回 500 + JSON，与其他入口一致
可观测性	✅ _handle_exception 内部已有 ConcurrencyException 的 metric 上报
可演进性	✅
可运维性	✅

测试

检查项	结果
新功能有对应测试	✅ 3 个测试覆盖所有 3 个入口
删除的测试有等价替代	✅ 无删除
边界 case 覆盖	✅ max_concurrency=0 是最极端的边界
分布式改动有多卡测试	✅ 不涉及分布式

代码质量与文档

检查项	结果
无关改动分离	✅
mega-PR 拆分	✅ 小 PR
Commit 原子性	✅ 单 commit，自包含
Commit message 准确性	✅ 准确描述了问题
PR description 说明动机和设计	❌ PR body 为空，缺少问题描述和修复说明（P3）
日志频率控制	✅ 不涉及

领域检查

A. 兼容性与配置 — 全部 ✅

B. 正确性与逻辑 — 全部 ✅

C. 线程安全与并发 — 全部 ✅

D. 性能 — 全部 ✅

E. 分布式 — 全部 ✅

F. 跨平台 — 全部 ✅

G. 语言与框架特有 — 全部 ✅

H. 测试与 CI — 全部 ✅

I. 代码质量 — 全部 ✅

---

Review 意见

小问题

1. PR description 为空 [P3]
   PR body 为空，建议补充简要的问题描述（如"并发限制触发时 chat_completion 返回非 JSON 响应"）和修复方案说明，方便后续 git log 追溯。

整体评价

这是一个干净、精准的 bug fix。问题定位准确：chat_completion 是三个请求入口中唯一缺少 increment() 异常捕获的路径。修复方式与已有的 inference/embedding 模式完全一致，测试覆盖充分（三个入口全部验证）。CI 已通过（功能测试 SUCCESS）。

✅ LGTM ready to ci

[LLLLKKKK]

🤖 AI Code Review — PR #843

PR 概述

Title: fix - Concurrency limit failed not return json
Author: jianglan89
规模: 2 files, +62/-5

修复 chat_completion 端点在并发限制触发时未返回 JSON 格式错误响应的 bug。将 increment() + generate_request_id() 包裹在 try/except 中，与 inference/embedding 端点保持一致。新增 3 个测试覆盖所有端点。

---

Review 意见

P2 建议

1. increment() 成功后 generate_request_id() 异常时并发计数器泄漏

   increment() 和 generate_request_id() 在同一个 try 块内。若 increment() 成功但 generate_request_id() 抛异常，early return 跳过了 decrement()，计数器永久 +1。实际风险很低（generate_request_id 是纯计算），且 inference/embedding 也有相同模式（pre-existing）。后续可考虑 context manager 统一管理。

2. PR description 为空 — 建议补充问题描述和修复说明。

P3 Nits

• 三个测试结构几乎相同，可用 parametrize 或 subTest 减少重复
• ConcurrencyException import 未在测试中使用，可移除或用于更精确断言
• asyncio.new_event_loop() 未 close()（pre-existing pattern）

---

结论

LGTM ready to ci — 无 P0/P1 问题，修复正确且测试充分，可进入 CI 验证和合入流程。P2/P3 建议后续改进但不阻塞。

[LLLLKKKK]

AI Code Review — PR #843

Summary: P0/0 · P1/0 · P2/0 · P3/0

Review status: LGTM

lgtm ready to ci

Strengths

• Aligns chat_completion error handling with the existing pattern in inference() and embedding(), ensuring ConcurrencyException returns a structured JSON error instead of an unhandled 500
• Good test coverage: tests all three endpoints (inference, embedding, chat_completion) with a zero-capacity controller