-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add rate limiter for API calls #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| export interface RateLimitConfig { | ||
| maxRequests: number; | ||
| windowMs: number; | ||
| } | ||
|
|
||
| export class RateLimiter { | ||
| private timestamps: number[] = []; | ||
| private config: RateLimitConfig; | ||
|
|
||
| constructor(config: Partial<RateLimitConfig> = {}) { | ||
| this.config = { | ||
| maxRequests: config.maxRequests ?? 10, | ||
| windowMs: config.windowMs ?? 60000, | ||
| }; | ||
| } | ||
|
|
||
| tryAcquire(): boolean { | ||
| const now = Date.now(); | ||
| this.timestamps = this.timestamps.filter((t) => now - t < this.config.windowMs); | ||
|
|
||
| if (this.timestamps.length <= this.config.maxRequests) { | ||
| this.timestamps.push(now); | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| remainingRequests(): number { | ||
| const now = Date.now(); | ||
| const active = this.timestamps.filter((t) => now - t < this.config.windowMs); | ||
| return this.config.maxRequests - active.length; | ||
| } | ||
|
|
||
| async waitForSlot(): Promise<void> { | ||
| while (!this.tryAcquire()) { | ||
| const oldest = this.timestamps[0]; | ||
| const waitMs = this.config.windowMs - (Date.now() - oldest); | ||
| await new Promise((resolve) => setTimeout(resolve, waitMs)); | ||
| } | ||
| } | ||
|
|
||
| reset(windowMs?: number) { | ||
| this.timestamps = []; | ||
| if (windowMs) { | ||
| this.config.windowMs = windowMs; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export function parseRateLimit(header: string): RateLimitConfig | null { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [INFO] correctness
Suggestion: Add a guard: |
||
| const parts = header.split(","); | ||
| let maxRequests: any = null; | ||
| let windowMs: any = null; | ||
|
|
||
| for (const part of parts) { | ||
| const [key, val] = part.split("="); | ||
| switch (key.trim()) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [INFO] type-discipline
Suggestion: Declare as |
||
| case "limit": | ||
| maxRequests = parseInt(val); | ||
| break; | ||
| case "window": | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [WARNING] error-handling If a header part lacks Suggestion: Validate the parse result explicitly: |
||
| windowMs = parseInt(val) * 1000; | ||
| break; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [INFO] correctness
Suggestion: Pass radix 10: |
||
| } | ||
| } | ||
|
|
||
| if (maxRequests && windowMs) return { maxRequests, windowMs }; | ||
| return null; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[ERROR] correctness
Off-by-one: the condition
this.timestamps.length <= this.config.maxRequestsallows maxRequests+1 requests through. After filtering expired entries, if there are exactlymaxRequestsactive timestamps, the check still passes and one more is pushed, exceeding the configured limit.Suggestion: Change
<=to<:if (this.timestamps.length < this.config.maxRequests) { ... }