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

Remove deprecated method calls. Fix #165. #166

Merged
merged 1 commit into from
Nov 7, 2014
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
2 changes: 1 addition & 1 deletion lib/rollbar/better_errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def show_error_page(*args)
controller = env['action_controller.instance']
request_data = controller.rollbar_request_data rescue nil
person_data = controller.rollbar_person_data rescue nil
exception_data = Rollbar.report_exception(exception, request_data, person_data)
exception_data = Rollbar.scope(:request => request_data, :person => person_data).error(exception)
rescue => e
Rollbar.log_warning "[Rollbar] Exception while reporting exception to Rollbar: #{e}"
end
Expand Down
4 changes: 2 additions & 2 deletions lib/rollbar/delayed_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ module Delayed
def self.wrap_worker
return if @wrapped
@wrapped = true

::Delayed::Worker.lifecycle.around(:invoke_job) do |job, *args, &block|
begin
block.call(job, *args)
rescue Exception => e
if job.attempts >= ::Rollbar.configuration.dj_threshold
data = ::Rollbar.configuration.report_dj_data ? job : nil
::Rollbar.report_exception(e, data)
::Rollbar.scope(:request => data).error(e)
end
raise e
end
Expand Down
6 changes: 3 additions & 3 deletions lib/rollbar/goalie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ def render_exception(env, exception)
controller = env['action_controller.instance']
request_data = controller.rollbar_request_data rescue nil
person_data = controller.rollbar_person_data rescue nil
exception_data = Rollbar.report_exception(exception, request_data, person_data)
exception_data = Rollbar.scope(:request => request_data, :person => person_data).error(exception)
rescue => e
Rollbar.log_warning "[Rollbar] Exception while reporting exception to Rollbar: #{e}"
Rollbar.log_warning "[Rollbar] Exception while reporting exception to Rollbar: #{e}"
end

# if an exception was reported, save uuid in the env
# so it can be displayed to the user on the error page
if exception_data.is_a?(Hash)
Expand Down
3 changes: 2 additions & 1 deletion lib/rollbar/rake.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
module Rake
class Application
alias_method :orig_display_error_message, :display_error_message

def display_error_message(ex)
Rollbar.report_exception(ex)
Rollbar.error(ex)
orig_display_error_message(ex)
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rollbar/rake_tasks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class RollbarTestingException < RuntimeError; end
exit
end

Rollbar.report_message("Test error from rollbar:test", "error")
Rollbar.error('Test error from rollbar:test')

begin
require './app/controllers/application_controller'
Expand Down
8 changes: 5 additions & 3 deletions lib/rollbar/sidekiq.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ def call(worker, msg, queue)
yield
rescue Exception => e
params = msg.reject{ |k| PARAM_BLACKLIST.include?(k) }
scope = { :request => { :params => params } }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine, though now that we have scope(), we could put this in a different part of the payload with a better name. Maybe custom? Will think about it more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but actually report_exception is using :request for this case, perhaps the users will start viewing the data in a different format/way in their occurrence page.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Let's keep it as is for now - just making a note for myself.


Rollbar.report_exception(e, :params => params)
Rollbar.scope(scope).report_exception(e)
raise
end
end
Expand All @@ -27,8 +28,9 @@ def call(worker, msg, queue)
Sidekiq.configure_server do |config|
config.error_handlers << Proc.new do |e, context|
params = context.reject{ |k| PARAM_BLACKLIST.include?(k) }

Rollbar.report_exception(e, :params => params)
scope = { :request => { :params => params } }

Rollbar.scope(scope).error(e)
end
end
end
2 changes: 1 addition & 1 deletion spec/dummyapp/app/controllers/home_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def deprecated_report_exception
begin
foo = bar
rescue => e
Rollbar.report_exception(e)
Rollbar.error(e)
end
render :json => {}
end
Expand Down
5 changes: 2 additions & 3 deletions spec/rollbar/delayed_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,10 @@ def initialize(options = {})
end

it 'sends the exception' do
expect_any_instance_of(Rollbar::Notifier).to receive(:error).with(kind_of(FailingJob::TestException))

expect do
Delayed::Job.enqueue(FailingJob.new)
end.to raise_error(FailingJob::TestException)

last_report = Rollbar.last_report
expect(last_report[:request]).to be_kind_of(DummyBackend::Job)
end
end
12 changes: 6 additions & 6 deletions spec/rollbar_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,7 @@ def backtrace
it 'should report simple messages' do
logger_mock.should_receive(:info).with('[Rollbar] Scheduling payload')
logger_mock.should_receive(:info).with('[Rollbar] Success')
Rollbar.report_message("Test message")
Rollbar.error('Test message')
end

it 'should not report anything when disabled' do
Expand All @@ -918,7 +918,7 @@ def backtrace
config.enabled = false
end

Rollbar.report_message("Test message that should be ignored")
Rollbar.error('Test message that should be ignored')

Rollbar.configure do |config|
config.enabled = true
Expand All @@ -927,8 +927,8 @@ def backtrace

it 'should report messages with extra data' do
logger_mock.should_receive(:info).with('[Rollbar] Success')
Rollbar.report_message("Test message with extra data", 'debug', :foo => "bar",
:hash => { :a => 123, :b => "xyz" })
Rollbar.debug('Test message with extra data', 'debug', :foo => "bar",
:hash => { :a => 123, :b => "xyz" })
end

# END Backwards
Expand Down Expand Up @@ -1127,7 +1127,7 @@ def backtrace
it 'sends a payload generated as String, not as a Hash' do
logger_mock.should_receive(:info).with('[Rollbar] Success')

Rollbar.report_exception(exception)
Rollbar.error(exception)
end

context 'with async failover handlers' do
Expand All @@ -1150,7 +1150,7 @@ def backtrace
it 'doesnt call any failover handler' do
expect(handler).not_to receive(:call)

Rollbar.report_exception(exception)
Rollbar.error(exception)
end
end

Expand Down