diff --git a/blockapi/test/v2/api/nft/test_opensea.py b/blockapi/test/v2/api/nft/test_opensea.py index f7b6b1e..f518257 100644 --- a/blockapi/test/v2/api/nft/test_opensea.py +++ b/blockapi/test/v2/api/nft/test_opensea.py @@ -84,7 +84,7 @@ def test_fetch_ntfs( nfts = api.fetch_nfts(nfts_test_address) assert len(nfts.data) == 2 assert len(fake_sleep_provider.calls) - assert fake_sleep_provider.calls[0] == ('https://api.opensea.io/', 0.25) + assert fake_sleep_provider.calls[0] == ('https://api.opensea.io/', 0.75) def test_fetch_ntfs_error_response(requests_mock, api, fake_sleep_provider): @@ -96,7 +96,7 @@ def test_fetch_ntfs_error_response(requests_mock, api, fake_sleep_provider): nfts = api.fetch_nfts(nfts_test_address) assert len(nfts.data) == 0 assert len(fake_sleep_provider.calls) - assert fake_sleep_provider.calls[0] == ('https://api.opensea.io/', 0.25) + assert fake_sleep_provider.calls[0] == ('https://api.opensea.io/', 0.75) def test_fetch_offers( @@ -114,7 +114,7 @@ def test_fetch_offers( offers = api.fetch_offers(test_collection_slug) assert len(offers.data) == 2 assert len(fake_sleep_provider.calls) - assert fake_sleep_provider.calls[0] == ('https://api.opensea.io/', 0.25) + assert fake_sleep_provider.calls[0] == ('https://api.opensea.io/', 0.75) def test_fetch_offers_error_response(requests_mock, api, offers_response): diff --git a/blockapi/test/v2/test_base.py b/blockapi/test/v2/test_base.py index 458822c..980069f 100644 --- a/blockapi/test/v2/test_base.py +++ b/blockapi/test/v2/test_base.py @@ -1,3 +1,4 @@ +import logging from unittest.mock import MagicMock, patch import pytest @@ -117,3 +118,60 @@ def mocked_500_with_success_response(): def test_5xx_will_retry(customizable_api, mocked_500_with_success_response): response = customizable_api.get_data("test_method") assert response.status_code == 200 + + +@pytest.fixture() +def mocked_429_with_success_response(): + mocked_throttled = MagicMock() + mocked_throttled.status_code = 429 + mocked_throttled.raise_for_status.side_effect = HTTPError( + "test_method", 429, "exception", {}, None + ) + mocked_throttled.json.return_value = {} + mocked_throttled.headers = {} + + mocked_success = MagicMock() + mocked_success.status_code = 200 + mocked_success.json.return_value = {} + mocked_success.headers = {} + + with patch('blockapi.v2.base.CustomizableBlockchainApi._get_response') as patched: + patched.side_effect = [ + mocked_throttled, + mocked_success, + ] + yield patched + + +def test_429_retried_logs_warning_not_error( + customizable_api, mocked_429_with_success_response, caplog +): + with caplog.at_level(logging.WARNING, logger='blockapi.v2.base'): + response = customizable_api.get_data("test_method") + + assert response.status_code == 200 + assert not any(r.levelno >= logging.ERROR for r in caplog.records) + assert any(r.levelno == logging.WARNING for r in caplog.records) + + +@pytest.fixture() +def mocked_429_only_response(): + mocked_throttled = MagicMock() + mocked_throttled.status_code = 429 + mocked_throttled.raise_for_status.side_effect = HTTPError( + "test_method", 429, "exception", {}, None + ) + mocked_throttled.json.return_value = {} + mocked_throttled.headers = {} + + with patch('blockapi.v2.base.CustomizableBlockchainApi._get_response') as patched: + patched.side_effect = [mocked_throttled] * 5 + yield patched + + +def test_429_exhausted_logs_error(customizable_api, mocked_429_only_response, caplog): + with caplog.at_level(logging.WARNING, logger='blockapi.v2.base'): + response = customizable_api.get_data("test_method") + + assert response.status_code == 429 + assert any(r.levelno == logging.ERROR for r in caplog.records) diff --git a/blockapi/v2/api/nft/magic_eden.py b/blockapi/v2/api/nft/magic_eden.py index d701ba9..eef9df4 100644 --- a/blockapi/v2/api/nft/magic_eden.py +++ b/blockapi/v2/api/nft/magic_eden.py @@ -399,7 +399,7 @@ def _should_retry(self, data: FetchResult) -> bool: return False retry = bool( - [t for t in data.errors if str(t).upper() == 'SERVICE UNAVAILABLE'] + [t for t in data.errors if 'SERVICE UNAVAILABLE' in str(t).upper()] ) if retry: logger.warning('Service unavailable - will retry after long sleep') diff --git a/blockapi/v2/api/nft/opensea.py b/blockapi/v2/api/nft/opensea.py index 90636e3..8b3223e 100644 --- a/blockapi/v2/api/nft/opensea.py +++ b/blockapi/v2/api/nft/opensea.py @@ -75,7 +75,7 @@ class OpenSeaApi(BlockchainApi, INftProvider, INftParser): api_options = ApiOptions( blockchain=Blockchain.ETHEREUM, base_url='https://api.opensea.io/', - rate_limit=0.25, # 4 per second + rate_limit=0.75, # 4 requests per 3 seconds ) supported_requests = { diff --git a/blockapi/v2/base.py b/blockapi/v2/base.py index c65926f..27ff6c5 100644 --- a/blockapi/v2/base.py +++ b/blockapi/v2/base.py @@ -113,15 +113,17 @@ def get_data( self.sleep_provider.sleep(self.base_url, seconds=sleep_seconds) continue except HTTPError: - logger.error(f"Request failed with http error: {response.status_code}") - retries -= 1 - if ( - retries <= 0 - or (response.status_code < 500 and response.status_code != 429) - or not self.sleep_provider - ): + retryable = response.status_code == 429 or response.status_code >= 500 + + if retries <= 0 or not retryable or not self.sleep_provider: + # Genuine failure: out of retries, non-retryable status, + # or nothing to pace the retry with. This is the only path + # that surfaces an error to the caller, so log at ERROR. + logger.error( + f"Request failed with http error: {response.status_code}" + ) time = self._get_response_time(response.headers) return FetchResult( status_code=response.status_code, @@ -137,9 +139,12 @@ def get_data( except ValueError: seconds = 60 + # Retryable error that will be retried, so this is expected + # noise (e.g. 429 throttling) rather than a failure: WARNING. logger.warning( - f'Too Many Requests: Will retry after {seconds}s sleep.' - f' Remaining attempts {retries}.' + f'Request failed with retryable http error' + f' {response.status_code}: will retry after {seconds}s' + f' sleep. Remaining attempts {retries}.' ) self.sleep_provider.sleep(self.base_url, seconds=seconds) continue @@ -193,15 +198,19 @@ def _check_and_get_from_response(self, response: Response) -> Dict: def _get_reason(response): reason = response.reason if not reason and response.status_code >= 400: - return f'Error {response.status_code}' + reason = f'Error {response.status_code}' - if not isinstance(reason, bytes): - return reason - - try: - return reason.decode("utf-8") - except UnicodeDecodeError: - return reason.decode("iso-8859-1") + if isinstance(reason, bytes): + try: + reason = reason.decode("utf-8") + except UnicodeDecodeError: + reason = reason.decode("iso-8859-1") + + # Append the response body so the error carries the server's own + # message instead of just the HTTP status phrase. Truncated to keep + # logs readable. + body = (response.text or '').strip() + return f'{reason}: {body[:500]}' if body else reason @staticmethod def _raise_from_response(response: Response) -> None: