feat(http): implement rate-limit handling with auto-retry (#11)#24
Merged
codebestia merged 6 commits intoJun 24, 2026
Merged
Conversation
Contributor
|
Hello @bukkybyte |
Contributor
|
Please fix the CI as well |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds HTTP error types, low-level sync and async HTTP clients with 429 retry handling, rewrites Gateway to use them, adds aiohttp as a dependency, and updates tests for the new request and retry behavior. ChangesRate-limit handling: HTTP clients, error types, and Gateway wiring
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Gateway
participant SyncHTTPClient
participant AsyncHTTPClient
Caller->>Gateway: process_payment(amount, currency)
Gateway->>SyncHTTPClient: request("POST", "/payments", payload)
SyncHTTPClient-->>Gateway: response body
Gateway-->>Caller: Dict[str, Any]
Caller->>Gateway: process_payment_async(amount, currency)
Gateway->>AsyncHTTPClient: request("POST", "/payments", payload)
AsyncHTTPClient-->>Gateway: response body
Gateway-->>Caller: Dict[str, Any]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shade/errors.py (1)
43-57: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRemove the duplicate methods inside
RateLimitError.These definitions override the 429-specific
__init__/__str__above, soRateLimitError(..., retry_after=...)will now fail andstatus_codeis no longer guaranteed to stay429.Proposed fix
class RateLimitError(HTTPError): @@ def __str__(self) -> str: # pragma: no cover base = super().__str__() if self.retry_after is not None: return f"{base} (retry after {self.retry_after}s)" return base - def __init__( - self, - message: str, - status_code: Optional[int] = None, - response_body: Optional[str] = None, - ) -> None: - self.message = message - self.status_code = status_code - self.response_body = response_body - super().__init__(message) - - def __str__(self) -> str: - if self.status_code is None: - return self.message - return f"{self.message} (status code: {self.status_code})"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shade/errors.py` around lines 43 - 57, RateLimitError’s duplicate __init__ and __str__ are overriding the 429-specific behavior and breaking retry_after handling. Remove the redundant methods from RateLimitError so it inherits the existing 429-aware constructor/formatter, and keep the status_code fixed at 429 via the shared error implementation in the errors module.
🧹 Nitpick comments (2)
src/shade/__init__.py (1)
15-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate
__all__while fixing the Ruff export-list warning.
GatewayandShadeErrorare listed twice here. Cleaning that up makes the public surface unambiguous and should simplify theRUF022follow-up.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shade/__init__.py` around lines 15 - 28, The export list in the __all__ definition contains duplicate symbols, which triggers the Ruff export-list warning. Update the __all__ list in src/shade/__init__.py to remove the repeated Gateway and ShadeError entries so each public export appears only once, keeping the intended API surface unchanged.Source: Linters/SAST tools
tests/test_gateway.py (1)
11-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a matching test for
process_payment_async.This file now verifies the sync path only, but the PR also adds
Gateway.process_payment_async()insrc/shade/gateway.py:79-87. A small test that patchesgateway._async_http.requestand asserts the same method/path/payload would close the main coverage gap in the new public API.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_gateway.py` around lines 11 - 21, Add a test for Gateway.process_payment_async to cover the new async public API alongside test_process_payment. In tests/test_gateway.py, mirror the existing sync test by patching gateway._async_http.request, invoking process_payment_async, and asserting it returns the mocked response and calls request with the same POST /payments payload used by process_payment. Use the existing Gateway test setup so the new async coverage stays consistent with the sync path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/shade/gateway.py`:
- Around line 27-45: The Gateway initializer currently allows an empty api_key,
but SyncHTTPClient and AsyncHTTPClient still attach Authorization headers, so
Gateway() can be created in a broken state. Update Gateway.__init__ to require a
non-empty api_key and fail immediately by raising an error before constructing
the HTTP clients. Keep the fix localized to Gateway.__init__ and ensure the
validation happens before SyncHTTPClient and AsyncHTTPClient are instantiated.
In `@src/shade/http.py`:
- Around line 123-133: The sync client constructor currently accepts any
caller-provided base_url, which can allow non-HTTP schemes through
urllib.request.urlopen. Update the __init__ path in the HTTP client to validate
that base_url is an absolute http:// or https:// URL before storing it, and
reject any other scheme early with a clear error; keep the same behavior in the
sync request flow so only HTTP/HTTPS endpoints are usable. Use the HTTP client
constructor and any request helper methods that consume self.base_url as the
reference points for the change.
In `@tests/test_rate_limit.py`:
- Around line 162-165: The test file contains semicolon-chained statements that
Ruff flags with E702, so split each combined assignment/increment into separate
lines throughout the affected helper blocks. Update the `fake_execute` function
and the other nearby test sections using the same pattern so each `nonlocal`
update, assignment, and increment is on its own line, which will unblock lint
and CI. Keep the logic unchanged while refactoring the affected test helpers and
response-index handling.
---
Outside diff comments:
In `@src/shade/errors.py`:
- Around line 43-57: RateLimitError’s duplicate __init__ and __str__ are
overriding the 429-specific behavior and breaking retry_after handling. Remove
the redundant methods from RateLimitError so it inherits the existing 429-aware
constructor/formatter, and keep the status_code fixed at 429 via the shared
error implementation in the errors module.
---
Nitpick comments:
In `@src/shade/__init__.py`:
- Around line 15-28: The export list in the __all__ definition contains
duplicate symbols, which triggers the Ruff export-list warning. Update the
__all__ list in src/shade/__init__.py to remove the repeated Gateway and
ShadeError entries so each public export appears only once, keeping the intended
API surface unchanged.
In `@tests/test_gateway.py`:
- Around line 11-21: Add a test for Gateway.process_payment_async to cover the
new async public API alongside test_process_payment. In tests/test_gateway.py,
mirror the existing sync test by patching gateway._async_http.request, invoking
process_payment_async, and asserting it returns the mocked response and calls
request with the same POST /payments payload used by process_payment. Use the
existing Gateway test setup so the new async coverage stays consistent with the
sync path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d4863d73-494a-402c-afe1-79494609b3bd
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
pyproject.tomlsrc/shade/__init__.pysrc/shade/errors.pysrc/shade/gateway.pysrc/shade/http.pytests/test_gateway.pytests/test_rate_limit.py
Contributor
|
@bukkybyte |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_gateway.py`:
- Around line 31-33: The test in test_gateway.py is using event-loop management
that should be replaced with a simpler synchronous async call. Update the test
that invokes gateway.process_payment_async to use asyncio.run instead of
asyncio.get_event_loop().run_until_complete, keeping the assertion logic the
same and ensuring the test remains a normal sync test without relying on
pytest-asyncio.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7fc759fd-83c0-4752-802a-53b614e3a310
📒 Files selected for processing (6)
src/shade/__init__.pysrc/shade/errors.pysrc/shade/gateway.pysrc/shade/http.pytests/test_gateway.pytests/test_rate_limit.py
🚧 Files skipped from review as they are similar to previous changes (5)
- src/shade/init.py
- src/shade/gateway.py
- src/shade/errors.py
- tests/test_rate_limit.py
- src/shade/http.py
Contributor
Author
resolved. please check |
codebestia
approved these changes
Jun 24, 2026
codebestia
left a comment
codebestia
left a comment
Contributor
There was a problem hiding this comment.
LGTM!
Nice Implementation.
Thank you for your contribution.
1 task
11 tasks
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
2 participants
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.
PR description:
Closes #11
What
Adds automatic 429 handling so callers never need to implement their own backoff.
Changes
src/shade/errors.py—RateLimitError(retry_after=...)in aShadeError → HTTPError → RateLimitErrorhierarchysrc/shade/http.py—SyncHTTPClientandAsyncHTTPClientwith retry loop;Retry-Afterheader parsing; exponential fallbacksrc/shade/gateway.py— wired to both clients; exposesprocess_payment_async()tests/test_rate_limit.py— 19 tests covering all acceptance criteriaBehaviour
Summary by CodeRabbit
Retry-Afterdetails when available.aiohttpas a runtime dependency for async requests.