Skip to content

feat(rate-limit/unstable): add rate limiting module#7063

Open
tomas-zijdemans wants to merge 18 commits intodenoland:mainfrom
tomas-zijdemans:rate-limit
Open

feat(rate-limit/unstable): add rate limiting module#7063
tomas-zijdemans wants to merge 18 commits intodenoland:mainfrom
tomas-zijdemans:rate-limit

Conversation

@tomas-zijdemans
Copy link
Copy Markdown
Contributor

@tomas-zijdemans tomas-zijdemans commented Mar 23, 2026

Rate limiting strategies for controlling how many operations can occur over time.

The primary API is createRateLimiter, a keyed rate limiter for the common case of "allow key X at most N requests per window." It supports fixed-window, sliding-window, token-bucket, and GCRA algorithms. For single-resource limiting, use the primitives: createTokenBucket, createFixedWindow, and createSlidingWindow.

Depends on @std/data-structures: Deque for async queue management and RollingCounter for sliding-window segment tracking.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 95.89303% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.45%. Comparing base (1cd63ca) to head (53d22fb).

Files with missing lines Patch % Lines
rate_limit/_redis_scripts.ts 83.33% 15 Missing and 4 partials ⚠️
rate_limit/_replenishing_limiter.ts 91.80% 9 Missing and 6 partials ⚠️
rate_limit/_algorithms.ts 96.55% 6 Missing and 1 partial ⚠️
rate_limit/_keyed_algorithms.ts 99.02% 0 Missing and 1 partial ⚠️
rate_limit/rate_limiter.ts 97.91% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7063      +/-   ##
==========================================
+ Coverage   94.41%   94.45%   +0.04%     
==========================================
  Files         630      641      +11     
  Lines       50490    51537    +1047     
  Branches     8949     9086     +137     
==========================================
+ Hits        47669    48680    +1011     
- Misses       2249     2273      +24     
- Partials      572      584      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

This is a well-architected and thoroughly tested package. The layered design (pure algorithms → keyed wrapper → async queue/timer/disposal → public API) is clean, and the algorithm implementations look mathematically correct. Solid work.

A few things to address before merge:

Blockers:

  1. CI: PR title validation is failing — rate-limit needs to be added to the scopes list in .github/workflows/title.yml in a preceding PR.
  2. Discussion: Is there a tracking issue or RFC for adding a rate-limiting package to @std? This is a significant addition (~5100 lines) and new packages typically need community buy-in.
  3. mod.ts: Ensure it has the @module JSDoc tag per repo conventions.

Design/correctness notes:

  1. peek() in keyed algorithms mutates state: peek() calls ops.advance(), which rotates segment counters in sliding-window. This means "peeking" isn't truly read-only — it advances time. This is necessary for correct metadata but should be documented explicitly.
  2. Depends on unstable APIs: Imports RollingCounter from @std/data-structures/unstable-rolling-counter and uses Deque. Fine for v0.1.0 but worth noting as a stability risk.
  3. TokenBucket result() uses this.computeRetryAfter() rather than calling the method directly — minor style inconsistency with the other algorithm implementations which inline the computation.
  4. Some JSDoc examples use limiter[Symbol.dispose]() syntax — consider using the using statement for consistency with newer examples elsewhere.
  5. Naming inconsistency: TokenBucketOptions uses tokenLimit + tokensPerPeriod + replenishmentPeriod, while createRateLimiter uses limit + window + tokensPerCycle. The different naming across the same domain could be confusing.
  6. Floating-point edge cases: TokenBucket uses fractional tokens internally (Math.floor(state.tokens) for remaining). No tests specifically exercise floating-point boundary behavior (e.g., tokens at 4.999... after refill).

@tomas-zijdemans
Copy link
Copy Markdown
Contributor Author

Thank you for the review. I pushed a new commit now:

  1. CI: PR title validation is failing:
    title.yml is updated as part of the PR, so that check will pass once this PR is merged. I felt it would be a bit weird to send a pre-PR for that, since that would assume the whole module is already accepted. I used the same approach for the XML module, hopefully this is OK 🤞

  2. Discussion: Is there a tracking issue:
    No. Rate limiting is a very common use case though, there's prior art. I used the .Net standard library as inspiration. Also there is this community discussion

  3. mod.ts @module tag:
    The tag is on line 24 in mod.ts, so no change was needed here ☺️

  4. peek() mutates state:
    I added a comment on peek() explaining that it advances the internal clock. Also added a matching comment on the implementation in _keyed_algorithms.ts.

  5. Unstable API dependencies
    I'd like to push back on this one. I think it's quite common that unstable imports unstable. Our lint checks will catch this when promoting to stable.

  6. result() style inconsistency
    Fixed in the latest commit. Thanks!

  7. Symbol.dispose in JSDoc examples:
    Replaced with using declaration in the RateLimitLease example in types.ts.

  8. Naming inconsistency.
    Fixed in the latest commit. Thanks!

  9. Floating-point edge cases :
    I added more tests for this in the latest PR. But honestly I think it's a bit overkill. There's no actual code path that produces fractional tokens.

Let me know if you agree/disagree or have any further feedback ☺️

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.

2 participants