Skip to content

Retry transient 5xx and network errors with exponential backoff - #4

Merged
jasiek merged 3 commits into
masterfrom
transient-retry
May 9, 2026
Merged

Retry transient 5xx and network errors with exponential backoff#4
jasiek merged 3 commits into
masterfrom
transient-retry

Conversation

@jasiek

@jasiek jasiek commented May 9, 2026

Copy link
Copy Markdown
Owner

Summary

Upstream genealogy services (Geneteka, GenBaza, Lubgens, Genealogia w Archiwach, GenPod) intermittently return 502/503/504 or briefly drop connections. Today these surface as tool failures even though a short retry almost always recovers. This PR adds a shared transient-error retry layer used by every live HTTP client.

What changed

  • New src/polish_genealogy_mcp/sources/_http_retry.pyrequest_with_retry() wraps an HTTP send callable with full-jitter exponential backoff. Retries on:
    • httpx.TransportError / httpx.TimeoutException
    • HTTP 502, 503, 504
      Defaults: 3 attempts, 1s base delay, 30s cap. Non-retryable responses are returned untouched (the caller still calls raise_for_status).
  • All five live clients now route their requests through the helper:
    • geneteka (search, get_regions_html)
    • genbaza (_get)
    • lubgens (search)
    • genealogia_w_archiwach (Vaadin bootstrap GET/POST + _post_uidl)
    • genpod (login GET/POST, post-login home GET, GraphQL POST)
      The per-source rate limiter's wait() is invoked inside the retried callable so pacing is still respected across retries.
  • Configurable via env, CLI, and Claude Desktop user_config:
    • GENEALOGY_RETRY_MAX_ATTEMPTS (default 3)
    • GENEALOGY_RETRY_BASE_DELAY (default 1)
    • GENEALOGY_RETRY_MAX_DELAY (default 30)
      Wired through _cli_config.CONFIG_ENTRIES and manifest.json.

Tests

  • New tests/test_http_retry.py: success on first try, retry-then-success on 502 and ConnectError, exhaustion on persistent 503 / ReadTimeout, no retry on 404, jitter respects max_delay.
  • tests/test_genbaza_client.py: existing 503 case keeps the HTTPStatusError assertion (now with GENEALOGY_RETRY_BASE_DELAY=0 to keep the test fast); added a 502 → 502 → 200 end-to-end retry case and a non-retried 404 case.
  • Full suite: 119 passed.

Reviewer notes

  • The helper deliberately accepts an injectable sleep so unit tests don't actually sleep; production code uses time.sleep.
  • Full jitter (random.uniform(0, capped_delay)) is used to avoid synchronized retries if multiple requests fail at once.
  • Each retry passes through the rate limiter — so for a chatty source with a 5s pacing window, a retry will still wait until the next slot. That is intentional: we'd rather be slow than 429'd.

🤖 Generated with Claude Code

jasiek and others added 3 commits May 9, 2026 19:25
Upstream genealogy services (Geneteka, GenBaza, Lubgens, Genealogia w
Archiwach, GenPod) intermittently return 502/503/504 or briefly drop
connections. These were surfacing as tool failures even though a quick
retry almost always succeeds.

Add a shared `request_with_retry` helper in `sources/_http_retry.py`
that wraps an HTTP send callable, retrying on `httpx.TransportError`,
`httpx.TimeoutException`, and 502/503/504 responses with full-jitter
exponential backoff (3 attempts / 1s base / 30s cap by default). Each
of the five live source clients now routes its requests through it,
keeping the per-source rate limiter inside the retried callable so
pacing is still respected on retries.

Tunable via env / CLI / Claude Desktop user_config:
GENEALOGY_RETRY_MAX_ATTEMPTS, GENEALOGY_RETRY_BASE_DELAY,
GENEALOGY_RETRY_MAX_DELAY.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The retry tuning (attempts / base delay / max delay) doesn't need to be
user-facing — keep them as module constants in `_http_retry.py` instead
of CLI flags, env vars, and Claude Desktop user_config fields. The
helper still accepts overrides as kwargs for tests.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace the per-call `request_with_retry(_send)` wrapper with
`RetryTransport`, an `httpx.BaseTransport` middleware. Each client now
sets `transport=RetryTransport(...)` once at construction; every
`client.get`/`client.post` automatically retries on transient errors,
and the call sites collapse back to plain HTTP calls — no `_send`
closures, no retry-aware control flow leaking into the source clients.

`RetryTransport` composes with any other transport (including the
`MockTransport` instances used in tests), so test wiring stays the same.
On 5xx exhaustion the last response is returned (preserving the
caller's `raise_for_status` behaviour); on transport-error exhaustion
the last exception is re-raised.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@jasiek
jasiek merged commit 2ac1030 into master May 9, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant