feat(reliability): bounded retry for transient LLM/embedding provider failures#928
Closed
seonghobae wants to merge 1 commit into
Closed
feat(reliability): bounded retry for transient LLM/embedding provider failures#928seonghobae wants to merge 1 commit into
seonghobae wants to merge 1 commit into
Conversation
… failures Request-path provider calls previously had zero retry: one flaky connection, timeout, 429, or provider 5xx became a user-facing failure (search embedding, summary extraction, translation, reply drafting). Adds services/retry.py — a stdlib-only retry_transient helper (no new dependency; requirements are hash-pinned): retries ONLY transient errors (APIConnectionError, APITimeoutError, RateLimitError, InternalServerError) with exponential backoff + jitter, capped at 2 retries / 8s delay. Auth and 4xx validation errors propagate immediately, preserving fail-closed behavior. Wired into the four provider call sites (embeddings.create, extraction parse, translation, reply drafting); error mapping and client lifecycle unchanged. Tests: tests/test_retry.py (5) — success passthrough, transient-then-success with exponential delays, budget exhaustion re-raises original error, non-transient fails immediately, custom retryable tuple. Regressions green locally: llm_service/embedding/search/llm_api/import_fixtures (92 passed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017RkKdtHRLG4wSLh6PVsp8J
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if attempt >= retries: | ||
| raise | ||
| delay = min(base_delay_seconds * (2**attempt), MAX_DELAY_SECONDS) | ||
| delay += random.uniform(0, delay / 2) |
Contributor
|
PR governance metadata gate is not ready for
|
Contributor
Author
|
Folded into #933, which is now retargeted to |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Gap
Request-path provider calls had zero retry: a single flaky connection, timeout, 429, or provider 5xx became a user-facing 500 — across search embeddings, summary extraction, translation, and reply drafting. (Reliability audit item #3.)
What
services/retry.py: stdlib-onlyretry_transient— no new dependency (requirements are hash-pinned). Retries ONLY transient errors (APIConnectionError,APITimeoutError,RateLimitError,InternalServerError) with exponential backoff + jitter (max 2 retries, 8s cap). Auth/4xx validation errors propagate immediately → existing fail-closed behavior preserved.embedding.generate_embeddings,extract_todos_and_summary,translate_email_body,draft_reply. Error mapping (EmbeddingGenerationError/LLMServiceError) and client lifecycle unchanged.Verification (local — CI currently frozen org-wide)
tests/test_retry.py(5 new): success passthrough · transient→success with exponential delays · budget exhaustion re-raises original · non-transient immediate fail · custom retryable tuple.test_llm_service+test_embedding+test_search+test_llm_api+test_import_fixtures= 92 passed.Not included (follow-up)
Circuit breaker / load-shedding when a provider is hard-down — retry alone is the right first slice; a breaker needs shared state and deserves its own design.
🤖 Generated with Claude Code