Repository navigation
fix(usage): honor active rateLimitedUntil cooldown on provider-limits sync - #11313
Closed
pacocartones wants to merge 1 commit into
Closed
pacocartones wants to merge 1 commit into
pacocartones wants to merge 1 commit into
Conversation
… sync maybeClearRecoveredQuotaState() gated every defensive guard behind lastErrorType === "quota_exhausted". Any other lastErrorType fell straight through to clearRecoveredProviderState and wiped a still-active rateLimitedUntil without ever consulting it. lastErrorType is not normalized across writers: the provider test route persists diagnosis.type (upstream_rate_limited, network_error, ...), which never equals "quota_exhausted", while persisting a real rateLimitedUntil. Combined with hasUsableQuota() returning true on the first non-zero window, a multi-day cooldown was cleared on every 5-10 minute sync, burning a real upstream call, taking the same 429 and cooling down again. Adds exported hasActiveCooldown() and hoists the quota-window guards out of the lastErrorType branch so they apply whenever a cooldown is still active. hasActiveCooldown fails OPEN on an unparseable timestamp so a corrupt value can never pin a connection offline forever. The change is additive: the quota_exhausted refinements keep their exact previous behaviour. Fixes cheaper-inference#11277
Contributor
Author
|
Closing this — #11310 landed the same fix and #11277 is closed as completed. Your version is the better shape: hoisting the guards out of the No action needed here. Thanks for the quick turnaround. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #11277
Problem
maybeClearRecoveredQuotaState()gated every defensive guard behindlastErrorType === "quota_exhausted". Any otherlastErrorTypefell straight through toclearRecoveredProviderStateand wiped a still-activerateLimitedUntilwithout ever consulting it.lastErrorTypeis not normalized across writers. The provider test route persistsdiagnosis.type(upstream_rate_limited,network_error, ...), which never equals"quota_exhausted", while persisting a realrateLimitedUntil. Combined withhasUsableQuota()returning true on the first non-zero window — so a connection with a 5h window and a weekly window looks recovered while the weekly one is still at zero — a multi-day cooldown was cleared on every 5-10 minute sync, burning a real upstream call, taking the same 429, and cooling down again. That is the loop the reporter observed with a 146-hour cooldown.Fix
Adds an exported
hasActiveCooldown()and hoists the quota-window guards out of thelastErrorTypebranch so they apply whenever a cooldown is still in force, regardless of which writer set it.hasActiveCooldowndeliberately fails open on an unparseable timestamp: a corrupt value must never be able to pin a connection offline forever.The change is additive — the
quota_exhaustedrefinements keep their exact previous behaviour for the case where the cooldown has already expired.Test
Added to the existing
tests/unit/provider-limits-recovery.test.ts.Verified failing on the base commit, 2 of 17:
With the fix, 17/17 pass:
Run with the repository's own unit runner (
node --testvia thetest:unitscript), not vitest —tests/unit/*.test.tsis covered by the former.Branch is up to date with
release/v3.8.50.