Skip to content

Commit 4da91fc

Browse files
committed
fix(verify): treat a rate-limit answer as transient, not as a dead link
check-urls recorded HTTP 429 as a liveness verdict and cached it for the full 30-day TTL. 3,998 of the 4,042 cited GSMArena URLs are currently parked as dead that way — yet every one of them answers 200 when asked at a civil pace. Those records can never be promoted, not because their sources are bad but because we asked too fast once. Three changes: * 429/503 is retried up to 3 times, honouring Retry-After when the host sends one. * A host that pushes back has its per-host interval multiplied for the rest of the run, so one rate-limited host stops cascading into 429s for every remaining URL on it. * A still-rate-limited URL is not written to the cache, and existing cached 429s are ignored on load. A 429 is not an answer, so it must not occupy an answer's slot. Refs #1
1 parent 42decca commit 4da91fc

3 files changed

Lines changed: 150 additions & 6 deletions

File tree

app/verify/cli.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,9 +409,18 @@ def cmd_check_urls(args: argparse.Namespace) -> int:
409409
max_workers=args.workers,
410410
min_interval=args.min_interval,
411411
)
412+
# A rate-limited answer is not a verdict — leave it out so the next run asks
413+
# again instead of parking the URL as dead for the whole TTL.
414+
throttled = sum(1 for r in results if r.transient)
412415
for r in results:
413-
cache[r.url] = http_check.result_to_entry(r, ts)
416+
if not r.transient:
417+
cache[r.url] = http_check.result_to_entry(r, ts)
414418
http_check.save_cache(cache)
419+
if throttled:
420+
print(
421+
f"note: {throttled} URL(s) rate-limited after retries; "
422+
"not cached, will retry next run"
423+
)
415424
print(f"cache: wrote {len(cache)} URL result(s) to data/_verify/state/url_cache.jsonl")
416425
_summarize_cache(cache, targets)
417426
return 0

app/verify/http_check.py

Lines changed: 76 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,28 @@
3232
)
3333

3434

35+
# "Come back later" is not "this link is dead". A host that rate-limits us says
36+
# nothing about whether the page exists, so these answers must never be cached as
37+
# a verdict — otherwise one impatient run marks a whole host dead for a TTL.
38+
TRANSIENT_STATUSES = frozenset({429, 503})
39+
RETRY_ATTEMPTS = 3
40+
RETRY_BACKOFF_S = (2.0, 6.0)
41+
MAX_RETRY_AFTER_S = 15.0
42+
# How much to slow a host down for the rest of the run once it has pushed back.
43+
RATE_LIMIT_PENALTY = 4.0
44+
45+
3546
class CheckResult(NamedTuple):
3647
url: str
3748
status: int | None
3849
final_url: str | None
3950
alive: bool
4051
reason: str
4152

53+
@property
54+
def transient(self) -> bool:
55+
return self.status in TRANSIENT_STATUSES
56+
4257

4358
# --- opener abstraction (injectable for tests) -----------------------------------
4459

@@ -93,10 +108,21 @@ def classify(original_url: str, status: int | None, final_url: str | None) -> tu
93108
return True, f"http-{status}"
94109

95110

96-
def check_one(url: str, opener: Any) -> CheckResult:
97-
"""HEAD first; fall back to GET when HEAD is rejected (405/403) or errors."""
111+
def _retry_after_seconds(exc: Exception) -> float | None:
112+
"""Seconds requested by a ``Retry-After`` header, clamped to something sane."""
113+
headers = getattr(exc, "headers", None)
114+
raw = headers.get("Retry-After") if headers is not None else None
115+
try:
116+
return min(float(raw), MAX_RETRY_AFTER_S) if raw is not None else None
117+
except (TypeError, ValueError):
118+
return None # HTTP-date form; fall back to our own backoff
119+
120+
121+
def _attempt(url: str, opener: Any) -> tuple[int | None, str | None, float | None]:
122+
"""One HEAD-then-GET pass. Returns (status, final_url, retry_after)."""
98123
status: int | None = None
99124
final: str | None = None
125+
retry_after: float | None = None
100126
for method in ("HEAD", "GET"):
101127
try:
102128
status, final = opener.open(url, method)
@@ -107,10 +133,31 @@ def check_one(url: str, opener: Any) -> CheckResult:
107133
code = getattr(exc, "code", None)
108134
if isinstance(code, int):
109135
status, final = code, getattr(exc, "url", None) or url
136+
retry_after = _retry_after_seconds(exc)
110137
if method == "HEAD" and code in (400, 403, 405, 501):
111138
continue
112139
break
113140
status, final = None, None
141+
return status, final, retry_after
142+
143+
144+
def check_one(
145+
url: str, opener: Any, *, on_rate_limit: Callable[[str], None] | None = None
146+
) -> CheckResult:
147+
"""HEAD first; fall back to GET when HEAD is rejected (405/403) or errors.
148+
149+
A rate-limit answer (429/503) is retried with backoff — the host is telling us
150+
to wait, not that the page is gone.
151+
"""
152+
status = final = retry_after = None
153+
for attempt in range(RETRY_ATTEMPTS):
154+
status, final, retry_after = _attempt(url, opener)
155+
if status not in TRANSIENT_STATUSES:
156+
break
157+
if on_rate_limit is not None:
158+
on_rate_limit(host_of(url))
159+
if attempt < RETRY_ATTEMPTS - 1:
160+
time.sleep(retry_after if retry_after is not None else RETRY_BACKOFF_S[attempt])
114161
alive, reason = classify(url, status, final)
115162
return CheckResult(url, status, final, alive, reason)
116163

@@ -124,13 +171,26 @@ class HostRateLimiter:
124171
def __init__(self, min_interval: float = 1.0) -> None:
125172
self.min_interval = min_interval
126173
self._last: dict[str, float] = {}
174+
self._interval: dict[str, float] = {}
127175
self._lock = threading.Lock()
128176

177+
def interval_for(self, host: str) -> float:
178+
return self._interval.get(host, self.min_interval)
179+
180+
def back_off(self, host: str, factor: float = RATE_LIMIT_PENALTY) -> None:
181+
"""A host pushed back — slow it down for the rest of the run.
182+
183+
Without this, one rate-limited host keeps being hammered at the global
184+
pace and every subsequent URL on it comes back 429.
185+
"""
186+
with self._lock:
187+
self._interval[host] = self.interval_for(host) * factor
188+
129189
def wait(self, host: str) -> None:
130190
with self._lock:
131191
now = time.time()
132192
prev = self._last.get(host, 0.0)
133-
sleep_for = max(0.0, self.min_interval - (now - prev))
193+
sleep_for = max(0.0, self.interval_for(host) - (now - prev))
134194
self._last[host] = now + sleep_for
135195
if sleep_for > 0:
136196
time.sleep(sleep_for)
@@ -172,7 +232,7 @@ def _get_opener() -> Any:
172232

173233
def _task(url: str) -> CheckResult:
174234
limiter.wait(host_of(url))
175-
return check_one(url, _get_opener())
235+
return check_one(url, _get_opener(), on_rate_limit=limiter.back_off)
176236

177237
if not urls:
178238
return []
@@ -184,7 +244,18 @@ def _task(url: str) -> CheckResult:
184244

185245

186246
def load_cache(path: Path = URL_CACHE_PATH) -> dict[str, dict[str, Any]]:
187-
return {e["url"]: e for e in ledger.iter_entries(path) if isinstance(e.get("url"), str)}
247+
"""Load the cache, dropping rate-limit answers written by older runs.
248+
249+
A 429/503 is not a verdict, so an entry holding one is not a cache hit —
250+
it is a URL we still have to check. Filtering on load heals a cache that a
251+
previous run poisoned (3,998 GSMArena pages were parked as dead this way,
252+
all of which answer 200 when asked at a civil pace).
253+
"""
254+
return {
255+
e["url"]: e
256+
for e in ledger.iter_entries(path)
257+
if isinstance(e.get("url"), str) and e.get("status") not in TRANSIENT_STATUSES
258+
}
188259

189260

190261
def _parse_ts(ts: str) -> datetime | None:

tests/verify/test_http_check.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,67 @@ def test_cache_roundtrip():
103103
assert loaded["https://x.com/y"]["alive"] is True
104104
finally:
105105
path.unlink(missing_ok=True)
106+
107+
108+
class _Http429(Exception):
109+
"""urllib-shaped 429, optionally carrying a Retry-After header."""
110+
111+
def __init__(self, url, retry_after=None):
112+
super().__init__("Too Many Requests")
113+
self.code = 429
114+
self.url = url
115+
self.headers = {"Retry-After": retry_after} if retry_after else {}
116+
117+
118+
class FlakyOpener(FakeOpener):
119+
"""Rate-limits the first ``fail_times`` calls, then answers normally."""
120+
121+
def __init__(self, table, fail_times):
122+
super().__init__(table)
123+
self.remaining = fail_times
124+
125+
def open(self, url, method):
126+
self.calls.append((url, method))
127+
if self.remaining > 0:
128+
self.remaining -= 1
129+
raise _Http429(url, retry_after="0")
130+
return self.table[url]
131+
132+
133+
def test_rate_limit_is_retried_then_succeeds(monkeypatch):
134+
monkeypatch.setattr(http_check.time, "sleep", lambda _s: None)
135+
url = "https://www.gsmarena.com/x-1.php"
136+
op = FlakyOpener({url: (200, url)}, fail_times=2)
137+
[res] = http_check.check_urls([url], opener_factory=lambda: op, min_interval=0)
138+
assert res.alive and res.status == 200 and not res.transient
139+
140+
141+
def test_persistent_rate_limit_is_transient_not_dead(monkeypatch):
142+
monkeypatch.setattr(http_check.time, "sleep", lambda _s: None)
143+
url = "https://www.gsmarena.com/y-2.php"
144+
op = FlakyOpener({url: (200, url)}, fail_times=99)
145+
[res] = http_check.check_urls([url], opener_factory=lambda: op, min_interval=0)
146+
assert res.status == 429 and res.transient # caller must not cache this as a verdict
147+
148+
149+
def test_rate_limit_slows_the_host_down(monkeypatch):
150+
monkeypatch.setattr(http_check.time, "sleep", lambda _s: None)
151+
url = "https://www.gsmarena.com/z-3.php"
152+
limiter = http_check.HostRateLimiter(min_interval=1.0)
153+
op = FlakyOpener({url: (200, url)}, fail_times=1)
154+
http_check.check_urls([url], opener_factory=lambda: op, limiter=limiter)
155+
assert limiter.interval_for("gsmarena.com") > 1.0
156+
157+
158+
def test_cached_rate_limit_entries_are_not_cache_hits(tmp_path):
159+
path = tmp_path / "url_cache.jsonl"
160+
path.write_text(
161+
'{"url": "https://www.gsmarena.com/a-1.php", "status": 429, "alive": false,'
162+
' "reason": "http-429", "checked_at": "2026-08-03T00:00:00Z"}\n'
163+
'{"url": "https://en.wikipedia.org/wiki/X", "status": 200, "alive": true,'
164+
' "reason": "http-200", "checked_at": "2026-08-03T00:00:00Z"}\n',
165+
encoding="utf-8",
166+
)
167+
cache = http_check.load_cache(path)
168+
assert "https://en.wikipedia.org/wiki/X" in cache
169+
assert "https://www.gsmarena.com/a-1.php" not in cache # a 429 is not an answer

0 commit comments

Comments
 (0)