Skip to content

Commit

Permalink
Send original error info with failsafe (#969)
Browse files Browse the repository at this point in the history
* feat: send original error inof with failsafe

* fix: truncate failsafe strings
  • Loading branch information
waltjones committed Jun 17, 2020
1 parent 6089aa7 commit 378f513
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 27 deletions.
11 changes: 9 additions & 2 deletions lib/rollbar/item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,18 @@ def handle_too_large_payload(stringified_payload, final_payload, attempts)
uuid = stringified_payload['data']['uuid']
host = stringified_payload['data'].fetch('server', {})['host']

original_error = {
:message => message,
:exception => exception,
:configuration => configuration,
:uuid => uuid,
:host => host
}

notifier.send_failsafe(
too_large_payload_string(attempts),
nil,
uuid,
host
original_error
)
logger.error("[Rollbar] Payload too large to be sent for UUID #{uuid}: #{Rollbar::JSON.dump(payload)}")
end
Expand Down
98 changes: 79 additions & 19 deletions lib/rollbar/notifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class Notifier

MUTEX = Mutex.new
EXTENSION_REGEXP = /.rollbar\z/.freeze
FAILSAFE_STRING_LENGTH = 10_000

def initialize(parent_notifier = nil, payload_options = nil, scope = nil)
if parent_notifier
Expand Down Expand Up @@ -158,7 +159,13 @@ def log(level, *args)
def report_with_rescue(level, message, exception, extra, context)
report(level, message, exception, extra, context)
rescue StandardError, SystemStackError => e
report_internal_error(e)
original_error = {
:message => message,
:exception => exception,
:configuration => configuration
}

report_internal_error(e, original_error)

'error'
end
Expand Down Expand Up @@ -262,32 +269,32 @@ def process_from_async_handler(payload)
end
end

def send_failsafe(message, exception, uuid = nil, host = nil)
exception_reason = failsafe_reason(message, exception)

log_error "[Rollbar] Sending failsafe response due to #{exception_reason}"

body = failsafe_body(exception_reason)

failsafe_data = {
def failsafe_initial_data(exception_reason)
{
:level => 'error',
:environment => configuration.environment.to_s,
:body => {
:message => {
:body => body
:body => failsafe_body(exception_reason)
}
},
:notifier => {
:name => 'rollbar-gem',
:version => VERSION
},
:custom => {
:orig_uuid => uuid,
:orig_host => host
},
:internal => true,
'failsafe' => true
}
end

def send_failsafe(message, exception, original_error = nil)
exception_reason = failsafe_reason(message, exception)

log_error "[Rollbar] Sending failsafe response due to #{exception_reason}"

failsafe_data = failsafe_initial_data(exception_reason)

failsafe_add_original_error_data(failsafe_data[:notifier], original_error)

failsafe_payload = {
'data' => failsafe_data
Expand All @@ -298,14 +305,67 @@ def send_failsafe(message, exception, uuid = nil, host = nil)
:notifier => self,
:configuration => configuration,
:logger => logger)
schedule_item(item)

process_item(item)
log_and_return_item_data(item)
rescue StandardError => e
log_error "[Rollbar] Error sending failsafe : #{e}"
end

failsafe_payload
end

def failsafe_add_original_error_data(payload_notifier, original_error)
return unless original_error

payload_notifier[:diagnostic] ||= {}

add_original_host(payload_notifier[:diagnostic], original_error)
add_original_uuid(payload_notifier[:diagnostic], original_error)
add_original_message(payload_notifier[:diagnostic], original_error)
add_original_error(payload_notifier[:diagnostic], original_error)
add_configured_options(payload_notifier, original_error)
end

def add_original_message(diagnostic, original_error)
diagnostic[:original_message] = original_error[:message].truncate(FAILSAFE_STRING_LENGTH) if original_error[:message]

rescue StandardError => e
diagnostic[:original_message] = "Failed: #{e.message}"
end

def add_original_error(diagnostic, original_error)
if original_error[:exception]
backtrace = original_error[:exception].backtrace
message = original_error[:exception].message
diagnostic[:original_error] = {
:message => message && message.truncate(FAILSAFE_STRING_LENGTH),
:stack => backtrace && backtrace.join(', ').truncate(FAILSAFE_STRING_LENGTH)
}
end

rescue StandardError => e
diagnostic[:original_error] = "Failed: #{e.message}"
end

def add_configured_options(payload_notifier, original_error)
if original_error[:configuration]
configured = original_error[:configuration].configured_options.configured
payload_notifier[:configured_options] = ::JSON.generate(configured).truncate(FAILSAFE_STRING_LENGTH)
end

rescue StandardError => e
payload_notifier[:configured_options] = "Failed: #{e.message}"
end

def add_original_host(diagnostic, original_error)
diagnostic[:original_host] = original_error[:host] if original_error[:host]
end

def add_original_uuid(diagnostic, original_error)
diagnostic[:original_uuid] = original_error[:uuid] if original_error[:uuid]
end

## Logging
%w[debug info warn error].each do |level|
define_method(:"log_#{level}") do |message|
Expand Down Expand Up @@ -461,31 +521,31 @@ def log_data(data)
# Reports an internal error in the Rollbar library. This will be reported within the configured
# Rollbar project. We'll first attempt to provide a report including the exception traceback.
# If that fails, we'll fall back to a more static failsafe response.
def report_internal_error(exception)
def report_internal_error(exception, original_error = nil)
log_error '[Rollbar] Reporting internal error encountered while sending data to Rollbar.'

configuration.execute_hook(:on_report_internal_error, exception)

begin
item = build_item('error', nil, exception, { :internal => true }, nil)
rescue StandardError => e
send_failsafe('build_item in exception_data', e)
send_failsafe('build_item in exception_data', e, original_error)
log_error "[Rollbar] Exception: #{exception}"
return
end

begin
process_item(item)
rescue StandardError => e
send_failsafe('error in process_item', e)
send_failsafe('error in process_item', e, original_error)
log_error "[Rollbar] Item: #{item}"
return
end

begin
log_instance_link(item['data'])
rescue StandardError => e
send_failsafe('error logging instance link', e)
send_failsafe('error logging instance link', e, original_error)
log_error "[Rollbar] Item: #{item}"
return
end
Expand Down
4 changes: 2 additions & 2 deletions spec/rollbar/item_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ def as_json(*)
host = payload['data']['server']['host']
log_message = "[Rollbar] Payload too large to be sent for UUID #{uuid}: #{Rollbar::JSON.dump(payload)}"

expect(notifier).to receive(:send_failsafe).with(rollbar_message, nil, uuid, host)
expect(notifier).to receive(:send_failsafe).with(rollbar_message, nil, hash_including(:uuid => uuid, :host => host))
expect(logger).to receive(:error).with(log_message)

item.dump
Expand All @@ -823,7 +823,7 @@ def as_json(*)
uuid = payload['data']['uuid']
log_message = "[Rollbar] Payload too large to be sent for UUID #{uuid}: #{Rollbar::JSON.dump(payload)}"

expect(notifier).to receive(:send_failsafe).with(rollbar_message, nil, uuid, nil)
expect(notifier).to receive(:send_failsafe).with(rollbar_message, nil, hash_including(:uuid => uuid))
expect(logger).to receive(:error).with(log_message)

item.dump
Expand Down
47 changes: 43 additions & 4 deletions spec/rollbar_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1102,6 +1102,40 @@ def backtrace
expect(extra_value).to be_eql('bad value 1')
end
end

context 'with failsafe error' do
let(:message) { 'original_error' }
let(:exception) { StandardError.new(message) }

it 'should report original error data' do
allow(Rollbar.notifier).to receive(:build_item).and_raise(StandardError.new('failsafe error'))
Rollbar.error(exception)

expect(Rollbar.last_report[:notifier][:diagnostic][:original_error][:message]).to be_eql(message)
end

it 'should report original error stack' do
allow(Rollbar.notifier).to receive(:build_item).and_raise(StandardError.new('failsafe error'))

exc_with_stack = nil
begin
raise exception
rescue => e
exc_with_stack = e
end
Rollbar.error(exc_with_stack)

expect(Rollbar.last_report[:notifier][:diagnostic][:original_error][:message]).to be_eql(message)
expect(Rollbar.last_report[:notifier][:diagnostic][:original_error][:stack].length).to be > 0
end

it 'should report original error data' do
allow(Rollbar.notifier).to receive(:build_item).and_raise(StandardError.new('failsafe error'))
Rollbar.error(message)

expect(Rollbar.last_report[:notifier][:diagnostic][:original_message]).to be_eql(message)
end
end
end

# Backwards
Expand Down Expand Up @@ -1814,11 +1848,16 @@ def backtrace
let(:host) { 'the-host' }
let(:uuid) { 'the-uuid' }
it 'sets the uuid and host in correct keys' do
original_error = {
:uuid => uuid,
:host => host
}

sent_payload = notifier.send(:send_failsafe, 'testing uuid and host',
exception, uuid, host)
exception, original_error)

expect(sent_payload['data'][:custom][:orig_uuid]).to be_eql('the-uuid')
expect(sent_payload['data'][:custom][:orig_host]).to be_eql('the-host')
expect(sent_payload['data'][:notifier][:diagnostic][:original_uuid]).to be_eql('the-uuid')
expect(sent_payload['data'][:notifier][:diagnostic][:original_host]).to be_eql('the-host')
end
end
end
Expand Down Expand Up @@ -2010,7 +2049,7 @@ class DummyClass

it 'retries the request' do
expect_any_instance_of(Net::HTTP).to receive(:request).exactly(3)
expect(Rollbar.notifier).to receive(:report_internal_error).with(net_exception)
expect(Rollbar.notifier).to receive(:report_internal_error).with(net_exception, anything)

Rollbar.info('foo')
end
Expand Down

0 comments on commit 378f513

Please sign in to comment.