-
Notifications
You must be signed in to change notification settings - Fork 36
Description
Feature Request: A possible hook for custom error handling in request strategies
Is your feature request related to a problem? Please describe.
When I want to implement custom logic for handling specific request errors, like timeouts or API rate limits, I find that I need to override the entire _make_request method in my RequestStrategy subclass.
The current implementation in RequestStrategy handles exceptions inside _make_request:
except Exception as error:
raise UnexpectedError(f"Error when contacting '{endpoint}'") from errorTo change this behavior, I have to copy the rest of the _make_request method's logic into my subclass. This works, but it can be a bit cumbersome and means my implementation might get out of sync if the base _make_request method is updated in the future.
Describe the solution you'd like
I was thinking that it might be helpful to have a dedicated method for handling these errors, which could be overridden by subclasses.
Perhaps we could introduce a new method, something like _handle_request_error, in the BaseRequestStrategy. The _make_request method would then call this new method in its except block:
# In RequestStrategy._make_request
try:
# ... existing request code ...
except Exception as error:
self._handle_request_error(error, endpoint)The default implementation in BaseRequestStrategy could keep the exact same behavior as today, so nothing would break for existing users:
# In BaseRequestStrategy
def _handle_request_error(self, error: Exception, endpoint: str) -> None:
raise UnexpectedError(f"Error when contacting '{endpoint}'") from errorThis way, if someone wanted to add custom handling for a specific error, they could just override the new _handle_request_error method. For example:
from requests.exceptions import ReadTimeout
class CustomRequestStrategy(RequestStrategy):
def _handle_request_error(self, error: Exception, endpoint: str) -> None:
if isinstance(error, ReadTimeout):
# My custom handling for timeouts
raise TimeoutError(f"Request to {endpoint} timed out.") from error
# For all other errors, I can just use the default behavior
super()._handle_request_error(error, endpoint)Describe alternatives you've considered
As mentioned, the main alternative today is to override the whole _make_request method. This is a valid approach, but this suggestion is aimed at making the process a little easier and reducing code duplication for users who only want to tweak the error handling.
Additional context
I think a change like this could be useful for handling different kinds of errors that might require special logic, like:
- Request timeouts
- Rate limiting responses
- Specific network issues
- API-specific error formats
This is just a suggestion, and I'm sure you would have a better idea of the best way to implement it. Thank you for considering this feature request