feat(v0): exponential backoff for transient HTTP failures (closes #63) #74

Merged

pdurlej merged 1 commit from gemini/issue-63 into main

2026-06-08 18:33:16 +02:00

claude commented

2026-06-08 18:22:26 +02:00

Collaborator

Authored by gemini (Gemini 3.1 Pro via Antigravity), under claude's supervision. claude read the full retry implementation line-by-line, verified the exception taxonomy + D20 invariant, ran tests independently, and is opening the PR.

What

Implements #63 — stdlib-only exponential backoff for transient HTTP failures in both transports (forgejo_client._text, ollama_client._urllib_transport). A single 503/timeout no longer fails the whole CLI; it retries with [1s, 2s] backoff over 3 attempts.

claude's line-by-line review (transport for a security tool — reviewed carefully)

✅ 4xx ≠ 429 hard-fail immediately (401→Auth, 403→Auth, 404→NotFound) — retry would hide real bugs; correctly NOT retried.
✅ 429 / 503 retried with exponential backoff; Retry-After integer header honored on 429.
✅ Timeout / connection errors retried (TimeoutError, socket.timeout, URLError whose reason is ConnectionReset/Refused/BrokenPipe). A non-connection URLError (e.g. DNS) is NOT retried — correct.
✅ D20 invariant preserved: OllamaUnparseableError is raised in _attempt after the transport returns, so the transport-level retry never sees it — unparseable output never auto-recovers. Test confirms urlopen called exactly once on the unparseable path.
✅ Injectable _sleep_fn (module-level) so tests don't actually sleep; env-configurable PATCHWARDEN_HTTP_MAX_ATTEMPTS / PATCHWARDEN_HTTP_BASE_DELAY_SECONDS; ::notice:: logged per retry (never silent).

Verified (claude, independently)

PYTHONPATH=src python3 -m unittest discover tests → 200/200 OK (186 baseline + 14 new).
grep -rnE "import (requests|httpx|tenacity|backoff)" src/patchwarden/ → zero hits (stdlib-only).
tests/test_d20_architectural_boundary.py → passes.
New tests assert exact sleep delays + urlopen call counts: 503→200 (1 retry, sleep 1.0), 503×2→200 (2 retries), 503×3→raise, 404→1 call, 429+Retry-After:2→sleep 2.0, socket.timeout→retried, unparseable→1 call.

Minor note (non-blocking, future cleanup)

max_attempts is read from env with int(...). A pathological PATCHWARDEN_HTTP_MAX_ATTEMPTS=0 would make the retry loop body never execute (returns None). Defaults (3) and any sane value (≥1) are correct. A defensive max(1, ...) clamp would close the edge case — fine to fold into a later pass; left as gemini's work.

Cousin discipline

Commit authored + committed by gemini (per-worktree config isolation — no clobber).
stdlib-only, unittest, no dataclass changes, no new merge/approve/network surface.

Closes #63.

> **Authored by gemini** (Gemini 3.1 Pro via Antigravity), under claude's supervision. claude read the full retry implementation line-by-line, verified the exception taxonomy + D20 invariant, ran tests independently, and is opening the PR. ## What Implements #63 — stdlib-only exponential backoff for transient HTTP failures in both transports (`forgejo_client._text`, `ollama_client._urllib_transport`). A single 503/timeout no longer fails the whole CLI; it retries with `[1s, 2s]` backoff over 3 attempts. ## claude's line-by-line review (transport for a security tool — reviewed carefully) - ✅ **4xx ≠ 429 hard-fail immediately** (401→Auth, 403→Auth, 404→NotFound) — retry would hide real bugs; correctly NOT retried. - ✅ **429 / 503 retried** with exponential backoff; `Retry-After` integer header honored on 429. - ✅ **Timeout / connection errors retried** (`TimeoutError`, `socket.timeout`, `URLError` whose reason is `ConnectionReset/Refused/BrokenPipe`). A non-connection `URLError` (e.g. DNS) is NOT retried — correct. - ✅ **D20 invariant preserved:** `OllamaUnparseableError` is raised in `_attempt` *after* the transport returns, so the transport-level retry never sees it — unparseable output never auto-recovers. Test confirms `urlopen` called exactly once on the unparseable path. - ✅ Injectable `_sleep_fn` (module-level) so tests don't actually sleep; env-configurable `PATCHWARDEN_HTTP_MAX_ATTEMPTS` / `PATCHWARDEN_HTTP_BASE_DELAY_SECONDS`; `::notice::` logged per retry (never silent). ## Verified (claude, independently) - `PYTHONPATH=src python3 -m unittest discover tests` → **200/200 OK** (186 baseline + 14 new). - `grep -rnE "import (requests|httpx|tenacity|backoff)" src/patchwarden/` → zero hits (stdlib-only). - `tests/test_d20_architectural_boundary.py` → passes. - New tests assert exact sleep delays + urlopen call counts: 503→200 (1 retry, sleep 1.0), 503×2→200 (2 retries), 503×3→raise, 404→1 call, 429+`Retry-After:2`→sleep 2.0, socket.timeout→retried, unparseable→1 call. ## Minor note (non-blocking, future cleanup) `max_attempts` is read from env with `int(...)`. A pathological `PATCHWARDEN_HTTP_MAX_ATTEMPTS=0` would make the retry loop body never execute (returns `None`). Defaults (3) and any sane value (≥1) are correct. A defensive `max(1, ...)` clamp would close the edge case — fine to fold into a later pass; left as gemini's work. ## Cousin discipline - Commit authored + committed by `gemini` (per-worktree config isolation — no clobber). - stdlib-only, unittest, no dataclass changes, no new merge/approve/network surface. Closes #63.

Rows
Columns