Repository navigation
fix(llm_client): map Faraday::Error HTTP failures so RetryHandler can retry them - #61
Open
minerva-sky wants to merge 2 commits into
Open
minerva-sky wants to merge 2 commits into
minerva-sky wants to merge 2 commits into
Conversation
… retry them ruby-openai 8.x re-raises Faraday::Error for 429/401/5xx/timeouts instead of wrapping them in OpenAI::Error, so the rescue in #execute_request never reached map_openai_error and every HTTP failure became a generic LlmError that RetryHandler refused to retry. Rescue Faraday::Error alongside OpenAI::Error and map by response_status (429 → rate limit, 401/403 → auth, 400/404/422 → invalid request, 5xx → server, otherwise network). WIP: specs for the new mapping branch land in the next commit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ock indentation Unit specs for map_openai_error with Faraday::TooManyRequestsError (429 + retry-after), Unauthorized/Forbidden (401/403), BadRequest/ResourceNotFound/ UnprocessableEntity (400/404/422), ServerError (503), a status-less Faraday::Error, and Faraday::TimeoutError. Integration specs through #complete: a Faraday 429 now surfaces as LlmRateLimitError instead of a generic LlmError, and with fail_on_error: true RetryHandler retries it and succeeds on the next attempt. Also re-indents the Faraday::Error branch, which landed flush-left in the previous commit (standardrb clean). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
1 participant
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.
Problem
LlmClient#execute_requestonly rescuedOpenAI::Error. ruby-openai 8.x (8.3.0 in the lockfile) re-raisesFaraday::Errorsubclasses for HTTP failures (429 / 401 / 5xx / timeouts) instead of wrapping them, somap_openai_errorwas unreachable for every real HTTP error. They fell through to a genericLlmError, which is not inRetryHandler's retryable set, so rate limits and server errors were never retried.Found by inspection of
llm_client.rb:163-183andretry_handler.rb:59-62; quality-loop candidate.Change
OpenAI::Error, Faraday::Errorin#execute_request.Faraday::Errorbranch inmap_openai_error, keyed onresponse_status: 429 →LlmRateLimitError(withretry-after), 401/403 →LlmAuthenticationError, 400/404/422 →LlmInvalidRequestError, 5xx →LlmServerError, no status →LlmNetworkError. ExistingFaraday::TimeoutError/Faraday::ConnectionFailedbranches are untouched and still win.#completeshowing a Faraday 429 surfaces asLlmRateLimitErrorand is retried to success withfail_on_error: true.Notes for review
fail_on_error: true, becausehandle_errorreturns a failedLlmResponseinstead of raising otherwise, soRetryHandlernever sees the error. That is pre-existing behaviour and out of scope here; flagging it in case you want a follow-up.bundle exec rakelocally: standard clean, 1396 examples, the only failure is the EventDispatcher timing spec (total_time < 0.1, got 0.40 undernicenext to Plex), unrelated.🤖 Generated with Claude Code