feat(error-rules): 添加错误响应覆盖功能#225
Conversation
Add ability to override error responses and status codes when error rules match: - New DB fields: override_response (JSONB) and override_status_code - Runtime override logic in error-handler with validation (400-599 range) - New validator for Claude API error format with 10KB size limit - Frontend: override section component with real-time JSON validation - Frontend: error rule tester to preview override behavior - Support empty message fallback to original error - Backward compatible: no behavior change without override config 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Summary of ChangesHello @sususu98, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求引入了一项重要功能，允许系统管理员根据预定义的错误规则自定义返回给客户端的错误响应和 HTTP 状态码。这使得系统能够将复杂的上游错误转换为更具描述性或标准化的客户端消息，同时仍然在后端记录原始错误以供调试。该功能包括对覆盖配置的严格验证、对 Claude API 错误格式的支持，以及一个直观的前端测试工具，以简化配置和验证过程。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📊 PR Size AnalysisThis PR is Large (L) with ~3,674 total lines changed across 26 files. Breakdown
📝 AssessmentThis PR implements a single cohesive feature (error response override functionality) with properly structured changes: ✅ Database layer: Schema + migration 🤔 Split RecommendationWhile this PR is large, splitting is NOT strongly recommended because:
If splitting is desired, the only reasonable approach would be:
However, this would require PR 1 to merge first and PR 2 would need the API from PR 1. 🤖 Automated analysis by Claude AI |
🔒 Security Scan Results✅ No critical or high severity security vulnerabilities detected This PR has been scanned against OWASP Top 10, CWE Top 25, and common security anti-patterns. The implementation follows security best practices. 📋 OWASP Top 10 Coverage
🛡️ Security Strengths Observed
📝 Informational Notes (Not Security Issues)
🔍 Scanned Files
🤖 Automated security scan by Claude AI - OWASP Top 10 & CWE coverage |
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
📋 Code Review Summary
This PR adds error response override functionality to the error rules system, allowing administrators to customize error responses when specific patterns match. The implementation is comprehensive with database migrations, backend logic, frontend UI, and i18n support.
🔍 Issues Found
- Critical (🔴): 0 issues
- High (🟠): 1 issue
- Medium (🟡): 3 issues
- Low (🟢): 1 issue
🎯 Priority Actions
- 🟠 High: The
buildErrorfunction signature inresponses.tswas not updated to acceptrequest_id, but it's being called with 5 arguments inerror-handler.ts- this will silently ignore the request_id parameter for non-override error paths - 🟡 Medium:
syncDefaultErrorRules()no longer emits theerrorRulesUpdatedevent, butrefreshCacheAction()relies on this for proper cache coordination - potential for inconsistent cache state in multi-instance deployments - 🟡 Medium: The
extractJsonFromStringmethod has unbounded loop iteration on malformed input - consider adding a max iteration limit - 🟡 Medium: Missing test coverage for the new override functionality, especially edge cases like empty messages and invalid status codes
💡 General Observations
- The error override validation is thorough with both schema-level and runtime checks
- The request_id extraction logic handles many edge cases but could benefit from unit tests
- Consider adding database-level constraints for the status code range to prevent invalid data from external sources
🤖 Automated review by Claude AI - focused on identifying issues for improvement
| @@ -6,17 +6,50 @@ All notable changes to this project will be documented in this file. | |||
|
|
|||
| ### Added | |||
|
|
|||
There was a problem hiding this comment.
🔴 Missing Feature Documentation: PR #225 feature not documented in CHANGELOG
Problem: This PR adds a significant new feature "Error Response Override" (错误响应覆盖) that allows users to customize error responses and status codes when error rules match. However, the CHANGELOG.md does not include any entry for this PR's main feature.
The PR:
- Adds
override_responseandoverride_status_codedatabase fields - Implements runtime override logic with validation (400-599 range)
- Adds Claude API error format validator with 10KB size limit
- Creates UI components for override configuration and error rule testing
- Supports i18n for all 5 languages
Impact: Users reviewing the changelog will not be aware of this new capability, which defeats the purpose of maintaining a changelog.
Suggested Fix:
Add an entry under ### Added section:
- Add error response override feature with customizable error responses and status codes for error rules (#225) @ding113Or more detailed:
- Add error response override feature allowing custom error responses and status codes when error rules match, with real-time JSON validation and error rule tester UI (#225) @ding113![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
这个 PR 引入了一个重要的功能：错误响应覆盖。它允许在错误规则匹配时自定义返回给客户端的响应体和状态码。整体实现非常出色，考虑了数据库迁移、后端覆盖逻辑、前端配置界面和测试工具等多个方面。代码质量很高，特别是在错误处理和验证方面做了很多健壮性设计，例如对 requestId 的提取和注入、对数据库中可能存在的脏数据进行运行时校验等。
我提出了几点改进建议，主要集中在提高代码的可维护性和简洁性上：
- 更新日志：建议在
CHANGELOG.md中为这个新功能添加一条记录。 - 代码去重：状态码的验证逻辑和相关常量在
error-handler.ts和error-rules.ts中存在重复，建议将其统一到error-override-validator.ts中，以遵循 DRY 原则。 - 代码简化：在
testErrorRuleAction中，对matchType的验证可以简化，因为其类型已由上游保证。
总的来说，这是一个高质量的提交，我的建议旨在做一些锦上添花的改进。
|
|
||
| ### Added | ||
|
|
||
| - Add provider availability monitoring dashboard with real-time health status, metrics, and heatmap visualization (#216) @ding113 |
There was a problem hiding this comment.
这是一个很棒的新功能，但它似乎没有被记录在 CHANGELOG.md 的 Added 部分。为了方便用户了解版本更新，建议为这个新的错误响应覆盖功能添加一条更新日志。
| - Add provider availability monitoring dashboard with real-time health status, metrics, and heatmap visualization (#216) @ding113 | |
| - Add error response override functionality with status code and body customization, including a frontend tester for real-time validation. | |
| - Add provider availability monitoring dashboard with real-time health status, metrics, and heatmap visualization (#216) @ding113 |
| /** 覆写状态码最小值 */ | ||
| const OVERRIDE_STATUS_CODE_MIN = 400; | ||
| /** 覆写状态码最大值 */ | ||
| const OVERRIDE_STATUS_CODE_MAX = 599; | ||
|
|
||
| /** | ||
| * 验证覆写状态码范围 | ||
| * | ||
| * @param statusCode - 要验证的状态码 | ||
| * @returns 错误消息（如果验证失败）或 null（验证通过） | ||
| */ | ||
| function validateOverrideStatusCodeRange(statusCode: number | null | undefined): string | null { | ||
| if (statusCode === null || statusCode === undefined) { | ||
| return null; | ||
| } | ||
|
|
||
| if ( | ||
| !Number.isInteger(statusCode) || | ||
| statusCode < OVERRIDE_STATUS_CODE_MIN || | ||
| statusCode > OVERRIDE_STATUS_CODE_MAX | ||
| ) { | ||
| return `覆写状态码必须是 ${OVERRIDE_STATUS_CODE_MIN}-${OVERRIDE_STATUS_CODE_MAX} 范围内的整数`; | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
这个 validateOverrideStatusCodeRange 函数和相关的常量是通用的验证逻辑。为了提高代码的模块化和可复用性，建议将它们统一移动到 src/lib/error-override-validator.ts 文件中。这样，其他地方（如 error-handler.ts）的重复逻辑就可以被替换掉。
| const validMatchTypes = ["regex", "contains", "exact"] as const; | ||
| const matchType = validMatchTypes.includes( | ||
| detection.matchType as (typeof validMatchTypes)[number] | ||
| ) | ||
| ? (detection.matchType as "regex" | "contains" | "exact") | ||
| : "regex"; |
There was a problem hiding this comment.
这里的 matchType 验证逻辑有些复杂。errorRuleDetector 的 detect 和 detectAsync 方法已经确保了返回的 detection.matchType 必定是 "regex", "contains", 或 "exact" 中的一个。我们可以通过直接进行类型断言来简化此处的代码，这样更清晰，也更符合类型契约。
| const validMatchTypes = ["regex", "contains", "exact"] as const; | |
| const matchType = validMatchTypes.includes( | |
| detection.matchType as (typeof validMatchTypes)[number] | |
| ) | |
| ? (detection.matchType as "regex" | "contains" | "exact") | |
| : "regex"; | |
| const matchType = detection.matchType as "regex" | "contains" | "exact"; |
| /** 覆写状态码最小值 */ | ||
| const OVERRIDE_STATUS_CODE_MIN = 400; | ||
| /** 覆写状态码最大值 */ | ||
| const OVERRIDE_STATUS_CODE_MAX = 599; |
There was a problem hiding this comment.
这里的常量 OVERRIDE_STATUS_CODE_MIN 和 OVERRIDE_STATUS_CODE_MAX 在 src/actions/error-rules.ts 中也重复定义了。为了避免代码重复并提高可维护性，建议将这些常量和相关的验证函数 (validateOverrideStatusCodeRange) 统一移动到 src/lib/error-override-validator.ts 文件中，并在此处及其他地方导入使用。
📝 Documentation Review📊 Issues Summary
⚡ Priority Fixes
📋 Review Coverage
💡 General ObservationsPositive aspects:
Issue to address: ### Added
- Add error response override feature with customizable error responses and status codes for error rules (#225) @ding113🤖 Automated docs review by Claude AI |
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
Detailed Review Comments
Issue 1: ProxyResponses.buildError signature mismatch (🟠 High)
File: src/app/v1/_lib/proxy/error-handler.ts:161
The buildError function in responses.ts has a 4-parameter signature, but error-handler.ts calls it with 5 arguments including safeRequestId. TypeScript won't catch this because extra arguments are silently ignored in JavaScript. This means the request_id will never appear in non-override error responses.
Fix: Update the buildError signature in responses.ts to accept an optional requestId parameter.
Issue 2: syncDefaultErrorRules event emission removed (🟡 Medium)
File: src/repository/error-rules.ts:355-356
The eventEmitter.emit('errorRulesUpdated') was removed from syncDefaultErrorRules(), but the comment says "由调用方决定是否刷新缓存". However, if multiple server instances are running, other instances won't know about the sync because the event is no longer emitted. This could lead to stale cache in distributed deployments.
Fix: Either restore the event emission or add documentation explaining that this function should only be called from refreshCacheAction which handles cache refresh.
Issue 3: extractJsonFromString unbounded loop (🟡 Medium)
File: src/app/v1/_lib/proxy/errors.ts:226-257 (extractJsonFromString method)
The extractJsonFromString method iterates over the entire string character by character. On malformed input with an opening brace but no matching closing brace, this will iterate through the entire string length. For very large error responses, this could cause performance issues.
Fix: Add a maximum iteration limit or string length check:
private static extractJsonFromString(str: string): string | null {
const MAX_JSON_LENGTH = 10000; // 10KB limit
if (!str.startsWith('{') || str.length > MAX_JSON_LENGTH) {
return null;
}
// ... rest of implementation
}Issue 4: Missing newline at end of migration file (🟢 Low)
File: drizzle/0021_broad_black_panther.sql
The migration file is missing a trailing newline, which is a POSIX standard for text files and can cause issues with some tools.
Fix: Add a newline at the end of the file.
2 participants
概述
添加错误规则匹配时覆盖错误响应和状态码的能力：
override_response(JSONB) 和override_status_code变更内容
数据库
errorRules表添加override_response(JSONB) 和override_status_code字段0021_broad_black_panther.sql后端
src/app/v1/_lib/proxy/error-handler.ts- 覆盖逻辑及验证src/app/v1/_lib/proxy/errors.ts- 错误响应处理src/lib/error-override-validator.ts- Claude API 错误格式验证器src/lib/error-rule-detector.ts- 增强规则检测src/repository/error-rules.ts- 仓库层更新src/actions/error-rules.ts- CRUD Server Actions前端
src/app/[locale]/settings/error-rules/_components/override-section.tsx- 覆盖配置 UIsrc/app/[locale]/settings/error-rules/_components/error-rule-tester.tsx- 规则测试器测试计划
🤖 Generated with Claude Code