Skip to content

fix(rate-limit): prevent endless queue with maxWait (#297) - #301

Merged
diegosouzapw merged 1 commit into
mainfrom
fix/issue-297-rate-limit-maxwait
Mar 11, 2026
Merged

diegosouzapw merged 1 commit into
mainfrom
fix/issue-297-rate-limit-maxwait

Conversation

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Summary

Fixes #297 — requests hang indefinitely when all providers are rate-limited.

Root Cause

Bottleneck was configured without maxWait, so when all providers returned 429 and reservoir=0, queued jobs waited forever. Clients (Cursor, Claude Code, VS Code) would show 'waiting...' indefinitely.

Fix

Added maxWait: MAX_WAIT_MS (default 2 minutes, configurable via RATE_LIMIT_MAX_WAIT_MS) to:

  • DEFAULT_SETTINGS (all ad-hoc limiters)
  • Custom rpm/tpm Bottleneck constructor
  • Auto-enabled API key Bottleneck constructor

When a job exceeds maxWait, Bottleneck throws a BottleneckError, which propagates as a clean 502/503 error to the client instead of hanging.

…#297)

When all provider quotas are exhausted (reservoir=0 after repeated 429s),
Bottleneck's schedule() would queue requests indefinitely since no maxWait
was configured. Clients (Cursor, Claude Code, VS Code) would hang forever.

Fix: add maxWait=120000 (2min, configurable via RATE_LIMIT_MAX_WAIT_MS env)
to DEFAULT_SETTINGS and all three Bottleneck constructors. When a job waits
longer than maxWait, Bottleneck rejects with a BottleneckError which
propagates as a 502/503 error to the client — a clean fail-fast instead
of infinite hang.
Copilot AI review requested due to automatic review settings March 11, 2026 03:01
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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!

This pull request addresses a critical issue where requests could become stuck in an endless queue when rate limits were hit across all providers. By implementing a configurable maximum wait time for queued jobs, the system now gracefully handles rate limit exhaustion, ensuring that requests either proceed within a defined timeframe or fail with a clear error, thereby improving client experience and system stability.

Highlights

  • Prevented indefinite queuing: Introduced a maximum wait time for rate-limited jobs, preventing requests from hanging indefinitely when all providers are exhausted.
  • Configurable maximum wait time: The maximum wait time (maxWait) is now configurable via the RATE_LIMIT_MAX_WAIT_MS environment variable, defaulting to 2 minutes.
  • Improved error handling: Jobs exceeding the maxWait will now fail with a BottleneckError, propagating as a clean 502/503 error to the client instead of causing indefinite hangs.
  • Applied maxWait across rate limiters: The maxWait setting has been added to default rate limiter settings, custom RPM/TPM Bottleneck constructors, and auto-enabled API key Bottleneck constructors.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • open-sse/services/rateLimitManager.ts
    • Defined MAX_WAIT_MS constant, configurable via RATE_LIMIT_MAX_WAIT_MS environment variable, defaulting to 120 seconds.
    • Added maxWait: MAX_WAIT_MS to the DEFAULT_SETTINGS object for ad-hoc limiters.
    • Included maxWait: MAX_WAIT_MS in the Bottleneck constructor for custom RPM/TPM limits.
    • Applied maxWait: MAX_WAIT_MS to the Bottleneck constructor used for auto-enabled API key limits.
Activity
  • No human activity has occurred on this pull request yet.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

// Max time (ms) a job can wait in queue before failing with a timeout error.
// Prevents infinite queuing when all providers are exhausted after a 429.
// Configurable via RATE_LIMIT_MAX_WAIT_MS env var (default: 2 minutes).
const MAX_WAIT_MS = parseInt(process.env.RATE_LIMIT_MAX_WAIT_MS || "120000", 10);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Missing validation for invalid or negative values — parseInt returns NaN for non-numeric strings (e.g., RATE_LIMIT_MAX_WAIT_MS=abc), and negative values would be invalid timeouts. Consider adding validation:

const MAX_WAIT_MS = Math.max(0, parseInt(process.env.RATE_LIMIT_MAX_WAIT_MS || "120000", 10));

@kilo-code-bot

kilo-code-bot Bot commented Mar 11, 2026

Copy link
Copy Markdown

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
open-sse/services/rateLimitManager.ts 65 Missing validation for invalid or negative RATE_LIMIT_MAX_WAIT_MS env var values — parseInt returns NaN for non-numeric strings, and negative values would be invalid timeouts
Files Reviewed (1 file)
  • open-sse/services/rateLimitManager.ts - 1 issue

Note: This PR adds a fail-safe mechanism (maxWait) to prevent requests from being stuck in infinite queues when all providers are exhausted after receiving 429 errors. The implementation correctly applies the MAX_WAIT_MS setting to all Bottleneck limiter constructors. The only issue found is the lack of input validation for the environment variable.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses an issue where requests could hang indefinitely by introducing a maxWait timeout to the Bottleneck rate limiters. The change correctly applies this new timeout to default settings, custom limiters, and auto-enabled limiters. My review includes a suggestion to improve the parsing of the timeout value from environment variables for better robustness and code consistency.

// Max time (ms) a job can wait in queue before failing with a timeout error.
// Prevents infinite queuing when all providers are exhausted after a 429.
// Configurable via RATE_LIMIT_MAX_WAIT_MS env var (default: 2 minutes).
const MAX_WAIT_MS = parseInt(process.env.RATE_LIMIT_MAX_WAIT_MS || "120000", 10);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current parsing of RATE_LIMIT_MAX_WAIT_MS can result in NaN if a non-numeric string is provided in the environment variable, which could cause unexpected behavior in the Bottleneck limiter. For safer and more consistent parsing, it's better to use the existing toNumber helper function, which is designed to handle such cases gracefully.

Suggested change
const MAX_WAIT_MS = parseInt(process.env.RATE_LIMIT_MAX_WAIT_MS || "120000", 10);
const MAX_WAIT_MS = toNumber(process.env.RATE_LIMIT_MAX_WAIT_MS, 120000);

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a configurable queue timeout to the adaptive rate limiting layer to prevent requests from waiting indefinitely when providers are rate-limited/exhausted (e.g., sustained 429 conditions).

Changes:

  • Introduces RATE_LIMIT_MAX_WAIT_MS (default 120000ms) to cap how long requests can remain queued.
  • Applies the timeout to both default limiter settings and limiters instantiated during startup initialization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

// Max time (ms) a job can wait in queue before failing with a timeout error.
// Prevents infinite queuing when all providers are exhausted after a 429.
// Configurable via RATE_LIMIT_MAX_WAIT_MS env var (default: 2 minutes).
const MAX_WAIT_MS = parseInt(process.env.RATE_LIMIT_MAX_WAIT_MS || "120000", 10);

Copilot AI Mar 11, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAX_WAIT_MS is derived via parseInt(...) without validating the result. If RATE_LIMIT_MAX_WAIT_MS is set to a non-numeric value (or includes separators like "120_000"), parseInt can yield NaN/partial parses, which then propagates into Bottleneck options and breaks the documented default behavior. Please validate/clamp this env var (e.g., use the existing toNumber helper with a 120000 fallback and enforce >= 0).

Suggested change
const MAX_WAIT_MS = parseInt(process.env.RATE_LIMIT_MAX_WAIT_MS || "120000", 10);
const RAW_MAX_WAIT_MS = toNumber(process.env.RATE_LIMIT_MAX_WAIT_MS, 120000);
const MAX_WAIT_MS = Math.max(0, Math.floor(RAW_MAX_WAIT_MS));

Copilot uses AI. Check for mistakes.
reservoir: null, // No initial reservoir — unlimited until we learn
reservoirRefreshAmount: null,
reservoirRefreshInterval: null,
maxWait: MAX_WAIT_MS, // Fail-fast: don't queue forever on 429 exhaustion

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL: maxWait is not a valid Bottleneck option — Bottleneck v2.19.5 does not support this property. It will be silently ignored, and jobs will still queue indefinitely. The intended fail-fast behavior will not work.

Consider using maxQueue to limit queue size (by count), or implementing a custom timeout wrapper using Promise.race() with a timeout promise.

reservoir: rpm,
reservoirRefreshAmount: rpm,
reservoirRefreshInterval: 60 * 1000,
maxWait: MAX_WAIT_MS,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL: maxWait is not a valid Bottleneck option — this will be silently ignored. See comment on line 74.

reservoir: DEFAULT_API_LIMITS.requestsPerMinute,
reservoirRefreshAmount: DEFAULT_API_LIMITS.requestsPerMinute,
reservoirRefreshInterval: 60 * 1000, // Refresh every minute
maxWait: MAX_WAIT_MS,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL: maxWait is not a valid Bottleneck option — this will be silently ignored. See comment on line 74.

@kilo-code-bot

kilo-code-bot Bot commented Mar 11, 2026

Copy link
Copy Markdown

Code Review Summary

Status: 4 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 3
WARNING 1
Issue Details (click to expand)

CRITICAL

File Line Issue
open-sse/services/rateLimitManager.ts 74 maxWait is not a valid Bottleneck option — the library does not support this property. It will be silently ignored, and jobs will still queue indefinitely.
open-sse/services/rateLimitManager.ts 120 Same issue as line 74 — maxWait will be silently ignored.
open-sse/services/rateLimitManager.ts 145 Same issue as line 74 — maxWait will be silently ignored.

WARNING

File Line Issue
open-sse/services/rateLimitManager.ts 65 Missing validation for invalid or negative values in parseInt. Consider using Math.max(0, parseInt(...)).
Files Reviewed (1 file)
  • open-sse/services/rateLimitManager.ts - 4 issues

Fix Link: Fix these issues in Kilo Cloud

@diegosouzapw
diegosouzapw merged commit be6c769 into main Mar 11, 2026
17 checks passed
@diegosouzapw
diegosouzapw deleted the fix/issue-297-rate-limit-maxwait branch March 11, 2026 03:20
diegosouzapw added a commit that referenced this pull request Mar 11, 2026
fix(docker): healthcheck now uses /api/monitoring/health (#296, PR #301)
fix(rate-limit): maxWait=120s on Bottleneck prevents endless queue (#297, PR #302)
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
…297-rate-limit-maxwait

fix(rate-limit): prevent endless queue with maxWait (cheaper-inference#297)
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
fix(docker): healthcheck now uses /api/monitoring/health (cheaper-inference#296, PR cheaper-inference#301)
fix(rate-limit): maxWait=120s on Bottleneck prevents endless queue (cheaper-inference#297, PR cheaper-inference#302)
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…297-rate-limit-maxwait

fix(rate-limit): prevent endless queue with maxWait (cheaper-inference#297)
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
fix(docker): healthcheck now uses /api/monitoring/health (cheaper-inference#296, PR cheaper-inference#301)
fix(rate-limit): maxWait=120s on Bottleneck prevents endless queue (cheaper-inference#297, PR cheaper-inference#302)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: 429 Rate Limit causes endless queuing / requests stuck forever after fallback failure

2 participants