ci: retry transient GH-API and NJSP-feed failures

Two daily-run failures this week:
- 6/26: NJSP feed returned `403 Forbidden` for `FAUQStats2024.xml` (transient
  upstream blip; 6/27 refresh against same URL succeeded).
- 6/27: GH API `429 "This endpoint is temporarily being throttled"` while
  walking parent commits in `crash-log.parquet` compute. With shallow CI
  clone (`actions/checkout` default `fetch-depth: 1`), `get_crash_log` falls
  through to `GithubCommit.parent` → `gh.get_commit` per step, which trips
  GitHub's secondary rate limit (PyGithub auto-retries primary, not
  secondary).

`nj_crashes/utils/retry`:
- `http_get_with_retry`: capped exp-backoff on connection errors and on
  `{403, 408, 429, 500, 502, 503, 504}`. Honors `Retry-After` (seconds or
  HTTP-date). 404 passes through (caller's current-year carve-out kept).
- `with_gh_retry`: decorator that retries `GithubException.status` in
  `{429, 502, 503, 504}` and honors the exception's `Retry-After` header.

Wire-in:
- `njsp/cli/refresh_data.update_years`: route the FAUQStats GET through
  `http_get_with_retry`. HEAD stays bare (already non-fatal).
- `nj_crashes/utils/github`: route all `gh.get_commit` / `gh.get_git_tree`
  / `gh.get_git_blob` / `repo.get_contents` calls through `with_gh_retry`-
  wrapped module-level helpers.

13 unit tests cover both helpers (status filtering, Retry-After honoring,
attempt exhaustion, connection-error path).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

Author: ryan-williams

nj_crashes/tests/test_retry.py

@@ -0,0 +1,151 @@
+from unittest.mock import MagicMock, patch
+
+import pytest
+import requests
+from github import GithubException
+
+from nj_crashes.utils.retry import (
+    _parse_retry_after,
+    http_get_with_retry,
+    with_gh_retry,
+)
+
+
+def test_parse_retry_after_seconds():
+    assert _parse_retry_after("5") == 5.0
+    assert _parse_retry_after("0") == 0.0
+
+
+def test_parse_retry_after_none_or_bad():
+    assert _parse_retry_after(None) is None
+    assert _parse_retry_after("") is None
+    assert _parse_retry_after("garbage") is None
+
+
+def test_parse_retry_after_http_date_clamped_nonnegative():
+    # Past dates should clamp to 0 (already past); future dates positive.
+    past = _parse_retry_after("Wed, 01 Jan 2020 00:00:00 GMT")
+    assert past == 0.0
+    future = _parse_retry_after("Wed, 01 Jan 2099 00:00:00 GMT")
+    assert future is not None and future > 0
+
+
+def _resp(status_code: int, headers: dict | None = None, content: bytes = b"ok"):
+    r = MagicMock(spec=requests.Response)
+    r.status_code = status_code
+    r.reason = "Reason"
+    r.headers = headers or {}
+    r.content = content
+    return r
+
+
+def test_http_get_returns_immediately_on_200():
+    with patch("nj_crashes.utils.retry.requests.get") as g, \
+         patch("nj_crashes.utils.retry.time.sleep") as sleep:
+        g.return_value = _resp(200)
+        res = http_get_with_retry("http://x", max_attempts=3)
+    assert res.status_code == 200
+    assert g.call_count == 1
+    sleep.assert_not_called()
+
+
+def test_http_get_retries_on_403_then_succeeds():
+    # NJSP 6/26 failure: transient 403, recovers on retry.
+    with patch("nj_crashes.utils.retry.requests.get") as g, \
+         patch("nj_crashes.utils.retry.time.sleep") as sleep:
+        g.side_effect = [_resp(403), _resp(200)]
+        res = http_get_with_retry("http://x", max_attempts=3, base=0.01, cap=0.01)
+    assert res.status_code == 200
+    assert g.call_count == 2
+    assert sleep.call_count == 1
+
+
+def test_http_get_honors_retry_after():
+    with patch("nj_crashes.utils.retry.requests.get") as g, \
+         patch("nj_crashes.utils.retry.time.sleep") as sleep:
+        g.side_effect = [_resp(429, headers={"Retry-After": "7"}), _resp(200)]
+        http_get_with_retry("http://x", max_attempts=3, base=0.01)
+    sleep.assert_called_once_with(7.0)
+
+
+def test_http_get_exhausts_attempts_and_returns_last_response():
+    with patch("nj_crashes.utils.retry.requests.get") as g, \
+         patch("nj_crashes.utils.retry.time.sleep"):
+        g.return_value = _resp(503)
+        res = http_get_with_retry("http://x", max_attempts=3, base=0.01)
+    assert res.status_code == 503
+    assert g.call_count == 3
+
+
+def test_http_get_passes_404_through_without_retry():
+    # 404 not in default retry_statuses — should return on first attempt.
+    with patch("nj_crashes.utils.retry.requests.get") as g, \
+         patch("nj_crashes.utils.retry.time.sleep") as sleep:
+        g.return_value = _resp(404)
+        res = http_get_with_retry("http://x", max_attempts=3)
+    assert res.status_code == 404
+    assert g.call_count == 1
+    sleep.assert_not_called()
+
+
+def test_http_get_retries_on_connection_error():
+    with patch("nj_crashes.utils.retry.requests.get") as g, \
+         patch("nj_crashes.utils.retry.time.sleep") as sleep:
+        g.side_effect = [requests.ConnectionError("nope"), _resp(200)]
+        res = http_get_with_retry("http://x", max_attempts=3, base=0.01)
+    assert res.status_code == 200
+    assert sleep.call_count == 1
+
+
+def test_gh_retry_returns_immediately_on_success():
+    calls = []
+
+    @with_gh_retry(max_attempts=3, base=0.01, cap=0.01)
+    def fn():
+        calls.append(1)
+        return "ok"
+
+    assert fn() == "ok"
+    assert calls == [1]
+
+
+def test_gh_retry_429_then_success():
+    seq = [
+        GithubException(429, {"message": "throttled"}, {"Retry-After": "0"}),
+        "ok",
+    ]
+    calls = []
+
+    @with_gh_retry(max_attempts=3, base=0.01, cap=0.01)
+    def fn():
+        calls.append(1)
+        v = seq.pop(0)
+        if isinstance(v, Exception):
+            raise v
+        return v
+
+    with patch("nj_crashes.utils.retry.time.sleep") as sleep:
+        assert fn() == "ok"
+    assert len(calls) == 2
+    sleep.assert_called_once_with(0.0)
+
+
+def test_gh_retry_non_retryable_status_raises_immediately():
+    @with_gh_retry(max_attempts=3, base=0.01, cap=0.01)
+    def fn():
+        raise GithubException(404, {"message": "not found"}, {})
+
+    with pytest.raises(GithubException) as ei:
+        fn()
+    assert ei.value.status == 404
+
+
+def test_gh_retry_exhausts_attempts_and_raises_last():
+    @with_gh_retry(max_attempts=3, base=0.01, cap=0.01)
+    def fn():
+        raise GithubException(429, {"message": "throttled"}, {})
+
+    with patch("nj_crashes.utils.retry.time.sleep"):
+        with pytest.raises(GithubException) as ei:
+            fn()
+    assert ei.value.status == 429

nj_crashes/utils/github.py

@@ -18,22 +18,42 @@
 from utz import proc
 
 from nj_crashes.utils.git import git_fmt
+from nj_crashes.utils.retry import with_gh_retry
 from njdot.rawdata.utils import singleton
 
 REPO = 'hudcostreets/nj-crashes'
 _gh: Github | None = None
 _gh_repo: Repository | None = None
 
 
+@with_gh_retry()
+def _gh_get_commit(sha: str) -> Commit:
+    return get_github_repo().get_commit(sha)
+
+
+@with_gh_retry()
+def _gh_get_git_tree(sha: str) -> GitTree:
+    return get_github_repo().get_git_tree(sha)
+
+
+@with_gh_retry()
+def _gh_get_git_blob(hexsha: str):
+    return get_github_repo().get_git_blob(hexsha)
+
+
+@with_gh_retry()
+def _gh_get_contents(path: str, ref: str | None = None):
+    return get_github_repo().get_contents(path, ref=ref)
+
+
 def expand_ref(ref: str) -> str:
     if fullmatch(r'^[0-9a-f]{40}$', ref):
         return ref
     else:
         try:
             return git_fmt(ref, fmt='%H')
         except CalledProcessError:
-            gh = get_github_repo()
-            return gh.get_commit(ref).sha
+            return _gh_get_commit(ref).sha
 
 
 def expand_refspec(refspec: str, *args: str) -> list[str]:
@@ -76,9 +96,9 @@ def load_github(
     ref: str | None = None,
     repo: Repository | None = None,
 ) -> bytes:
-    if repo is None:
-        repo = get_github_repo()
-    return repo.get_contents(path, ref=ref).decoded_content
+    if repo is not None:
+        return repo.get_contents(path, ref=ref).decoded_content
+    return _gh_get_contents(path, ref=ref).decoded_content
 
 
 def load_pqt_github(
@@ -98,8 +118,7 @@ class GithubBlob:
 
     @property
     def data_stream(self):
-        gh = get_github_repo()
-        blob = gh.get_git_blob(self.hexsha)
+        blob = _gh_get_git_blob(self.hexsha)
         return BytesIO(b64decode(blob.content))
         # return BytesIO(process.output('gh', 'api', '-H', 'Accept: application/vnd.github.raw+json', f'/repos/{REPO}/git/blobs/{self.hexsha}'))
         # TODO: streaming response
@@ -118,14 +137,12 @@ class GithubTree:
 
     @staticmethod
     def from_commit(commit: Commit) -> 'GithubTree':
-        gh = get_github_repo()
-        tree = gh.get_git_tree(commit.raw_data['commit']['tree']['sha'])
+        tree = _gh_get_git_tree(commit.raw_data['commit']['tree']['sha'])
         return GithubTree(tree)
 
     @staticmethod
     def from_sha(sha: str) -> 'GithubTree':
-        gh = get_github_repo()
-        return GithubTree(gh.get_git_tree(sha))
+        return GithubTree(_gh_get_git_tree(sha))
 
     @cached_property
     def raw_data(self):
@@ -171,9 +188,8 @@ def blobs(self):
 
     @property
     def trees(self):
-        gh = get_github_repo()
         return [
-            GithubTree(gh.get_git_tree(e['sha']))
+            GithubTree(_gh_get_git_tree(e['sha']))
             for e in self.children if e['type'] == 'tree'
         ]
 
@@ -187,13 +203,11 @@ class GithubCommit:
 
     @staticmethod
     def from_git(git_commit: git.Commit) -> 'GithubCommit':
-        gh = get_github_repo()
-        return GithubCommit(gh.get_commit(git_commit.hexsha))
+        return GithubCommit(_gh_get_commit(git_commit.hexsha))
 
     @staticmethod
     def from_sha(sha: str) -> 'GithubCommit':
-        gh = get_github_repo()
-        return GithubCommit(gh.get_commit(sha))
+        return GithubCommit(_gh_get_commit(sha))
 
     @property
     def raw_data(self):
@@ -213,9 +227,8 @@ def tree_sha(self) -> str:
 
     @property
     def parents(self) -> list['GithubCommit']:
-        gh = get_github_repo()
         return [
-            GithubCommit(gh.get_commit(p['sha']))
+            GithubCommit(_gh_get_commit(p['sha']))
             for p in self.raw_data['parents']
         ]
 
@@ -229,8 +242,7 @@ def parent(self) -> 'GithubCommit':
 
     @cached_property
     def tree(self) -> GithubTree:
-        gh = get_github_repo()
-        tree = gh.get_git_tree(self.tree_sha)
+        tree = _gh_get_git_tree(self.tree_sha)
         return GithubTree(tree)
 
 

nj_crashes/utils/retry.py

@@ -0,0 +1,109 @@
+"""Retry helpers for HTTP fetches and PyGithub API calls.
+
+Both wrappers honor a `Retry-After` header when present (in seconds or
+HTTP-date form), and fall back to capped exponential backoff with jitter.
+
+GitHub's "secondary" rate limit returns 429 with a textual "temporarily being
+throttled" message; PyGithub auto-retries the primary (X-RateLimit-Remaining)
+limit but not this one. `with_gh_retry` covers the gap. `http_get_with_retry`
+covers transient upstream failures (the NJSP feed has thrown 403/5xx).
+"""
+from __future__ import annotations
+
+import random
+import time
+from email.utils import parsedate_to_datetime
+from datetime import datetime, timezone
+from functools import wraps
+
+import requests
+from github import GithubException
+
+from .log import err
+
+
+GH_RETRY_STATUSES = frozenset({429, 502, 503, 504})
+HTTP_RETRY_STATUSES = frozenset({403, 408, 429, 500, 502, 503, 504})
+
+
+def _parse_retry_after(value: str | None) -> float | None:
+    """Parse a Retry-After header value (seconds-int or HTTP-date)."""
+    if not value:
+        return None
+    try:
+        return float(int(value))
+    except (TypeError, ValueError):
+        pass
+    try:
+        when = parsedate_to_datetime(value)
+    except (TypeError, ValueError):
+        return None
+    if when is None:
+        return None
+    if when.tzinfo is None:
+        when = when.replace(tzinfo=timezone.utc)
+    return max(0.0, (when - datetime.now(timezone.utc)).total_seconds())
+
+
+def _backoff_seconds(attempt: int, base: float = 5.0, cap: float = 60.0) -> float:
+    """Exponential backoff with jitter, capped at `cap`."""
+    return min(cap, base * (2 ** attempt)) + random.uniform(0, base / 2)
+
+
+def with_gh_retry(max_attempts: int = 5, base: float = 5.0, cap: float = 60.0):
+    """Decorator: retry the wrapped fn on transient GitHub API failures."""
+    def deco(fn):
+        @wraps(fn)
+        def wrapper(*args, **kwargs):
+            for attempt in range(max_attempts):
+                try:
+                    return fn(*args, **kwargs)
+                except GithubException as e:
+                    if e.status not in GH_RETRY_STATUSES or attempt == max_attempts - 1:
+                        raise
+                    headers = getattr(e, 'headers', None) or {}
+                    retry_after = _parse_retry_after(
+                        headers.get('retry-after') or headers.get('Retry-After')
+                    )
+                    sleep_s = retry_after if retry_after is not None else _backoff_seconds(attempt, base, cap)
+                    err(f"GH API {e.status}; sleeping {sleep_s:.1f}s (attempt {attempt + 1}/{max_attempts})")
+                    time.sleep(sleep_s)
+        return wrapper
+    return deco
+
+
+def http_get_with_retry(
+    url: str,
+    *,
+    headers: dict | None = None,
+    timeout: float = 30,
+    max_attempts: int = 4,
+    base: float = 3.0,
+    cap: float = 30.0,
+    retry_statuses: frozenset[int] = HTTP_RETRY_STATUSES,
+) -> requests.Response:
+    """GET `url` with retry on transient failures (connection errors + listed
+    statuses, including 403 — observed transiently from the NJSP feed).
+
+    Returns the final `Response` (which may still be non-200 after exhausting
+    attempts — the caller decides whether to raise).
+    """
+    last_res = None
+    for attempt in range(max_attempts):
+        try:
+            res = requests.get(url, allow_redirects=True, timeout=timeout, headers=headers)
+        except requests.RequestException as e:
+            if attempt == max_attempts - 1:
+                raise
+            sleep_s = _backoff_seconds(attempt, base, cap)
+            err(f"GET {url}: {e}; sleeping {sleep_s:.1f}s (attempt {attempt + 1}/{max_attempts})")
+            time.sleep(sleep_s)
+            continue
+        last_res = res
+        if res.status_code not in retry_statuses or attempt == max_attempts - 1:
+            return res
+        retry_after = _parse_retry_after(res.headers.get('Retry-After'))
+        sleep_s = retry_after if retry_after is not None else _backoff_seconds(attempt, base, cap)
+        err(f"GET {url}: {res.status_code} {res.reason}; sleeping {sleep_s:.1f}s (attempt {attempt + 1}/{max_attempts})")
+        time.sleep(sleep_s)
+    return last_res