Merge pull request 'v0.3.32: Honor Retry-After on HTTP 429 with jittered exponential backoff' (#28) from fix/v0.3.32-retry-after-backoff into master

Reviewed-on: #28
2026-07-01 21:04:34 -05:00
parent 9300cc9e1e 1b3c961ff2
commit 20dd717b9c
2 changed files with 311 additions and 9 deletions
@@ -7,10 +7,14 @@ antivirus alerts (e.g. Bitdefender).
 import os
 import glob
 import json
+import time
 import base64
+import random
+import datetime
 import subprocess
 import urllib.request
 import urllib.error
+from email.utils import parsedate_to_datetime
 from dotenv import dotenv_values, load_dotenv

 # Load standard .env if present
@@ -148,23 +152,123 @@ def add_remote_args(parser):
    parser.add_argument("--repo", help="Override the repository.")


-def api_request(method, url, auth_header, payload=None):
+def _env_int(name, default):
+    """Read a non-negative int from the environment, falling back to *default*."""
+    try:
+        value = int(os.environ[name])
+    except (KeyError, ValueError, TypeError):
+        return default
+    return value if value >= 0 else default
+
+
+def _env_float(name, default):
+    """Read a non-negative float from the environment, falling back to *default*."""
+    try:
+        value = float(os.environ[name])
+    except (KeyError, ValueError, TypeError):
+        return default
+    return value if value >= 0 else default
+
+
+# Retry/backoff configuration for HTTP 429 (rate-limit) responses.
+# Overridable via environment; safe defaults otherwise.
+DEFAULT_MAX_RETRIES = _env_int("GITEA_MAX_RETRIES", 3)
+DEFAULT_BASE_DELAY = _env_float("GITEA_RETRY_BASE_DELAY", 1.0)   # seconds
+DEFAULT_MAX_DELAY = _env_float("GITEA_RETRY_MAX_DELAY", 60.0)    # seconds
+
+
+def parse_retry_after(value, now=None):
+    """Parse a ``Retry-After`` header into a non-negative delay in seconds.
+
+    Supports both forms defined by RFC 7231:
+    - a non-negative integer number of seconds (e.g. ``"120"``)
+    - an HTTP-date (e.g. ``"Wed, 21 Oct 2015 07:28:00 GMT"``)
+
+    Returns ``None`` when *value* is missing, blank, or unparseable, so the
+    caller can fall back to computed backoff. Past dates clamp to ``0``.
+    """
+    if value is None:
+        return None
+    value = value.strip()
+    if not value:
+        return None
+
+    # Seconds form (integer). Reject non-integer numerics like "1.5".
+    try:
+        seconds = int(value)
+        return max(0, seconds)
+    except ValueError:
+        pass
+
+    # HTTP-date form.
+    try:
+        when = parsedate_to_datetime(value)
+    except (TypeError, ValueError):
+        return None
+    if when is None:
+        return None
+    if when.tzinfo is None:
+        # RFC dates without a zone are UTC.
+        when = when.replace(tzinfo=datetime.timezone.utc)
+
+    now_ts = now if now is not None else time.time()
+    return max(0.0, when.timestamp() - now_ts)
+
+
+def backoff_delay(attempt, base=DEFAULT_BASE_DELAY, cap=DEFAULT_MAX_DELAY, rand=random.random):
+    """Full-jitter exponential backoff delay in seconds for a 0-indexed *attempt*.
+
+    Returns a random value in ``[0, min(cap, base * 2**attempt)]``. Full jitter
+    spreads retries across the whole window to avoid a thundering herd.
+    """
+    ceiling = min(cap, base * (2 ** attempt))
+    return rand() * ceiling
+
+
+def api_request(method, url, auth_header, payload=None, *,
+                max_retries=None, base_delay=None, max_delay=None,
+                sleep_func=time.sleep, rand_func=random.random,
+                now_func=time.time):
    """Make an authenticated JSON request to the Gitea API.

-    Returns parsed JSON on success, raises on HTTP errors.
+    Returns parsed JSON on success, raises ``RuntimeError`` on HTTP errors.
+
+    On HTTP 429 the request is retried up to *max_retries* times: honoring a
+    valid ``Retry-After`` header (seconds or HTTP-date) when present, otherwise
+    using capped jittered exponential backoff. Non-429 errors and successful
+    responses are unchanged. The ``*_func`` parameters are injection points for
+    deterministic testing.
    """
+    if max_retries is None:
+        max_retries = DEFAULT_MAX_RETRIES
+    if base_delay is None:
+        base_delay = DEFAULT_BASE_DELAY
+    if max_delay is None:
+        max_delay = DEFAULT_MAX_DELAY
+
    data = json.dumps(payload).encode("utf-8") if payload is not None else None
    req = urllib.request.Request(url, data=data, method=method)
    req.add_header("Authorization", auth_header)
    req.add_header("Content-Type", "application/json")
    req.add_header("User-Agent", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36")
-    try:
-        with urllib.request.urlopen(req) as resp:
-            body = resp.read().decode("utf-8")
-            return json.loads(body) if body else None
-    except urllib.error.HTTPError as e:
-        error_body = e.read().decode("utf-8", errors="replace")
-        raise RuntimeError(f"HTTP {e.code}: {error_body}") from e
+
+    attempt = 0
+    while True:
+        try:
+            with urllib.request.urlopen(req) as resp:
+                body = resp.read().decode("utf-8")
+                return json.loads(body) if body else None
+        except urllib.error.HTTPError as e:
+            if e.code == 429 and attempt < max_retries:
+                header = e.headers.get("Retry-After") if e.headers else None
+                delay = parse_retry_after(header, now=now_func())
+                if delay is None:
+                    delay = backoff_delay(attempt, base_delay, max_delay, rand_func)
+                attempt += 1
+                sleep_func(delay)
+                continue
+            error_body = e.read().decode("utf-8", errors="replace")
+            raise RuntimeError(f"HTTP {e.code}: {error_body}") from e


 def repo_api_url(host, org, repo):
@@ -0,0 +1,198 @@
+"""Tests for HTTP 429 Retry-After handling and jittered backoff in gitea_auth.
+
+All timing is made deterministic by injecting ``sleep_func``, ``rand_func`` and
+``now_func`` — no real sleeping or randomness, and no real network calls.
+"""
+import io
+import sys
+import email.message
+import unittest
+import urllib.error
+from email.utils import formatdate
+from unittest.mock import MagicMock, patch
+
+sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
+import gitea_auth  # noqa: E402
+
+
+FIXED_NOW = 1_000_000.0
+
+
+def _http_error(code, retry_after=None, body=b"error"):
+    """Build a urllib HTTPError with an optional Retry-After header."""
+    hdrs = email.message.Message()
+    if retry_after is not None:
+        hdrs["Retry-After"] = retry_after
+    return urllib.error.HTTPError("http://example.test", code, "msg", hdrs,
+                                  io.BytesIO(body))
+
+
+class _FakeResponse:
+    """Minimal context-manager stand-in for a urlopen() success."""
+
+    def __init__(self, body=b""):
+        self._body = body
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, *exc):
+        return False
+
+    def read(self):
+        return self._body
+
+
+def _call(side_effect, **kwargs):
+    """Run api_request with urlopen patched to the given side_effect list."""
+    defaults = dict(
+        sleep_func=MagicMock(),
+        rand_func=lambda: 0.5,
+        now_func=lambda: FIXED_NOW,
+    )
+    defaults.update(kwargs)
+    with patch("gitea_auth.urllib.request.urlopen", side_effect=side_effect):
+        result = gitea_auth.api_request("GET", "http://example.test", "token x",
+                                        **defaults)
+    return result, defaults["sleep_func"]
+
+
+class TestParseRetryAfter(unittest.TestCase):
+
+    def test_seconds_form(self):
+        self.assertEqual(gitea_auth.parse_retry_after("120"), 120)
+
+    def test_zero_seconds(self):
+        self.assertEqual(gitea_auth.parse_retry_after("0"), 0)
+
+    def test_negative_seconds_clamped(self):
+        self.assertEqual(gitea_auth.parse_retry_after("-5"), 0)
+
+    def test_http_date_future(self):
+        header = formatdate(FIXED_NOW + 30, usegmt=True)
+        delay = gitea_auth.parse_retry_after(header, now=FIXED_NOW)
+        self.assertAlmostEqual(delay, 30.0, delta=1.0)
+
+    def test_http_date_past_clamped_to_zero(self):
+        header = formatdate(FIXED_NOW - 30, usegmt=True)
+        delay = gitea_auth.parse_retry_after(header, now=FIXED_NOW)
+        self.assertEqual(delay, 0.0)
+
+    def test_none_returns_none(self):
+        self.assertIsNone(gitea_auth.parse_retry_after(None))
+
+    def test_blank_returns_none(self):
+        self.assertIsNone(gitea_auth.parse_retry_after("   "))
+
+    def test_garbage_returns_none(self):
+        self.assertIsNone(gitea_auth.parse_retry_after("soon"))
+
+    def test_float_string_returns_none(self):
+        # Non-integer numerics are not valid seconds and are not HTTP-dates.
+        self.assertIsNone(gitea_auth.parse_retry_after("1.5"))
+
+
+class TestBackoffDelay(unittest.TestCase):
+
+    def test_zero_jitter_gives_zero(self):
+        self.assertEqual(gitea_auth.backoff_delay(0, base=1.0, cap=60.0, rand=lambda: 0.0), 0.0)
+
+    def test_full_jitter_gives_ceiling(self):
+        # attempt=3 -> base*2**3 = 8, below cap
+        self.assertEqual(gitea_auth.backoff_delay(3, base=1.0, cap=60.0, rand=lambda: 1.0), 8.0)
+
+    def test_delay_capped(self):
+        # base*2**10 = 1024, capped at 60
+        self.assertEqual(gitea_auth.backoff_delay(10, base=1.0, cap=60.0, rand=lambda: 1.0), 60.0)
+
+    def test_delay_within_bounds(self):
+        for attempt in range(6):
+            for r in (0.0, 0.25, 0.5, 0.75, 1.0):
+                delay = gitea_auth.backoff_delay(attempt, base=1.0, cap=30.0, rand=lambda: r)
+                ceiling = min(30.0, 1.0 * (2 ** attempt))
+                self.assertGreaterEqual(delay, 0.0)
+                self.assertLessEqual(delay, ceiling)
+
+
+class TestApiRequestRetry(unittest.TestCase):
+
+    def test_success_no_retry(self):
+        result, sleep = _call([_FakeResponse(b'{"ok": true}')])
+        self.assertEqual(result, {"ok": True})
+        sleep.assert_not_called()
+
+    def test_non_429_error_raises_immediately(self):
+        with self.assertRaises(RuntimeError) as ctx:
+            _call([_http_error(500, body=b"boom")])
+        self.assertIn("HTTP 500", str(ctx.exception))
+
+    def test_non_429_error_does_not_sleep(self):
+        sleep = MagicMock()
+        with self.assertRaises(RuntimeError):
+            _call([_http_error(404)], sleep_func=sleep)
+        sleep.assert_not_called()
+
+    def test_honors_retry_after_seconds(self):
+        result, sleep = _call([
+            _http_error(429, retry_after="5"),
+            _FakeResponse(b'{"ok": true}'),
+        ])
+        self.assertEqual(result, {"ok": True})
+        sleep.assert_called_once_with(5)
+
+    def test_honors_retry_after_http_date(self):
+        header = formatdate(FIXED_NOW + 42, usegmt=True)
+        result, sleep = _call([
+            _http_error(429, retry_after=header),
+            _FakeResponse(b'{"ok": true}'),
+        ])
+        self.assertEqual(result, {"ok": True})
+        self.assertEqual(sleep.call_count, 1)
+        self.assertAlmostEqual(sleep.call_args[0][0], 42.0, delta=1.0)
+
+    def test_invalid_retry_after_falls_back_to_backoff(self):
+        # rand=1.0, base=2, attempt 0 -> ceiling min(100, 2*1) = 2.0
+        result, sleep = _call(
+            [_http_error(429, retry_after="whenever"), _FakeResponse(b"{}")],
+            rand_func=lambda: 1.0, base_delay=2.0, max_delay=100.0,
+        )
+        self.assertEqual(result, {})
+        sleep.assert_called_once_with(2.0)
+
+    def test_missing_retry_after_falls_back_to_backoff(self):
+        result, sleep = _call(
+            [_http_error(429), _FakeResponse(b"{}")],
+            rand_func=lambda: 1.0, base_delay=1.0, max_delay=100.0,
+        )
+        self.assertEqual(result, {})
+        sleep.assert_called_once_with(1.0)
+
+    def test_gives_up_after_max_retries(self):
+        # max_retries=3 -> 3 sleeps, then the 4th failure raises.
+        errors = [_http_error(429, retry_after="1") for _ in range(4)]
+        sleep = MagicMock()
+        with self.assertRaises(RuntimeError) as ctx:
+            _call(errors, sleep_func=sleep, max_retries=3)
+        self.assertIn("HTTP 429", str(ctx.exception))
+        self.assertEqual(sleep.call_count, 3)
+
+    def test_no_infinite_loop_when_always_429(self):
+        # Far more failures than retries available; must still terminate.
+        errors = [_http_error(429) for _ in range(20)]
+        sleep = MagicMock()
+        with self.assertRaises(RuntimeError):
+            _call(errors, sleep_func=sleep, max_retries=2)
+        self.assertEqual(sleep.call_count, 2)
+
+    def test_recovers_after_several_429s(self):
+        result, sleep = _call([
+            _http_error(429, retry_after="1"),
+            _http_error(429, retry_after="1"),
+            _FakeResponse(b'{"done": 1}'),
+        ], max_retries=5)
+        self.assertEqual(result, {"done": 1})
+        self.assertEqual(sleep.call_count, 2)
+
+
+if __name__ == "__main__":
+    unittest.main()