open-swe/agent/utils/github_http.py
Johannes du Plessis 7dd758f845
feat: shared GitHub HTTP helper with retries, rate-limit handling [closes OPE-45] (#1565)
* feat: shared GitHub HTTP helper with retries, rate-limit handling, and sane timeouts

Introduces agent/utils/github_http.py — a single place for GitHub API HTTP
calls with 30s/10s-connect timeouts (vs httpx's 5s default), exponential
backoff with jitter, Retry-After header support, and 429/secondary-rate-limit
detection. Migrates the reviewer publish path (reviewer_publish.py,
reviewer_diff.py, github_checks.py, github_ci.py) from one-shot
httpx.AsyncClient() calls to the shared helper.

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>

* fix: don't retry transport errors on non-idempotent GitHub writes

POST/DELETE/PATCH can create side effects server-side even when the client
gets a timeout or connection reset. Only retry transport errors for
idempotent methods (GET, HEAD, PUT, DELETE). 429/5xx status codes are still
retried for all methods since the server explicitly did not process the
request.

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>

* fix: don't retry 502/504 on non-idempotent GitHub writes

502 (bad gateway) and 504 (gateway timeout) are ambiguous — the upstream
may have processed the write before the gateway returned an error. Only
retry these for idempotent methods. 429 and 503 are still retried for all
methods since the server explicitly did not process the request.

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>

---------

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
2026-06-17 14:45:57 -07:00

194 lines
7 KiB
Python

"""Shared GitHub HTTP helper with sane timeouts, retries, and rate-limit handling.
All GitHub API calls in the reviewer publish path (and gradually everywhere else)
should go through ``github_request`` instead of raw ``httpx.AsyncClient`` calls.
This centralises:
- **Timeouts**: httpx defaults to 5 s which is too aggressive for paginated
GitHub/GraphQL fetches. The default here is 30 s read / 10 s connect.
- **Retries**: exponential backoff with jitter for retryable HTTP status codes
and transport errors, gated by method idempotency to prevent duplicate writes.
See ``github_request`` for the full retry matrix.
- **Rate-limit awareness**: respects ``Retry-After`` headers and detects
GitHub secondary rate limits (403 with ``X-RateLimit-Remaining: 0`` or a
"secondary rate limit" body message), backing off before retrying.
"""
from __future__ import annotations
import asyncio
import logging
import random
from collections.abc import AsyncIterator
from contextlib import asynccontextmanager
from typing import Any
import httpx
logger = logging.getLogger(__name__)
GITHUB_API_BASE = "https://api.github.com"
GITHUB_GRAPHQL = "https://api.github.com/graphql"
GITHUB_HEADERS_VERSION = "2022-11-28"
DEFAULT_TIMEOUT = httpx.Timeout(30.0, connect=10.0, pool=5.0)
DEFAULT_MAX_RETRIES = 3
_ALWAYS_RETRYABLE_STATUS = frozenset({429, 503})
_IDEMPOTENT_RETRYABLE_STATUS = frozenset({502, 504})
_SECONDARY_RATE_LIMIT_MARKERS = ("secondary rate limit", "rate limit")
_RETRYABLE_TRANSPORT_METHODS = frozenset({"GET", "HEAD", "OPTIONS", "PUT", "DELETE"})
_BASE_BACKOFF = 1.0
_BACKOFF_MULTIPLIER = 2.0
_MAX_BACKOFF = 60.0
_JITTER_FACTOR = 0.25
def github_headers(token: str) -> dict[str, str]:
"""Standard GitHub API headers for a bearer token."""
return {
"Authorization": f"Bearer {token}",
"Accept": "application/vnd.github+json",
"X-GitHub-Api-Version": GITHUB_HEADERS_VERSION,
}
def _is_secondary_rate_limit(response: httpx.Response) -> bool:
if response.status_code != 403:
return False
if response.headers.get("X-RateLimit-Remaining") == "0":
return True
body = (response.text or "").lower()
return any(marker in body for marker in _SECONDARY_RATE_LIMIT_MARKERS)
def _is_retryable_response(response: httpx.Response, method: str) -> bool:
if response.status_code in _ALWAYS_RETRYABLE_STATUS:
return True
if response.status_code in _IDEMPOTENT_RETRYABLE_STATUS:
return method.upper() in _RETRYABLE_TRANSPORT_METHODS
return _is_secondary_rate_limit(response)
def _retry_after_seconds(response: httpx.Response) -> float | None:
retry_after = response.headers.get("Retry-After")
if retry_after:
try:
return float(retry_after)
except ValueError:
return None
return None
def _compute_backoff(response: httpx.Response | None, attempt: int) -> float:
if response is not None:
retry_after = _retry_after_seconds(response)
if retry_after is not None:
return min(retry_after, _MAX_BACKOFF)
base = _BASE_BACKOFF * (_BACKOFF_MULTIPLIER**attempt)
jitter = base * random.uniform(-_JITTER_FACTOR, _JITTER_FACTOR)
return min(base + jitter, _MAX_BACKOFF)
@asynccontextmanager
async def github_client(
*,
token: str | None = None,
timeout: httpx.Timeout | float | None = None,
headers: dict[str, str] | None = None,
) -> AsyncIterator[httpx.AsyncClient]:
"""Yield an ``httpx.AsyncClient`` with sane GitHub defaults.
The token (when provided) is baked into the default headers so callers
don't need to pass headers on every request. A custom ``timeout`` can
override the default 30 s / 10 s-connect timeout.
"""
merged_headers: dict[str, str] = {}
if token:
merged_headers.update(github_headers(token))
if headers:
merged_headers.update(headers)
async with httpx.AsyncClient(
headers=merged_headers or None,
timeout=timeout or DEFAULT_TIMEOUT,
) as client:
yield client
async def github_request(
client: httpx.AsyncClient,
method: str,
url: str,
*,
max_retries: int = DEFAULT_MAX_RETRIES,
**kwargs: Any,
) -> httpx.Response:
"""Execute a single GitHub API request with retries and rate-limit handling.
Returns the ``httpx.Response`` for non-retryable status codes and for
retryable status codes that have exhausted retries (caller should call
``raise_for_status()``).
Retry matrix:
| Condition | Idempotent (GET, PUT, DELETE…) | Non-idempotent (POST, PATCH) |
|-----------------------------------|--------------------------------|------------------------------|
| Transport error (timeout/reset) | Retry with backoff | Raise immediately |
| 429 / 503 / secondary rate limit | Retry with backoff | Retry with backoff |
| 502 / 504 (ambiguous gateway) | Retry with backoff | Raise immediately |
429 and 503 are safe to retry for any method: the server explicitly did
not process the request. 502/504 are ambiguous — the upstream may have
processed the write before the gateway returned an error — so they are
only retried for idempotent methods. Transport errors are only retried
for idempotent methods for the same reason.
"""
method_upper = method.upper()
retry_transport = method_upper in _RETRYABLE_TRANSPORT_METHODS
method_func = getattr(client, method.lower())
last_exc: Exception | None = None
for attempt in range(max_retries + 1):
try:
response = await method_func(url, **kwargs)
except (httpx.TimeoutException, httpx.TransportError) as exc:
last_exc = exc
if retry_transport and attempt < max_retries:
delay = _compute_backoff(None, attempt)
logger.warning(
"GitHub API %s %s raised %s, retrying in %.1fs (attempt %d/%d)",
method,
url,
type(exc).__name__,
delay,
attempt + 1,
max_retries,
)
await asyncio.sleep(delay)
continue
raise
if _is_retryable_response(response, method):
if attempt < max_retries:
delay = _compute_backoff(response, attempt)
logger.warning(
"GitHub API %s %s returned %d, retrying in %.1fs (attempt %d/%d)",
method,
url,
response.status_code,
delay,
attempt + 1,
max_retries,
)
await asyncio.sleep(delay)
continue
logger.warning(
"GitHub API %s %s returned %d after %d retries, giving up",
method,
url,
response.status_code,
max_retries,
)
return response
raise last_exc or httpx.HTTPError("Max retries exceeded")