diff --git a/lib/faraday/error.rb b/lib/faraday/error.rb index 84cca27e2..12ff15d77 100644 --- a/lib/faraday/error.rb +++ b/lib/faraday/error.rb @@ -79,26 +79,47 @@ def exc_msg_and_response!(exc, response = nil) # Pulls out potential parent exception and response hash. def exc_msg_and_response(exc, response = nil) - if exc.is_a?(Exception) + case exc + when Exception [exc, exc.message, response] - elsif exc.is_a?(Hash) || exc.is_a?(Faraday::Env) - http_status = exc.fetch(:status) - - request = exc.fetch(:request, nil) - - if request.nil? - exception_message = "the server responded with status #{http_status} - method and url are not available " \ - 'due to include_request: false on Faraday::Response::RaiseError middleware' - else - exception_message = "the server responded with status #{http_status} for #{request.fetch(:method).upcase} " \ - "#{request.fetch(:url)}" - end - - [nil, exception_message, exc] + when Hash + [nil, build_error_message_from_hash(exc), exc] + when Faraday::Env + [nil, build_error_message_from_env(exc), exc] else [nil, exc.to_s, response] end end + + private + + def build_error_message_from_hash(hash) + # Be defensive with external Hash objects - they might be missing keys + status = hash.fetch(:status, nil) + request = hash.fetch(:request, nil) + + return fallback_error_message(status) if request.nil? + + method = request.fetch(:method, nil) + url = request.fetch(:url, nil) + build_status_error_message(status, method, url) + end + + def build_error_message_from_env(env) + # Faraday::Env is internal - we can make reasonable assumptions about its structure + build_status_error_message(env.status, env.method, env.url) + end + + def build_status_error_message(status, method, url) + method_str = method ? method.to_s.upcase : '' + url_str = url ? url.to_s : '' + "the server responded with status #{status} for #{method_str} #{url_str}" + end + + def fallback_error_message(status) + "the server responded with status #{status} - method and url are not available " \ + 'due to include_request: false on Faraday::Response::RaiseError middleware' + end end # Faraday client error class. Represents 4xx status responses. diff --git a/spec/faraday/error_spec.rb b/spec/faraday/error_spec.rb index 05285f762..bb5007713 100644 --- a/spec/faraday/error_spec.rb +++ b/spec/faraday/error_spec.rb @@ -89,5 +89,87 @@ it { expect(subject.response_headers).to eq(headers) } it { expect(subject.response_body).to eq(body) } end + + context 'with hash missing status key' do + let(:exception) { { body: 'error body' } } + + it { expect(subject.wrapped_exception).to be_nil } + it { expect(subject.response).to eq(exception) } + it { expect(subject.message).to eq('the server responded with status - method and url are not available due to include_request: false on Faraday::Response::RaiseError middleware') } + end + + context 'with hash with status but missing request data' do + let(:exception) { { status: 404, body: 'not found' } } # missing request key + + it { expect(subject.wrapped_exception).to be_nil } + it { expect(subject.response).to eq(exception) } + it { expect(subject.message).to eq('the server responded with status 404 - method and url are not available due to include_request: false on Faraday::Response::RaiseError middleware') } + end + + context 'with hash with status and request but missing method in request' do + let(:exception) { { status: 404, body: 'not found', request: { url: 'http://example.com/test' } } } # missing method + + it { expect(subject.wrapped_exception).to be_nil } + it { expect(subject.response).to eq(exception) } + it { expect(subject.message).to eq('the server responded with status 404 for http://example.com/test') } + end + + context 'with hash with status and request but missing url in request' do + let(:exception) { { status: 404, body: 'not found', request: { method: :get } } } # missing url + + it { expect(subject.wrapped_exception).to be_nil } + it { expect(subject.response).to eq(exception) } + it { expect(subject.message).to eq('the server responded with status 404 for GET ') } + end + + context 'with properly formed Faraday::Env' do + # This represents the normal case - a well-formed Faraday::Env object + # with all the standard properties populated as they would be during + # a typical HTTP request/response cycle + let(:exception) { Faraday::Env.new } + + before do + exception.status = 500 + exception.method = :post + exception.url = URI('https://api.example.com/users') + exception.request = Faraday::RequestOptions.new + exception.response_headers = { 'content-type' => 'application/json' } + exception.response_body = '{"error": "Internal server error"}' + exception.request_headers = { 'authorization' => 'Bearer token123' } + exception.request_body = '{"name": "John"}' + end + + it { expect(subject.wrapped_exception).to be_nil } + it { expect(subject.response).to eq(exception) } + it { expect(subject.message).to eq('the server responded with status 500 for POST https://api.example.com/users') } + end + + context 'with Faraday::Env missing status key' do + let(:exception) { Faraday::Env.new } + + before do + exception[:body] = 'error body' + # Intentionally not setting status + end + + it { expect(subject.wrapped_exception).to be_nil } + it { expect(subject.response).to eq(exception) } + it { expect(subject.message).to eq('the server responded with status for ') } + end + + context 'with Faraday::Env with direct method and url properties' do + let(:exception) { Faraday::Env.new } + + before do + exception.status = 404 + exception.method = :get + exception.url = URI('http://example.com/test') + exception[:body] = 'not found' + end + + it { expect(subject.wrapped_exception).to be_nil } + it { expect(subject.response).to eq(exception) } + it { expect(subject.message).to eq('the server responded with status 404 for GET http://example.com/test') } + end end end