S2 also retrying 403's (#1017)

Author: jamesbraza

paperqa/clients/semantic_scholar.py

@@ -5,6 +5,7 @@
 from collections.abc import Collection
 from datetime import datetime
 from enum import IntEnum, auto
+from functools import partial
 from http import HTTPStatus
 from itertools import starmap
 from typing import Any
@@ -17,6 +18,7 @@
 from paperqa.utils import (
     _get_with_retrying,
     clean_upbibtex,
+    is_retryable,
     strings_similarity,
     union_collections_to_ordered_list,
 )
@@ -125,6 +127,12 @@ async def _s2_get_with_retrying(url: str, **get_kwargs) -> dict[str, Any]:
             get_kwargs.get("timeout")
             or aiohttp.ClientTimeout(SEMANTIC_SCHOLAR_API_REQUEST_TIMEOUT)
         ),
+        # On 7/21/2025, flaky ClientResponseError was seen with 'citations' traversals on
+        # paper ID 3516396ffa1fd32d4327e199d9b97ec67dc0439a with DOI 10.1126/science.2821624
+        # > aiohttp.client_exceptions.ClientResponseError: 403, message='Forbidden'
+        retry_predicate=partial(
+            is_retryable, additional_status_codes={HTTPStatus.FORBIDDEN}
+        ),
         **get_kwargs,
     )
 

paperqa/utils.py

@@ -10,7 +10,7 @@
 import re
 import string
 import unicodedata
-from collections.abc import Awaitable, Collection, Iterable, Iterator, Mapping
+from collections.abc import Awaitable, Callable, Collection, Iterable, Iterator, Mapping
 from datetime import datetime
 from functools import reduce
 from http import HTTPStatus
@@ -26,8 +26,8 @@
 from pybtex.style.formatting import unsrtalpha
 from pybtex.style.template import FieldIsMissing
 from tenacity import (
+    AsyncRetrying,
     before_sleep_log,
-    retry,
     retry_if_exception,
     stop_after_attempt,
     wait_incrementing,
@@ -402,7 +402,10 @@ def create_bibtex_key(author: list[str], year: str | int, title: str) -> str:
     return remove_substrings(key, FORBIDDEN_KEY_CHARACTERS)
 
 
-def is_retryable(exc: BaseException) -> bool:
+def is_retryable(
+    exc: BaseException,
+    additional_status_codes: Collection[HTTPStatus | int] | None = None,
+) -> bool:
     """Check if an exception is known to be a retryable HTTP issue."""
     if isinstance(
         exc, aiohttp.ServerDisconnectedError | aiohttp.ClientConnectionResetError
@@ -411,33 +414,56 @@ def is_retryable(exc: BaseException) -> bool:
         # > aiohttp.client_exceptions.ClientConnectionResetError:
         # > Cannot write to closing transport
         return True
-    return isinstance(exc, aiohttp.ClientResponseError) and exc.status in {
+    retry_status_codes: set[int] = {
         httpx.codes.INTERNAL_SERVER_ERROR.value,
         httpx.codes.GATEWAY_TIMEOUT.value,
     }
+    if additional_status_codes:
+        retry_status_codes.update(
+            status_code.value if isinstance(status_code, HTTPStatus) else status_code
+            for status_code in additional_status_codes
+        )
+    return (
+        isinstance(exc, aiohttp.ClientResponseError)
+        and exc.status in retry_status_codes
+    )
 
 
-@retry(
-    retry=retry_if_exception(is_retryable),
-    before_sleep=before_sleep_log(logger, logging.WARNING),
-    stop=stop_after_attempt(5),
-    wait=wait_incrementing(0.1, 0.1),
-)
-async def _get_with_retrying(
+async def _get_with_retrying(  # type: ignore[return]  # noqa: RET503
     url: str,
     session: aiohttp.ClientSession,
     http_exception_mappings: dict[HTTPStatus | int, Exception] | None = None,
+    retry_predicate: Callable[[BaseException], bool] = is_retryable,
     **get_kwargs,
 ) -> dict[str, Any]:
-    """Get from a URL with retrying protection."""
-    try:
-        async with session.get(url, **get_kwargs) as response:
-            response.raise_for_status()
-            return await response.json()
-    except aiohttp.ClientResponseError as e:
-        if http_exception_mappings and e.status in http_exception_mappings:
-            raise http_exception_mappings[e.status] from e
-        raise
+    """Get from a URL with retrying protection.
+
+    Args:
+        url: Target URL for the GET request.
+        session: Session for the GET request.
+        http_exception_mappings: Optional mapping of HTTP status codes to
+            custom Exceptions to be thrown.
+        retry_predicate: Optional predicate to dictate when to retry.
+        **get_kwargs: Optional additional keyword arguments for the GET request.
+
+    Returns:
+        JSON result from the GET request.
+    """
+    async for attempt in AsyncRetrying(
+        retry=retry_if_exception(retry_predicate),
+        before_sleep=before_sleep_log(logger, logging.WARNING),
+        stop=stop_after_attempt(5),
+        wait=wait_incrementing(0.1, 0.1),
+    ):
+        with attempt:
+            try:
+                async with session.get(url, **get_kwargs) as response:
+                    response.raise_for_status()
+                    return await response.json()
+            except aiohttp.ClientResponseError as e:
+                if http_exception_mappings and e.status in http_exception_mappings:
+                    raise http_exception_mappings[e.status] from e
+                raise
 
 
 def union_collections_to_ordered_list(collections: Iterable) -> list: