Add Discord rate limiting with exponential backoff and Redis coordination (#284) #288
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| import asyncio | ||
| import random | ||
| import logging | ||
| import time | ||
| import discord | ||
| import redis.asyncio as redis | ||
|
|
||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class DiscordRateLimiter: | ||
| def __init__(self, redis_url: str | None = None, max_retries: int = 3): | ||
| self.max_retries = max_retries | ||
| self.redis = redis.from_url(redis_url) if redis_url else None | ||
|
Comment on lines
+12
to
+15
Contributor
There was a problem hiding this comment. Missing Redis connection error handling and cleanup. If Redis becomes unavailable after initialization, Consider wrapping Redis operations in try/except and adding a cleanup method: ♻️ Suggested resilience pattern class DiscordRateLimiter:
def __init__(self, redis_url: str | None = None, max_retries: int = 3):
self.max_retries = max_retries
self.redis = redis.from_url(redis_url) if redis_url else None
+
+ async def close(self):
+ if self.redis:
+ await self.redis.close()And in try:
reset_time = await self.redis.get(key)
except Exception:
logger.warning("Redis unavailable, skipping rate-limit check")
return🤖 Prompt for AI Agents |
||
|
|
||
| def _calculate_backoff(self, attempt: int, retry_after: float) -> float: | ||
| """ | ||
| Exponential backoff with jitter: | ||
| delay = (2 ** attempt * retry_after) + jitter | ||
| """ | ||
| jitter = random.uniform(0, 0.3) | ||
| return (2 ** attempt * retry_after) + jitter | ||
|
|
||
| async def _wait_if_limited(self, bucket: str) -> None: | ||
| """Wait if Redis indicates this bucket is currently rate limited.""" | ||
| if not self.redis: | ||
| return | ||
|
|
||
| key = f"discord_ratelimit:{bucket}" | ||
| reset_time = await self.redis.get(key) | ||
|
|
||
| if reset_time: | ||
| delay = float(reset_time) - time.time() | ||
| if delay > 0: | ||
| logger.warning( | ||
| f"Bucket {bucket} rate limited. Waiting {delay:.2f}s" | ||
| ) | ||
| await asyncio.sleep(delay) | ||
|
|
||
| async def _set_limit(self, bucket: str, retry_after: float) -> None: | ||
| """Store rate limit reset timestamp in Redis.""" | ||
| if not self.redis: | ||
| return | ||
|
|
||
| key = f"discord_ratelimit:{bucket}" | ||
| reset_at = time.time() + retry_after | ||
|
|
||
| await self.redis.set( | ||
| key, | ||
| reset_at, | ||
| ex=int(retry_after) + 1 | ||
| ) | ||
|
|
||
| async def execute_with_retry(self, func, bucket: str, *args, **kwargs): | ||
| """ | ||
| Execute a Discord API call with automatic retry on 429. | ||
| """ | ||
|
|
||
| for attempt in range(self.max_retries + 1): | ||
| try: | ||
| await self._wait_if_limited(bucket) | ||
| return await func(*args, **kwargs) | ||
|
|
||
| except discord.HTTPException as e: | ||
| if e.status != 429: | ||
| raise | ||
|
|
||
| retry_after = getattr(e, "retry_after", 1) | ||
|
|
||
| await self._set_limit(bucket, retry_after) | ||
|
|
||
| delay = self._calculate_backoff(attempt, retry_after) | ||
|
|
||
| logger.warning( | ||
| f"429 hit for bucket {bucket}. " | ||
| f"Attempt {attempt + 1}/{self.max_retries}. " | ||
| f"Retrying in {delay:.2f}s" | ||
| ) | ||
|
|
||
| await asyncio.sleep(delay) | ||
|
|
||
| logger.error(f"Max retries exhausted for bucket {bucket}") | ||
| raise discord.HTTPException( | ||
| response=None, | ||
| message="Discord rate limit exceeded after retries." | ||
|
Comment on lines
+55
to
+86
Contributor
There was a problem hiding this comment. Wasted sleep on last failed attempt and misleading log message. Two issues in the retry loop:
♻️ Proposed fix except discord.HTTPException as e:
if e.status != 429:
raise
retry_after = getattr(e, "retry_after", 1)
await self._set_limit(bucket, retry_after)
+ if attempt >= self.max_retries:
+ break
+
delay = self._calculate_backoff(attempt, retry_after)
logger.warning(
f"429 hit for bucket {bucket}. "
- f"Attempt {attempt + 1}/{self.max_retries}. "
+ f"Attempt {attempt + 1}/{self.max_retries + 1}. "
f"Retrying in {delay:.2f}s"
)
await asyncio.sleep(delay)🤖 Prompt for AI Agents |
||
| ) | ||
|
Comment on lines
+83
to
+87
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check discord.HTTPException implementation
pip install discord.py==2.4.0 2>/dev/null
python3 << 'EOF'
import inspect
import discord
# Get the HTTPException class
try:
src = inspect.getsource(discord.HTTPException.__init__)
print("=== discord.HTTPException.__init__ ===")
print(src)
print("\n")
except Exception as e:
print(f"Error getting __init__: {e}")
# Also check the class definition
try:
src = inspect.getsource(discord.HTTPException)
print("=== discord.HTTPException class (first 100 lines) ===")
lines = src.split('\n')[:100]
print('\n'.join(lines))
except Exception as e:
print(f"Error getting class: {e}")
EOFRepository: AOSSIE-Org/Devr.AI Length of output: 5210 🏁 Script executed: # Check the context in backend/rate_limiter.py
cat -n backend/rate_limiter.py | sed -n '75,95p'Repository: AOSSIE-Org/Devr.AI Length of output: 620 🏁 Script executed: # Search for usages of discord.HTTPException in the codebase
rg "discord\.HTTPException" --type py -A 3 -B 3Repository: AOSSIE-Org/Devr.AI Length of output: 1233 Replace Passing Consider creating a mock response object (as done in tests with 🤖 Prompt for AI Agents |
||
Oops, something went wrong.
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.
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.
channel_locksdict grows unboundedly.A new
asyncio.Lockis allocated for every unique channel ID and never evicted. Over time (especially with thread-per-user model in_get_or_create_thread), this will leak memory. Consider an LRU eviction strategy or periodic cleanup of locks for channels no longer inactive_threads.🤖 Prompt for AI Agents