Repository navigation
feat: add ModelScope provider-specific 429 handling and retry logic - #2202
Conversation
* "Claude PR Assistant workflow" * "Claude Code Review workflow"
Bumps [mermaid](https://github.com/mermaid-js/mermaid) from 11.14.0 to 11.15.0. - [Release notes](https://github.com/mermaid-js/mermaid/releases) - [Commits](https://github.com/mermaid-js/mermaid/compare/mermaid@11.14.0...mermaid@11.15.0) --- updated-dependencies: - dependency-name: mermaid dependency-version: 11.15.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
- Add modelscopePolicy.ts: centralized ModelScope rate limit parsing and 429 classification - Support dual provider detection: provider ID and baseURL domain matching - Implement per-model lock on quota exhaustion (not connection-wide) - Add automatic retry with backoff for rate-limited requests (max 3 attempts) - Parse Modelscope-Ratelimit-* headers for accurate quota tracking - Fix Docker build: add build tools for native module compilation - Fix .dockerignore: allow docs/openapi.yaml for container build
![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 specialized handling for the ModelScope provider, including a new policy service to classify 429 errors and manage retries. It also updates the Docker build environment with necessary dependencies and integrates ModelScope-specific rate-limiting and locking logic into the chat handler. Feedback suggests optimizing the reading of 429 response bodies with size limits or timeouts to ensure efficiency and recommends a future refactor to centralize the fragmented model-locking logic.
| const bodyPeek = await res.response | ||
| .clone() | ||
| .text() | ||
| .catch(() => ""); |
There was a problem hiding this comment.
Cloning the response and reading the full body as text for every 429 error can be inefficient, especially if the upstream response is large. While 429 bodies are typically small, it's safer to limit the read size or ensure this only happens for ModelScope. Since isModelScope() is already checked, this is acceptable, but consider adding a timeout or size limit to the text() call if possible in this environment.
| if (isModelScope() && connectionId) { | ||
| lockModel(provider, connectionId, model, "quota_exhausted", quotaCooldownMs); | ||
| console.warn( | ||
| `[provider] Node ${connectionId} ModelScope model quota exhausted (${statusCode}) for ${model} - ${Math.ceil(quotaCooldownMs / 1000)}s (connection stays active)` | ||
| ); |
There was a problem hiding this comment.
The explicit check for isModelScope() here to trigger lockModel bypasses the hasPerModelQuota logic in accountFallback.ts. While necessary because hasPerModelQuota lacks access to credentials for URL-based detection, this creates a slight fragmentation of model-locking logic. Consider if hasPerModelQuota could be updated to accept providerSpecificData in a future refactor to centralize this policy.
# Conflicts: # .github/workflows/claude-code-review.yml # .github/workflows/claude.yml
|
Thanks @InkshadeWoods. I synced this branch with release/v3.8.0, removed unrelated workflow/docker/package-lock changes, tightened the ModelScope 429 classifier so temporary rate-limit headers remain retryable, added policy coverage, and validated locally with:\n\n- |
e6aee3d
into
cheaper-inference:release/v3.8.0
…heaper-inference#2202) Integrated into release/v3.8.0 after syncing the contributor branch, removing unrelated workflow/docker/package-lock changes, tightening ModelScope 429 classification, and validating policy coverage locally.
…heaper-inference#2202) Integrated into release/v3.8.0 after syncing the contributor branch, removing unrelated workflow/docker/package-lock changes, tightening ModelScope 429 classification, and validating policy coverage locally.
…heaper-inference#2202) Integrated into release/v3.8.0 after syncing the contributor branch, removing unrelated workflow/docker/package-lock changes, tightening ModelScope 429 classification, and validating policy coverage locally.
Description
Add ModelScope (魔搭) provider-specific 429 error handling and retry logic to improve reliability when using ModelScope-hosted models.
Changes:
Summary
When using ModelScope as a provider, 429 rate limit errors are now automatically detected and retried (up to 3 attempts) instead of immediately failing. The system correctly distinguishes between rate limiting (retryable) and quota exhaustion (not retryable) using ModelScope's response headers.
Related Issues
Validation
npm run lintnpm run test:unitnpm run test:coverage>= 60%for statements, lines, functions, and branchesTests Added Or Updated
No new tests added. The changes are isolated to ModelScope-specific 429 handling logic in existing files (
open-sse/handlers/chatCore.tsandopen-sse/services/modelscopePolicy.ts). The functionality was manually tested with a live ModelScope provider (DeepSeek-V4-Flash) and verified working.Coverage Notes
Changes are in
open-sse/handlers/chatCore.tsandopen-sse/services/modelscopePolicy.ts. These files handle request routing and error classification. Existing tests for chatCore.ts should cover the new ModelScope branch.Reviewer Notes
The ModelScope-specific logic is isolated in
modelscopePolicy.tsand only activates when the provider is identified as ModelScope. No breaking changes to existing functionality. Docker build fixes are included to ensure the project builds successfully in containerized environments.