Skip to content

Commit

Permalink
Guard against various Faraday exception response formats (#1428)
Browse files Browse the repository at this point in the history
* Fix randomly failing test by stubbing getconf shell out
* Guard against other Faraday exception response formats
* Fix tests for older Faraday versions
* Update CHANGELOG
  • Loading branch information
dalibor authored Jan 9, 2024
1 parent 5467b31 commit bd51c21
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 6 deletions.
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)
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

0 comments on commit bd51c21

Please sign in to comment.