From 0d741022a930a6eed769f0f84abbb9386f991af5 Mon Sep 17 00:00:00 2001 From: samm81 Date: Sat, 6 Sep 2025 15:24:14 +0800 Subject: [PATCH] http: add rate limiting (mainly for google) - google calendar uses the `403` and `429` codes to perform rate limiting [1][2]. this pr adds `tenacity` to perform exponential back off as suggested in google calendar's docs [3]. [1]: https://developers.google.com/workspace/calendar/api/guides/errors#403_rate_limit_exceeded [2]: https://developers.google.com/workspace/calendar/api/guides/errors#429_too_many_requests [3]: https://developers.google.com/workspace/calendar/api/guides/quota#backoff --- pyproject.toml | 1 + tests/storage/test_http.py | 41 +++++++++++ tests/unit/test_retry.py | 140 +++++++++++++++++++++++++++++++++++++ vdirsyncer/http.py | 53 ++++++++++++++ 4 files changed, 235 insertions(+) create mode 100644 tests/unit/test_retry.py diff --git a/pyproject.toml b/pyproject.toml index 3f2f6db..3bdf28a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,6 +39,7 @@ dependencies = [ "requests>=2.20.0", "aiohttp>=3.8.2,<4.0.0", "aiostream>=0.4.3,<0.8.0", + "tenacity>=9.0.0", ] dynamic = ["version"] diff --git a/tests/storage/test_http.py b/tests/storage/test_http.py index 091dfd9..cfb86ce 100644 --- a/tests/storage/test_http.py +++ b/tests/storage/test_http.py @@ -1,13 +1,16 @@ from __future__ import annotations import pytest +import aiohttp from aioresponses import CallbackResult from aioresponses import aioresponses from tests import normalize_item from vdirsyncer.exceptions import UserError +from vdirsyncer.http import request from vdirsyncer.http import BasicAuthMethod from vdirsyncer.http import DigestAuthMethod +from vdirsyncer.http import UsageLimitReached from vdirsyncer.storage.http import HttpStorage from vdirsyncer.storage.http import prepare_auth @@ -120,3 +123,41 @@ def test_verify_false_disallowed(aio_connector): HttpStorage(url="http://example.com", verify=False, connector=aio_connector) assert "must be a path to a pem-file." in str(excinfo.value).lower() + + +@pytest.mark.asyncio +async def test_403_usage_limit_exceeded(aio_connector): + url = "http://127.0.0.1/test_403" + error_body = { + "error": { + "errors": [ + { + "domain": "usageLimits", + "message": "Calendar usage limits exceeded.", + "reason": "quotaExceeded", + } + ], + "code": 403, + "message": "Calendar usage limits exceeded.", + } + } + + async with aiohttp.ClientSession(connector=aio_connector) as session: + with aioresponses() as m: + m.get(url, status=403, payload=error_body, repeat=True) + with pytest.raises(UsageLimitReached): + await request("GET", url, session) + + +@pytest.mark.asyncio +async def test_403_without_usage_limits_domain(aio_connector): + """A 403 JSON error without the Google 'usageLimits' domain should not be + treated as UsageLimitReached and should surface as ClientResponseError. + """ + url = "http://127.0.0.1/test_403_no_usage_limits" + + async with aiohttp.ClientSession(connector=aio_connector) as session: + with aioresponses() as m: + m.get(url, status=403, repeat=True) + with pytest.raises(aiohttp.ClientResponseError): + await request("GET", url, session) diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py new file mode 100644 index 0000000..e1374f6 --- /dev/null +++ b/tests/unit/test_retry.py @@ -0,0 +1,140 @@ +from __future__ import annotations + +import json +from unittest.mock import AsyncMock, Mock + +import aiohttp +import pytest +from aioresponses import aioresponses + +from vdirsyncer.exceptions import Error as VdirsyncerError +from vdirsyncer.http import UsageLimitReached, request + + +@pytest.mark.asyncio +async def _create_mock_response(status: int, body: str | dict): + raw_body = body + if isinstance(body, dict): + text_body = json.dumps(body) + else: + text_body = body + + mock_response = AsyncMock() + mock_response.status = status + mock_response.ok = 200 <= status < 300 + mock_response.reason = "OK" if mock_response.ok else "Forbidden" + mock_response.headers = ( + {"Content-Type": "application/json"} + if isinstance(raw_body, dict) + else {"Content-Type": "text/plain"} + ) + mock_response.text.return_value = text_body + if isinstance(raw_body, dict): + mock_response.json.return_value = raw_body + else: + mock_response.json.side_effect = ValueError("Not JSON") + mock_response.raise_for_status = Mock( + side_effect=( + aiohttp.ClientResponseError( + request_info=AsyncMock(), + history=(), + status=status, + message=mock_response.reason, + headers=mock_response.headers, + ) + if not mock_response.ok + else None + ) + ) + + return mock_response + + +@pytest.mark.asyncio +async def test_request_retry_on_usage_limit(): + url = "http://example.com/api" + max_retries = 5 # As configured in the @retry decorator + + mock_session = AsyncMock() + + # Simulate (max_retries - 1) 403 errors and then a 200 OK + mock_session.request.side_effect = [ + await _create_mock_response( + 403, + { + "error": { + "errors": [{"domain": "usageLimits", "reason": "quotaExceeded"}] + } + }, + ) + for _ in range(max_retries - 1) + ] + [await _create_mock_response(200, "OK")] + + async with ( + aiohttp.ClientSession() + ) as session: # Dummy session. Will be replaced by mock_session at call + response = await request("GET", url, mock_session) + + assert response.status == 200 + assert mock_session.request.call_count == max_retries + + +@pytest.mark.asyncio +async def test_request_retry_exceeds_max_attempts(): + url = "http://example.com/api" + max_retries = 5 # As configured in the @retry decorator + + mock_session = AsyncMock() + # Simulate max_retries 403 errors and then a 200 OK + mock_session.request.side_effect = [ + await _create_mock_response( + 403, + { + "error": { + "errors": [{"domain": "usageLimits", "reason": "quotaExceeded"}] + } + }, + ) + for _ in range(max_retries) + ] + + async with ( + aiohttp.ClientSession() + ) as session: # Dummy session. Will be replaced by mock_session at call + with pytest.raises(UsageLimitReached): + await request("GET", url, mock_session) + assert mock_session.request.call_count == max_retries + + +@pytest.mark.asyncio +async def test_request_no_retry_on_generic_403_json(): + url = "http://example.com/api" + + mock_session = AsyncMock() + # Generic non-Google 403 error payload (e.g., GitHub-style) + mock_session.request.side_effect = [ + await _create_mock_response(403, {"message": "API rate limit exceeded"}) + ] + + async with aiohttp.ClientSession() as session: + with pytest.raises(aiohttp.ClientResponseError): + await request("GET", url, mock_session) + # Should not retry because it's not the Google quotaExceeded shape + assert mock_session.request.call_count == 1 + + +@pytest.mark.asyncio +async def test_request_no_retry_on_generic_403_text(): + url = "http://example.com/api" + + mock_session = AsyncMock() + # Plain-text 403 body mentioning rate limits, but not structured as Google error + mock_session.request.side_effect = [ + await _create_mock_response(403, "Rate limit exceeded") + ] + + async with aiohttp.ClientSession() as session: + with pytest.raises(aiohttp.ClientResponseError): + await request("GET", url, mock_session) + # Should not retry because the JSON shape is not Google quotaExceeded + assert mock_session.request.call_count == 1 diff --git a/vdirsyncer/http.py b/vdirsyncer/http.py index 7b152dd..954e5be 100644 --- a/vdirsyncer/http.py +++ b/vdirsyncer/http.py @@ -12,6 +12,12 @@ from ssl import create_default_context import aiohttp import requests.auth from requests.utils import parse_dict_header +from tenacity import ( + retry, + retry_if_exception_type, + stop_after_attempt, + wait_exponential, +) from . import __version__ from . import exceptions @@ -148,6 +154,47 @@ def prepare_client_cert(cert): return cert +class UsageLimitReached(exceptions.Error): + pass + + +async def _is_quota_exceeded_google(response: aiohttp.ClientResponse) -> bool: + """Return True if the response JSON indicates Google-style `usageLimits` exceeded. + + Expected shape: + {"error": {"errors": [{"domain": "usageLimits", ...}], ...}} + + See https://developers.google.com/workspace/calendar/api/guides/errors#403_usage_limits_exceeded + """ + try: + data = await response.json(content_type=None) + except Exception: + return False + + if not isinstance(data, dict): + return False + + error = data.get("error") + if not isinstance(error, dict): + return False + + errors = error.get("errors") + if not isinstance(errors, list): + return False + + for entry in errors: + if isinstance(entry, dict) and entry.get("domain") == "usageLimits": + return True + + return False + + +@retry( + stop=stop_after_attempt(5), + wait=wait_exponential(multiplier=1, min=4, max=10), + retry=retry_if_exception_type(UsageLimitReached), + reraise=True, +) async def request( method, url, @@ -210,6 +257,12 @@ async def request( # some other error, will be handled later on break + if response.status == 429: + raise UsageLimitReached(response.reason) + + if response.status == 403 and await _is_quota_exceeded_google(response): + raise UsageLimitReached(response.reason) + # See https://github.com/kennethreitz/requests/issues/2042 content_type = response.headers.get("Content-Type", "") if (