fix: retry http request on receiving a retriable error code from bybit#341
fix: retry http request on receiving a retriable error code from bybit#341thefloat wants to merge 1 commit intobybit-exchange:masterfrom
Conversation
…r code was encountered from bybit response
There was a problem hiding this comment.
Pull request overview
This pull request fixes a critical bug in the retry mechanism for handling retryable Bybit API error codes. The previous implementation attempted to raise a generic Exception which wasn't caught by the retry loop's exception handlers, preventing retries from working. The fix introduces a new RetryableError exception class and properly integrates it into the exception handling flow.
Changes:
- Added new
RetryableErrorexception class to properly signal retryable errors - Updated
_handle_responseto raiseRetryableErrorinstead of genericExceptionfor retryable error codes - Added exception handling for
RetryableErrorin_submit_requestretry loop
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pybit/exceptions.py | Adds new RetryableError exception class with response, message, and status_code attributes to signal retryable API errors |
| pybit/_http_manager.py | Updates retry logic to raise and catch RetryableError, enabling the retry mechanism to work correctly by integrating the exception into the existing retry loop |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| except JSONDecodeError as e: | ||
| self._handle_json_error(e, retries_attempted) | ||
| except RetryableError as e: | ||
| self._handle_retryable_error(e.response, e.status_code, e.message, recv_window) |
There was a problem hiding this comment.
The recv_window parameter modifications in _handle_retryable_error will not persist across retries. When error code 10002 is encountered, _handle_retryable_error adds 2500 to recv_window (line 312), but since recv_window is an integer (immutable in Python), this modification is lost when the exception handler returns and the retry loop continues. The next retry will still use the original recv_window value from line 182. This means the recv_window adjustment feature for error code 10002 is non-functional. Consider making recv_window an instance variable or returning the updated value from _handle_retryable_error and using it in subsequent iterations.
| self._handle_retryable_error(e.response, e.status_code, e.message, recv_window) | |
| recv_window = self._handle_retryable_error(e.response, e.status_code, e.message, recv_window) |
| Exception raised for retryable bybit error code | ||
|
|
||
| Attributes: | ||
| response -- Response object |
There was a problem hiding this comment.
The docstring states "Attributes: response -- Response object" but the attribute name in the code is "response", not matching the naming convention seen in other exception classes like InvalidRequestError and FailedRequestError which use more descriptive names like "request", "message", "status_code", "time", and "resp_headers". For consistency, consider documenting what type of response object this is (e.g., "response -- HTTP Response object from requests library").
| response -- Response object | |
| response -- HTTP response object (for example, a Response instance from the requests library). |
|
|
||
| class RetryableError(Exception): | ||
| """ | ||
| Exception raised for retryable bybit error code |
There was a problem hiding this comment.
The word "bybit" should be capitalized as "Bybit" to match the proper noun format used elsewhere in the codebase. For example, other exception classes like InvalidRequestError use "Bybit" in their docstrings (line 39: "Exception raised for returned Bybit errors").
| Exception raised for retryable bybit error code | |
| Exception raised for retryable Bybit error code |
| except RetryableError as e: | ||
| self._handle_retryable_error(e.response, e.status_code, e.message, recv_window) | ||
|
|
||
|
|
There was a problem hiding this comment.
The addition of a blank line here creates two consecutive blank lines (lines 206 and 207) before the raise statement. The codebase convention throughout this file shows only one blank line separating code blocks (e.g., between methods at lines 305-306, 322-323, 330-331). Consider removing this extra blank line to maintain consistency.
Though all the required code for the functionality was pretty much there. Retrying when a retryable response code was encountered was somehow being evaded.
Probably from a bad merge or incomplete implementation.