Repository navigation
feat(account-fallback): add daily quota lockout for quota_exhausted errors - #1621
clousky2020 wants to merge 7 commits into
Conversation
![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
This pull request introduces a mechanism to handle daily quota exhaustion by locking models until the following day at 0:00, specifically targeting providers like ModelScope. It includes updates to the account fallback service, chat handlers, and a significant number of translation strings in the English locale. The review feedback focuses on maintaining language consistency by using English for code comments, removing an unused variable in the date utility, and refactoring the error detection logic in the chat handler to use a helper function and safer string handling.
| // 对于日限额耗尽 (quota_exhausted),设置冷却到第二天 0 点 | ||
| // 使用 exactCooldownMs 避免指数缩放，确保精确到明天 0 点 |
There was a problem hiding this comment.
Please use English for comments to maintain consistency with the existing codebase.
| // 对于日限额耗尽 (quota_exhausted),设置冷却到第二天 0 点 | |
| // 使用 exactCooldownMs 避免指数缩放，确保精确到明天 0 点 | |
| // For daily quota exhausted (quota_exhausted), set cooldown until tomorrow 0:00 | |
| // Use exactCooldownMs to avoid exponential scaling, ensuring precision to tomorrow 0:00 |
| const now = new Date(); | ||
| const tomorrow = new Date(now); | ||
| const nowMs = Date.now(); | ||
| const now = new Date(nowMs); |
There was a problem hiding this comment.
The variable now is declared but never used. It should be removed to keep the code clean.
| lockModel, | ||
| recordModelLockoutFailure, |
There was a problem hiding this comment.
Import the isDailyQuotaExhausted helper to maintain consistency in error detection logic.
| lockModel, | |
| recordModelLockoutFailure, | |
| lockModel, | |
| recordModelLockoutFailure, | |
| isDailyQuotaExhausted, | |
| } from "@omniroute/open-sse/services/accountFallback.ts"; |
There was a problem hiding this comment.
Resolved: imported and using isDailyQuotaExhausted helper
| // 6. 日限额错误检查 - 必须在 markAccountUnavailable 之前执行 | ||
| // 检查是否是日限额耗尽错误 (如 ModelScope/Kimi 的 "today's quota for model") | ||
| // 日限额锁定会覆盖后续的 rate_limited 锁定，确保锁定到第二天 0:00 | ||
| let isDailyQuotaExhausted = false; | ||
| if (result.status === 429 && result.error?.includes("today's quota")) { | ||
| // 解析具体哪个模型限额了 | ||
| const match = result.error.match(/today's quota for model ([^,]+)/); | ||
| const limitedModel = match ? match[1].trim() : model; |
There was a problem hiding this comment.
To improve code safety and maintain consistency:
- Use English for comments to match the rest of the codebase.
- Use the
isDailyQuotaExhaustedhelper instead of a hardcoded string check. - Cast
result.errorto a string before calling.match()to avoid potential runtime errors if the error is not a string. - Rename the local variable to
dailyQuotaExhaustedto avoid shadowing the helper function name.
| // 6. 日限额错误检查 - 必须在 markAccountUnavailable 之前执行 | |
| // 检查是否是日限额耗尽错误 (如 ModelScope/Kimi 的 "today's quota for model") | |
| // 日限额锁定会覆盖后续的 rate_limited 锁定，确保锁定到第二天 0:00 | |
| let isDailyQuotaExhausted = false; | |
| if (result.status === 429 && result.error?.includes("today's quota")) { | |
| // 解析具体哪个模型限额了 | |
| const match = result.error.match(/today's quota for model ([^,]+)/); | |
| const limitedModel = match ? match[1].trim() : model; | |
| // 6. Daily quota error check - must be executed before markAccountUnavailable | |
| // Check if it's a daily quota exhausted error (e.g., ModelScope/Kimi "today's quota for model") | |
| // Daily quota lockout overrides subsequent rate_limited lockout, ensuring lockout until tomorrow 0:00 | |
| let dailyQuotaExhausted = false; | |
| const errorStr = String(result.error || ""); | |
| if (result.status === 429 && isDailyQuotaExhausted(errorStr)) { | |
| // Parse which specific model is limited | |
| const match = errorStr.match(/today's quota for model ([^,]+)/); | |
| const limitedModel = match ? match[1].trim() : model; |
| const match = result.error.match(/today's quota for model ([^,]+)/); | ||
| const limitedModel = match ? match[1].trim() : model; | ||
|
|
||
| // 锁定该 connection 的该模型直到第二天 0 点 |
There was a problem hiding this comment.
Please use English for comments to maintain consistency.
| // 锁定该 connection 的该模型直到第二天 0 点 | |
| // Lock this model for this connection until tomorrow 0:00 |
There was a problem hiding this comment.
Resolved: using isDailyQuotaExhausted helper, casting error to string, renamed variable to dailyQuotaExhausted
| }) | ||
| ); | ||
|
|
||
| isDailyQuotaExhausted = true; |
There was a problem hiding this comment.
Update the variable name to match the suggested rename.
| isDailyQuotaExhausted = true; | |
| dailyQuotaExhausted = true; |
| ) { | ||
| markAccountExhaustedFrom429(credentials.connectionId, provider); | ||
| // 日限额错误已经单独处理，这里只处理普通 rate_limit | ||
| if (!isDailyQuotaExhausted) { |
There was a problem hiding this comment.
Update the variable name to match the suggested rename.
| if (!isDailyQuotaExhausted) { | |
| if (!dailyQuotaExhausted) { |
Review: Changes Required Before MergeHi @clousky2020, thanks for this contribution! The daily quota lockout logic is solid and the tests are well-written. However, there are a few issues that need to be addressed before we can merge: 1. ❌ Rebase Required (Merge Conflicts)This PR conflicts with PRs #1624 and #1626 on 2. ❌ zh-CN Punctuation — Must RevertThe PR systematically replaces standard Chinese fullwidth punctuation ( Examples that must be reverted: # ❌ Wrong — don't change punctuation
-"服务器当前存在异常，部分功能可能暂时不可用。"
+"服务器当前存在异常,部分功能可能暂时不可用."
# ✅ Correct — keep fullwidth punctuation
"服务器当前存在异常，部分功能可能暂时不可用。"3. ❌ Translate Chinese Comments to EnglishProduction code must use English comments per project conventions. Please translate: # In src/sse/handlers/chat.ts:
-// 锁定该 connection 的该模型直到第二天 0 点
+// Lock this model on this connection until tomorrow 00:00
-// 日限额错误已经单独处理，这里只处理普通 rate_limit
+// Daily quota errors are handled above; only process regular rate_limit here
-// 7. Mark account as quota-exhausted on 429 response (非日限额错误)
+// 7. Mark account as quota-exhausted on 429 response (non-daily-quota errors)Also in test file -// 计算到本地时间第二天 0:00 的毫秒数
+// Calculate milliseconds until tomorrow 00:00 local time
-// 考虑时区偏移：函数使用本地时间，测试环境可能使用 UTC
+// Account for timezone offset: function uses local time, test env may use UTC
-// 计算实际值与预期值的差异
+// Calculate difference between actual and expected values
-// 允许 ±5 分钟的误差（300,000 ms）
+// Allow ±5 minute tolerance (300,000 ms)✅ What's Good
Once these issues are fixed and the branch is rebased, we'll merge it right away. Thanks! 🙏 |
|
Hey @clousky2020, friendly follow-up! The v3.7.1 release is wrapping up and we would love to include your daily quota lockout feature. The 3 items from the previous review are still needed:
The core quota lockout logic is solid and the tests are well-structured -- just these cleanup items before we can merge. Let us know if you need any help with the rebase! |
cheaper-inference#1630) Cherry-pick from PR cheaper-inference#1630 by @rdself. Fixes Docker build failures caused by lockfile peer dependency mismatch during npm ci.
…ailures - Override plist to v4.0.0 which passes mandatory mimeType to DOMParser - Add USERPROFILE sanitization on Windows to avoid EPERM on junction points - Add NPM_CONFIG_LEGACY_PEER_DEPS to Electron workflow install step
- translate hundreds of placeholder strings in zh-CN to actual Chinese text across costs, audit, providers, settings, evals, docs, and agents pages - normalize Chinese punctuation from full-width commas to half-width throughout zh-CN - refine English labels to sentence case and more concise wording in costs, audit, and provider sections - replace placeholder descriptions with real user-facing text for expiration banners, Zed import, CLI tools redirect, and architecture labels - add new i18n keys for running requests panel, custom CLI tab, today range filter, and dynamic configured count
… errors - when reason is "quota_exhausted" and exactCooldownMs is not provided, set cooldown to milliseconds until tomorrow 0:00 - refactor getMsUntilTomorrow to use Date.now() consistently and cap at 24 hours for DST edge cases 🐛 fix(chat): handle daily quota exhaustion before generic rate limiting - detect "today's quota" error string from providers like ModelScope/Kimi - use recordModelLockoutFailure with quota_exhausted reason to lock the specific model until next day 0:00 - conditionally skip generic rate_limited fallback when daily quota exhaustion already handled ✅ test(account-fallback): add unit tests for quota_exhausted cooldown - verify lockout cooldown is set to milliseconds until tomorrow 0:00 for quota_exhausted reason - verify regular backoff (profile baseCooldownMs) is used for non-quota reasons
- translate quota exhaustion comments in recordModelLockoutFailure to English ♻️ refactor(accountFallback): remove unused variable in getMsUntilTomorrow - remove unused `now` variable that was only assigned but never referenced
…ling - Replace hardcoded string check with isDailyQuotaExhausted helper - Cast result.error to string before .match() to avoid runtime errors - Rename local variable to dailyQuotaExhausted to avoid shadowing helper function - Translate comments to English for codebase consistency Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ents - Restore fullwidth Chinese punctuation (，。：) in zh-CN.json that was incorrectly changed to ASCII - Translate remaining Chinese comments in chat.ts to English - Translate Chinese comments in account-fallback-service.test.ts to English Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6057f14 to
e3d16d6
Compare
|
Superseded by #1644 which was merged into |
Summary
Changes
🤖 Generated with Claude Code