Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guard against various Faraday exception response formats #1428

Merged
merged 4 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ endif::[]
[[release-notes-4.x]]
=== Ruby Agent version 4.x

[float]
===== Fixed
- Guard against various Faraday exception response formats {pull}1428[#1428]

[[release-notes-4.7.0]]
==== 4.7.0

Expand Down
10 changes: 8 additions & 2 deletions lib/elastic_apm/spies/faraday.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,14 @@ def run_request(method, url, body, headers, &block)
yield req if block
end
rescue Faraday::ClientError, Faraday::ServerError => e # Faraday::Response::RaiseError
status = e.response_status if e.respond_to?(:response_status)
status ||= e.response&.fetch(:status)
picandocodigo marked this conversation as resolved.
Show resolved Hide resolved
status =
if e.respond_to?(:response_status)
e.response_status
elsif e.response && e.response.respond_to?(:status)
e.response.status
elsif e.response && e.response.respond_to?(:fetch)
e.response[:status]
end
http = span&.context&.http
if http && status
http.status_code = status.to_s
Expand Down
6 changes: 4 additions & 2 deletions spec/elastic_apm/metrics/cpu_mem_set_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ module Metrics
'system.memory.total': 4_042_711_040,
'system.process.cpu.total.norm.pct': 0.2,
'system.process.memory.size': 53_223_424,
'system.process.memory.rss.bytes': 12_738_560
'system.process.memory.rss.bytes': 25_477_120
)
end

Expand All @@ -72,7 +72,7 @@ module Metrics
'system.memory.total': 4_042_711_040,
'system.process.cpu.total.norm.pct': 0.2,
'system.process.memory.size': 53_223_424,
'system.process.memory.rss.bytes': 12_738_560
'system.process.memory.rss.bytes': 25_477_120
)
end
end
Expand Down Expand Up @@ -100,6 +100,8 @@ def mock_proc_files(
proc_stat_format: :debian,
proc_meminfo_format: nil
)
allow_any_instance_of(ElasticAPM::Metrics::CpuMemSet::Linux::Meminfo).
to receive(:`).with('getconf PAGESIZE').and_return('8192')
{
'/proc/stat' =>
["proc_stat_#{proc_stat_format}", { user: user, idle: idle }],
Expand Down
75 changes: 73 additions & 2 deletions spec/elastic_apm/spies/faraday_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ module ElasticAPM
end
end

it 'should capture status_code' do
it 'captures status_code' do
WebMock.stub_request(:get, 'http://example.com')
.to_return(status: [404, 'Not Found'])

Expand All @@ -217,7 +217,7 @@ module ElasticAPM
expect(http.status_code).to match('404')
end

it 'should handle a nil response' do
it 'handles a nil response' do
WebMock.stub_request(:get, 'http://example.com')
.to_raise(Faraday::ClientError)

Expand All @@ -235,6 +235,77 @@ module ElasticAPM
expect(http.status_code).to be nil
end

it 'handles faraday response' do
class FaradayErrorWithResponseObject < Faraday::ClientError
def response
Faraday::Response.new(status: 500)
end
undef response_status
end
WebMock.stub_request(:get, 'http://example.com')
.to_raise(FaradayErrorWithResponseObject.new(nil))

with_agent do
begin
ElasticAPM.with_transaction 'Faraday Middleware test' do
client.get('http://example.com')
end
rescue Faraday::ClientError
end
end
span, = @intercepted.spans

http = span.context.http
expect(http.status_code).to eq('500')
end

it 'handles faraday response hash' do
class FaradayErrorWithResponseHash < Faraday::ClientError
def response
{ status: 500 }
end
undef response_status
end
WebMock.stub_request(:get, 'http://example.com')
.to_raise(FaradayErrorWithResponseHash.new(nil))

with_agent do
begin
ElasticAPM.with_transaction 'Faraday Middleware test' do
client.get('http://example.com')
end
rescue Faraday::ClientError
end
end
span, = @intercepted.spans

http = span.context.http
expect(http.status_code).to eq('500')
end

it 'does not raise error when response is string' do
class FaradayErrorWithResponseString < Faraday::ClientError
def response
'whatever'
end
undef response_status
end
WebMock.stub_request(:get, 'http://example.com')
.to_raise(FaradayErrorWithResponseString.new(nil))

with_agent do
begin
ElasticAPM.with_transaction 'Faraday Middleware test' do
client.get('http://example.com')
end
rescue Faraday::ClientError
end
end
span, = @intercepted.spans

http = span.context.http
expect(http.status_code).to be nil
end
end
end
end
Loading