diff --git a/lib/agentic/llm_client.rb b/lib/agentic/llm_client.rb index ae7d4dc..8a6581b 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 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") }