From 19a69bbd880eca02491bed44c712da17428304cd Mon Sep 17 00:00:00 2001
From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com>
Date: Sat, 8 Aug 2026 11:27:40 +0200
Subject: [PATCH] fix: treat only quota signals as fatal

---
 scripts/upstream.py    | 25 +++++++++++++++++++------
 tests/test_upstream.py | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/scripts/upstream.py b/scripts/upstream.py
index a4a4a9a5..b66f6193 100644
--- a/scripts/upstream.py
+++ b/scripts/upstream.py
@@ -139,6 +139,23 @@ def _headers(url: str, accept_json: bool = False) -> dict[str, str]:
     return headers
 
 
+def _http_failure(url: str, exc: urllib.error.HTTPError) -> UpstreamError:
+    """Classify an HTTP error. Only a real quota signal is fatal.
+
+    A forge answering 403 is usually refusing the request, not reporting a
+    quota: small Forgejo instances behind anti-bot filters do it routinely.
+    GitHub reports exhaustion with X-RateLimit-Remaining: 0, and 429 is the
+    standard quota status everywhere, so those two are the only fatal cases.
+    """
+    if exc.code == 429:
+        return RateLimitError(f"{url}: HTTP 429")
+    if exc.code == 403 and exc.headers is not None:
+        remaining = exc.headers.get("X-RateLimit-Remaining")
+        if remaining is not None and remaining.strip() == "0":
+            return RateLimitError(f"{url}: HTTP 403, quota exhausted")
+    return UpstreamError(f"{url}: HTTP {exc.code}")
+
+
 def _http_text(url: str) -> str | None:
     """Body of a GET, or None on 404. Replaced in tests."""
     req = urllib.request.Request(url, headers=_headers(url))
@@ -148,9 +165,7 @@ def _http_text(url: str) -> str | None:
     except urllib.error.HTTPError as exc:
         if exc.code == 404:
             return None
-        if exc.code in (403, 429):
-            raise RateLimitError(f"{url}: HTTP {exc.code}") from exc
-        raise UpstreamError(f"{url}: HTTP {exc.code}") from exc
+        raise _http_failure(url, exc) from exc
     except urllib.error.URLError as exc:
         raise UpstreamError(f"{url}: {exc.reason}") from exc
 
@@ -164,9 +179,7 @@ def _http_json(url: str) -> object | None:
     except urllib.error.HTTPError as exc:
         if exc.code == 404:
             return None
-        if exc.code in (403, 429):
-            raise RateLimitError(f"{url}: HTTP {exc.code}") from exc
-        raise UpstreamError(f"{url}: HTTP {exc.code}") from exc
+        raise _http_failure(url, exc) from exc
     except urllib.error.URLError as exc:
         raise UpstreamError(f"{url}: {exc.reason}") from exc
 
diff --git a/tests/test_upstream.py b/tests/test_upstream.py
index d33fa3b2..693f28b3 100644
--- a/tests/test_upstream.py
+++ b/tests/test_upstream.py
@@ -6,6 +6,7 @@ import os
 import sys
 import tempfile
 import unittest
+import urllib.error
 from pathlib import Path
 
 sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "scripts"))
@@ -137,6 +138,43 @@ class TestTokenScope(unittest.TestCase):
         self.assertNotIn("Authorization", h)
 
 
+class TestHttpFailure(unittest.TestCase):
+    """Only an actual quota signal may abort a whole run."""
+
+    @staticmethod
+    def _error(code, headers=None):
+        return urllib.error.HTTPError(
+            "https://host/x", code, "msg", headers, None
+        )
+
+    def test_429_is_a_rate_limit(self):
+        self.assertIsInstance(
+            upstream._http_failure("u", self._error(429)), upstream.RateLimitError
+        )
+
+    def test_403_with_exhausted_quota_is_a_rate_limit(self):
+        exc = self._error(403, {"X-RateLimit-Remaining": "0"})
+        self.assertIsInstance(
+            upstream._http_failure("u", exc), upstream.RateLimitError
+        )
+
+    def test_403_with_quota_left_is_not(self):
+        exc = self._error(403, {"X-RateLimit-Remaining": "4970"})
+        failure = upstream._http_failure("u", exc)
+        self.assertIsInstance(failure, upstream.UpstreamError)
+        self.assertNotIsInstance(failure, upstream.RateLimitError)
+
+    def test_bare_403_from_a_forge_is_not_a_rate_limit(self):
+        failure = upstream._http_failure("u", self._error(403))
+        self.assertIsInstance(failure, upstream.UpstreamError)
+        self.assertNotIsInstance(failure, upstream.RateLimitError)
+
+    def test_525_is_a_plain_upstream_error(self):
+        failure = upstream._http_failure("u", self._error(525))
+        self.assertIsInstance(failure, upstream.UpstreamError)
+        self.assertNotIsInstance(failure, upstream.RateLimitError)
+
+
 class TestCache(unittest.TestCase):
     def setUp(self):
         self.tmp = tempfile.TemporaryDirectory()