Skip to content

Commit

Permalink
fix logging on request wait/backoff handling
Browse files Browse the repository at this point in the history
  • Loading branch information
geo-martino committed Jun 27, 2024
1 parent 6fbcae1 commit 5220380
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 10 deletions.
2 changes: 1 addition & 1 deletion musify/api/cache/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ async def request(self, method: str, url: str | URL, json: Any = None, persist:

yield response

if persist and repository is not None and not isinstance(response, CachedResponse):
if persist and repository is not None and response.ok and not isinstance(response, CachedResponse):
await repository.save_response(response)

async def _get_cached_response(
Expand Down
26 changes: 17 additions & 9 deletions musify/api/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,11 @@ class RequestHandler:
"backoff_start",
"backoff_factor",
"backoff_count",
"_initial_backoff_logged",
"_backoff_start_logged",
"wait_time",
"wait_increment",
"wait_max",
"_wait_start_logged",
)

@property
Expand Down Expand Up @@ -102,14 +103,16 @@ def __init__(self, connector: Callable[[], ClientSession], authoriser: APIAuthor
self.backoff_factor = 1.932
#: The maximum number of request attempts to make before giving up and raising an exception
self.backoff_count = 10
self._initial_backoff_logged = False
self._backoff_start_logged = False

#: The initial time in seconds to wait after receiving a response from a request
self.wait_time = 0
#: The amount in seconds to increase the wait time by each time a rate limit is hit i.e. 429 response
self.wait_increment = 0.1
#: The maximum time in seconds that the wait time can be incremented to
self.wait_max = 1
self._wait_start_logged = False


async def __aenter__(self) -> Self:
if self.closed:
Expand Down Expand Up @@ -182,12 +185,12 @@ async def request(self, method: str, url: str | URL, **kwargs) -> dict[str, Any]
raise APIError("Max retries exceeded")

# exponential backoff
self._log_initial_backoff()
self._log_backoff_start()
self.log(method=method, url=url, message=f"Request failed: retrying in {backoff} seconds...")
await asyncio.sleep(backoff)
backoff *= self.backoff_factor

self._initial_backoff_logged = False
self._backoff_start_logged = False
return data

@contextlib.asynccontextmanager
Expand Down Expand Up @@ -248,15 +251,15 @@ async def _response_is_ok(self, response: ClientResponse) -> bool:
return int(error_status) < 400
return response.ok

def _log_initial_backoff(self) -> None:
if self._initial_backoff_logged:
def _log_backoff_start(self) -> None:
if self._backoff_start_logged:
return

self.logger.info_extra(
"\33[93mRequest failed: retrying using backoff strategy. "
f"Will retry request {self.backoff_count} more times and timeout in {self.timeout} seconds...\33[0m"
)
self._initial_backoff_logged = True
self._backoff_start_logged = True

async def _log_response(self, response: ClientResponse, method: str, url: str | URL) -> None:
"""Log the method, URL, response text, and response headers."""
Expand Down Expand Up @@ -324,8 +327,13 @@ async def _wait_for_rate_limit_timeout(self, response: ClientResponse) -> bool:
f"Retry again at {wait_str}"
)

self.logger.info_extra(f"\33[93mRate limit exceeded. Retrying again at {wait_str}\33[0m")
sleep(wait_time)
if not self._wait_start_logged:
self.logger.info_extra(f"\33[93mRate limit exceeded. Retrying again at {wait_str}\33[0m")
self._wait_start_logged = True

await asyncio.sleep(wait_time)
self._wait_start_logged = False

return True

@staticmethod
Expand Down

0 comments on commit 5220380

Please sign in to comment.