Mistral: per-key rate limiting, retry/backoff and improved error handling for streaming requests#65
Conversation
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR implements per-key rate limiting and retry/backoff for Mistral API requests. While the approach is sound, there are 4 critical issues that must be fixed before merge:
Critical Issues (Must Fix):
- Retry loop logic error (lines 1153-1160): The
consecutiveFailurescounter can prevent the code from cycling through all available keys when all are temporarily blocked - Network error handling gap (lines 1197-1211): IOException catch doesn't distinguish between network failures and HTTP errors, causing incorrect cooldown application
- Null safety crash risk (lines 1228-1232): Streaming callback can crash if
selectedKeyForResponseis null - Missing validation (lines 1126-1131): Empty key list validation happens too late, risking NoSuchElementException
Testing Recommendation:
Before merging, test these scenarios:
- All API keys receiving 429 errors simultaneously
- Network timeout during request
- Empty/invalid API key configurations
The implementation correctly replaces the global rate limiter with per-key tracking and adds retry logic for 429/5xx errors, but the issues above prevent it from functioning correctly in edge cases.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| } catch (e: IOException) { | ||
| val requestEndMs = System.currentTimeMillis() | ||
| markKeyCooldown(selectedKey, requestEndMs) | ||
| blockedKeysThisRound.add(selectedKey) | ||
| consecutiveFailures++ | ||
| if (consecutiveFailures >= 5) { | ||
| throw IOException("Mistral request failed after 5 attempts: ${e.message}", e) | ||
| } | ||
| if (retryResponse == null || stopExecutionFlag.get()) { | ||
| throw IOException("Mistral rate limit: Kein Erfolg innerhalb von 5 Sekunden.") | ||
| withContext(Dispatchers.Main) { | ||
| replaceAiMessageText( | ||
| "Mistral Netzwerkfehler (Versuch $consecutiveFailures/5). Wiederhole...", | ||
| isPending = true | ||
| ) | ||
| } | ||
| response = retryResponse | ||
| } |
There was a problem hiding this comment.
🛑 Logic Error: After closing a failed response, the code continues to use selectedKey in error handling without checking if the request actually failed due to network issues versus retryable API errors. The IOException catch block at line 1197 doesn't distinguish between network failures and HTTP errors, potentially causing incorrect cooldown application when the response wasn't even received.
| while (response == null && consecutiveFailures < 5) { | ||
| if (stopExecutionFlag.get()) break | ||
|
|
||
| val now = System.currentTimeMillis() | ||
| val keyPool = availableKeys.filter { it !in blockedKeysThisRound }.ifEmpty { | ||
| blockedKeysThisRound.clear() | ||
| availableKeys | ||
| } |
There was a problem hiding this comment.
🛑 Logic Error: The retry loop condition consecutiveFailures < 5 combined with the increment logic can cause the loop to exit without ever successfully getting a response when all keys are blocked. If all keys fail with retryable errors, blockedKeysThisRound is cleared and the same keys are retried, but consecutiveFailures continues incrementing, potentially hitting 5 before cycling through all available keys even once.
app/src/main/kotlin/com/google/ai/sample/feature/multimodal/PhotoReasoningViewModel.kt
Show resolved
Hide resolved
app/src/main/kotlin/com/google/ai/sample/feature/multimodal/PhotoReasoningViewModel.kt
Show resolved
Hide resolved
…otoReasoningViewModel.kt Co-authored-by: amazon-q-developer[bot] <208079219+amazon-q-developer[bot]@users.noreply.github.com>
…otoReasoningViewModel.kt Co-authored-by: amazon-q-developer[bot] <208079219+amazon-q-developer[bot]@users.noreply.github.com>
|
/q für die anderen beiden auch commit suggestion anbieten. |
|
I'll implement the following changes: • Add null safety check for selectedKeyForResponse in streaming callback (app/src/main/kotlin/com/google/ai/sample/feature/multimodal/PhotoReasoningViewModel.kt:1231-1237) Note: The validation check at line 1131 for empty API keys is already present in the code and functions correctly, so no change is needed for that suggestion. |
|
✅ Changes have been committed. I've implemented the requested improvements to the Mistral API error handling and retry logic. |
- Add null safety check for selectedKeyForResponse in streaming callback - Distinguish between network failures and HTTP errors in IOException handling - Improve retry loop to cycle through all available keys before hitting failure limit - Update retry attempt messages to reflect dynamic max attempts calculation
Android-PowerUser
merged commit b080797
into
codex/analyze-technical-debt-and-complexity-o6bm9p
1 participant
Motivation
Description
lastMistralRequestTimeMslimiter with a per-key cooldown mapmistralNextAllowedRequestAtMsByKeyand track the last token key/time vialastMistralTokenKeyandlastMistralTokenTimeMs.429and>=500as retryable failures while surfacing other errors immediately.markKeyCooldown,remainingWaitForKeyMs, andisRetryableMistralFailureto centralize cooldown and failure logic, and improve user-facing pending messages during retries.Testing
./gradlew :app:testDebugUnitTestand lint checks with./gradlew :app:lint, both of which completed successfully.Codex Task