From df6810cdef0e7035228c2eaa3d0c7e4d90c0a397 Mon Sep 17 00:00:00 2001 From: Minerva Sky <258328972+minerva-sky@users.noreply.github.com> Date: Mon, 12 Oct 2026 01:04:45 -0400 Subject: [PATCH 1/2] fix(llm_client): map Faraday::Error HTTP failures so RetryHandler can retry them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- lib/agentic/llm_client.rb | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/agentic/llm_client.rb b/lib/agentic/llm_client.rb index ae7d4dc..db96660 100644 --- a/lib/agentic/llm_client.rb +++ b/lib/agentic/llm_client.rb @@ -161,7 +161,10 @@ def without_retry(messages, parameters, output_schema, fail_on_error, stream_cal content = @last_response.dig("choices", 0, "message", "content") LlmResponse.success(@last_response, content) end - rescue OpenAI::Error => e + rescue OpenAI::Error, Faraday::Error => e + # ruby-openai 8.x re-raises Faraday::Error for HTTP failures (429/401/5xx/timeouts) + # rather than wrapping them in OpenAI::Error, so both must be mapped here or + # they fall through to the generic LlmError and RetryHandler never retries. error = map_openai_error(e) Agentic.logger.error("OpenAI API error: #{error.message}") handle_error(error, fail_on_error) @@ -288,6 +291,27 @@ def map_openai_error(error) "OpenAI API connection error: #{error.message}", network_exception: error ) +when defined?(Faraday::Error) && Faraday::Error + status = error.respond_to?(:response_status) ? error.response_status : nil + headers = error.respond_to?(:response_headers) ? (error.response_headers || {}) : {} + response = error.respond_to?(:response) && error.response.is_a?(Hash) ? error.response : nil + + case status + when 429 + Errors::LlmRateLimitError.new( + "OpenAI API rate limit exceeded: #{error.message}", + retry_after: headers["retry-after"]&.to_i, + response: response + ) + when 401, 403 + Errors::LlmAuthenticationError.new("OpenAI API authentication error: #{error.message}", response: response) + when 400, 404, 422 + Errors::LlmInvalidRequestError.new("Invalid request to OpenAI API: #{error.message}", response: response) + when 500..599 + Errors::LlmServerError.new("OpenAI API server error: #{error.message}", response: response) + else + Errors::LlmNetworkError.new("OpenAI API connection error: #{error.message}", network_exception: error) + end when OpenAI::Error # Parse error message to determine specific error type message = error.message.to_s.downcase From de83108a5473392b2e88ad1136f67ff04de398c0 Mon Sep 17 00:00:00 2001 From: Minerva Sky <258328972+minerva-sky@users.noreply.github.com> Date: Mon, 12 Oct 2026 02:09:42 -0400 Subject: [PATCH 2/2] test(llm_client): cover Faraday::Error mapping and retry path; fix block 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 --- lib/agentic/llm_client.rb | 42 +++++++++---------- spec/agentic/llm_client_spec.rb | 71 +++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 21 deletions(-) diff --git a/lib/agentic/llm_client.rb b/lib/agentic/llm_client.rb index db96660..8a6581b 100644 --- a/lib/agentic/llm_client.rb +++ b/lib/agentic/llm_client.rb @@ -291,27 +291,27 @@ def map_openai_error(error) "OpenAI API connection error: #{error.message}", network_exception: error ) -when defined?(Faraday::Error) && Faraday::Error - status = error.respond_to?(:response_status) ? error.response_status : nil - headers = error.respond_to?(:response_headers) ? (error.response_headers || {}) : {} - response = error.respond_to?(:response) && error.response.is_a?(Hash) ? error.response : nil - - case status - when 429 - Errors::LlmRateLimitError.new( - "OpenAI API rate limit exceeded: #{error.message}", - retry_after: headers["retry-after"]&.to_i, - response: response - ) - when 401, 403 - Errors::LlmAuthenticationError.new("OpenAI API authentication error: #{error.message}", response: response) - when 400, 404, 422 - Errors::LlmInvalidRequestError.new("Invalid request to OpenAI API: #{error.message}", response: response) - when 500..599 - Errors::LlmServerError.new("OpenAI API server error: #{error.message}", response: response) - else - Errors::LlmNetworkError.new("OpenAI API connection error: #{error.message}", network_exception: error) - end + when defined?(Faraday::Error) && Faraday::Error + status = error.respond_to?(:response_status) ? error.response_status : nil + headers = error.respond_to?(:response_headers) ? (error.response_headers || {}) : {} + response = (error.respond_to?(:response) && error.response.is_a?(Hash)) ? error.response : nil + + case status + when 429 + Errors::LlmRateLimitError.new( + "OpenAI API rate limit exceeded: #{error.message}", + retry_after: headers["retry-after"]&.to_i, + response: response + ) + when 401, 403 + Errors::LlmAuthenticationError.new("OpenAI API authentication error: #{error.message}", response: response) + when 400, 404, 422 + Errors::LlmInvalidRequestError.new("Invalid request to OpenAI API: #{error.message}", response: response) + when 500..599 + Errors::LlmServerError.new("OpenAI API server error: #{error.message}", response: response) + else + Errors::LlmNetworkError.new("OpenAI API connection error: #{error.message}", network_exception: error) + end when OpenAI::Error # Parse error message to determine specific error type message = error.message.to_s.downcase diff --git a/spec/agentic/llm_client_spec.rb b/spec/agentic/llm_client_spec.rb index 2871d26..f0bb11b 100644 --- a/spec/agentic/llm_client_spec.rb +++ b/spec/agentic/llm_client_spec.rb @@ -279,6 +279,36 @@ end end + context "when the HTTP layer raises Faraday::TooManyRequestsError (ruby-openai 8.x)" do + let(:faraday_429) do + Faraday::TooManyRequestsError.new(nil, {status: 429, headers: {"retry-after" => "1"}, body: "rate limited"}) + end + + it "maps to LlmRateLimitError rather than a generic LlmError" do + allow(openai_client).to receive(:chat).and_raise(faraday_429) + + result = subject.complete(messages, use_retries: false) + + expect(result.success?).to be false + expect(result.error).to be_a(Agentic::Errors::LlmRateLimitError) + expect(result.error.retry_after).to eq(1) + end + + it "is retried by RetryHandler and succeeds on the next attempt" do + calls = 0 + allow(openai_client).to receive(:chat) do + calls += 1 + raise faraday_429 if calls == 1 + success_response + end + + result = subject.complete(messages, fail_on_error: true) + + expect(result.success?).to be true + expect(calls).to eq(2) + end + end + context "when rate limit error occurs" do before do allow(openai_client).to receive(:chat).and_raise(rate_limit_error) @@ -488,6 +518,47 @@ end describe "#map_openai_error" do + context "with Faraday errors re-raised by ruby-openai 8.x" do + def faraday_error(klass, status, headers: {}, body: nil) + klass.new(nil, {status: status, headers: headers, body: body}) + end + + it "maps 429 to LlmRateLimitError with retry-after" do + error = faraday_error(Faraday::TooManyRequestsError, 429, headers: {"retry-after" => "7"}, body: "slow down") + mapped = subject.send(:map_openai_error, error) + expect(mapped).to be_a(Agentic::Errors::LlmRateLimitError) + expect(mapped.retry_after).to eq(7) + end + + it "maps 401 and 403 to LlmAuthenticationError" do + expect(subject.send(:map_openai_error, faraday_error(Faraday::UnauthorizedError, 401))) + .to be_a(Agentic::Errors::LlmAuthenticationError) + expect(subject.send(:map_openai_error, faraday_error(Faraday::ForbiddenError, 403))) + .to be_a(Agentic::Errors::LlmAuthenticationError) + end + + it "maps 400, 404 and 422 to LlmInvalidRequestError" do + [[Faraday::BadRequestError, 400], [Faraday::ResourceNotFound, 404], [Faraday::UnprocessableEntityError, 422]].each do |klass, status| + expect(subject.send(:map_openai_error, faraday_error(klass, status))) + .to be_a(Agentic::Errors::LlmInvalidRequestError) + end + end + + it "maps 5xx to LlmServerError" do + mapped = subject.send(:map_openai_error, faraday_error(Faraday::ServerError, 503, body: "upstream down")) + expect(mapped).to be_a(Agentic::Errors::LlmServerError) + end + + it "maps a Faraday::Error with no HTTP status to LlmNetworkError" do + mapped = subject.send(:map_openai_error, Faraday::Error.new("socket reset")) + expect(mapped).to be_a(Agentic::Errors::LlmNetworkError) + end + + it "still maps Faraday::TimeoutError to LlmTimeoutError" do + mapped = subject.send(:map_openai_error, Faraday::TimeoutError.new("execution expired")) + expect(mapped).to be_a(Agentic::Errors::LlmTimeoutError) + end + end context "with different OpenAI error types" do let(:timeout_error) { OpenAI::Timeout.new("Request timeout") } let(:rate_limit_error) { OpenAI::RateLimitError.new("Rate limit") }