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

Enable Rails 7 error reporter via Rollbar config #1161

Merged
merged 5 commits into from
Aug 30, 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: 2 additions & 2 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ Style/FrozenStringLiteralComment:
Enabled: false

Style/HashSyntax:
EnforcedStyle: hash_rockets
EnforcedStyle: no_mixed_keys
SupportedStyles:
- hash_rockets
- no_mixed_keys

Style/Lambda:
Enabled: false
Expand Down
16 changes: 16 additions & 0 deletions lib/rollbar/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class Configuration
:web_base,
:write_to_file
attr_reader :before_process,
:enable_rails_error_subscriber,
:logger_level,
:project_gem_paths,
:send_extra_frame_data,
Expand Down Expand Up @@ -104,6 +105,7 @@ def initialize
@disable_core_monkey_patch = false
@disable_rack_monkey_patch = false
@enable_error_context = true
@enable_rails_error_subscriber = false
@dj_threshold = 0
@dj_use_scoped_block = false
@async_skip_report_handler = nil
Expand Down Expand Up @@ -337,6 +339,20 @@ def send_extra_frame_data=(value)
@send_extra_frame_data = value
end

def enable_rails_error_subscriber=(enable)
return if !defined?(::Rails) || ::Rails.gem_version < ::Gem::Version.new('7.1.0')

if @enable_rails_error_subscriber && !enable
::Rails.error.unsubscribe(Rollbar::ErrorSubscriber)
end

if !@enable_rails_error_subscriber && enable
::Rails.error.subscribe(Rollbar::ErrorSubscriber.new)
end

@enable_rails_error_subscriber = enable
end

# allow params to be read like a hash
def [](option)
send(option)
Expand Down
3 changes: 2 additions & 1 deletion lib/rollbar/exception_reporter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ def report_exception_to_rollbar(env, exception)
end

def capture_uncaught?
Rollbar.configuration.capture_uncaught != false
Rollbar.configuration.capture_uncaught != false &&
!Rollbar.configuration.enable_rails_error_subscriber
matux marked this conversation as resolved.
Show resolved Hide resolved
end

def log_exception_message(exception)
Expand Down
24 changes: 3 additions & 21 deletions lib/rollbar/logger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@ module Rollbar
class Logger < ::Logger
class Error < RuntimeError; end

class DatetimeFormatNotSupported < Error; end

class FormatterNotSupported < Error; end

def initialize
@level = ERROR
super(nil)

self.level = ERROR
end

def add(severity, message = nil, progname = nil)
Expand All @@ -39,22 +37,6 @@ def <<(message)
error(message)
end

def formatter=(_)
raise(FormatterNotSupported)
end

def formatter
raise(FormatterNotSupported)
end

def datetime_format=(_)
raise(DatetimeFormatNotSupported)
end

def datetime_format
raise(DatetimeFormatNotSupported)
end

# Returns a Rollbar::Notifier instance with the current global scope and
# with a logger writing to /dev/null so we don't have a infinite loop
# when Rollbar.configuration.logger is Rails.logger.
Expand Down
8 changes: 8 additions & 0 deletions lib/rollbar/plugins/rails.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,11 @@ def secure_headers_middleware?
Rollbar::Js::Frameworks::Rails.new.load(self)
end
end

Rollbar.plugins.define('rails-error-subscriber') do
dependency { defined?(Rails::VERSION) && Rails::VERSION::MAJOR >= 7 }

execute! do
require 'rollbar/plugins/rails/error_subscriber'
end
end
12 changes: 12 additions & 0 deletions lib/rollbar/plugins/rails/error_subscriber.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module Rollbar
class ErrorSubscriber
def report(error, handled:, severity:, context:, source: nil)
# The default `nil` for capture_uncaught means `true`. so check for false.
return unless handled || Rollbar.configuration.capture_uncaught != false

extra = context.is_a?(Hash) ? context.deep_dup : {}
extra[:custom_data_method_context] = source
Rollbar.log(severity, error, extra)
end
matux marked this conversation as resolved.
Show resolved Hide resolved
end
end
96 changes: 96 additions & 0 deletions spec/controllers/home_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,102 @@ def send_req(meth, path, args)
end
end

describe 'rails error subscriber', :type => 'request' do
let(:notifier) { Rollbar.notifier }

before do
Rollbar.configure do |config|
config.enable_rails_error_subscriber = true
config.capture_uncaught = nil
end
end

after do
Rollbar.configure do |config|
config.enable_rails_error_subscriber = false
config.capture_uncaught = nil
end
end

context 'when Rails Error Subscriber is enabled', if: ::Rails.gem_version >= ::Gem::Version.new('7.1.0') do
it '`handle` should not raise an error and report a warning via rails error subscriber' do
logger_mock.should_receive(:info).with('[Rollbar] Success').never

expect(Rollbar).to receive(:log) do |level, _, extra|
expect(extra[:custom_data_method_context]).to be_eql('application')
expect(level.to_s).to be_eql('warning')
end

get '/handle_rails_error'
end

it '`handle` should report a warning via rails error subscriber when capture_uncaught is false' do
logger_mock.should_receive(:info).with('[Rollbar] Success').never

Rollbar.configure do |config|
config.capture_uncaught = false
end

expect(Rollbar).to receive(:log) do |level, _, extra|
expect(extra[:custom_data_method_context]).to be_eql('application')
expect(level.to_s).to be_eql('warning')
end

get '/handle_rails_error'
end

it '`report` should raise an error and report an error via rails error subscriber' do
logger_mock.should_receive(:info).with('[Rollbar] Success').never

expect(Rollbar).to receive(:log) do |level, _, extra|
expect(extra[:custom_data_method_context]).to be_eql('application')
expect(level.to_s).to be_eql('error')
end

expect do
get '/record_rails_error'
end.to raise_exception(RuntimeError, 'Record Rails error')
end

it 'uncaught exception should raise an error and report an error via rails error subscriber' do
logger_mock.should_receive(:info).with('[Rollbar] Success').never

expect(Rollbar).to receive(:log) do |level, _, extra|
expect(extra[:custom_data_method_context]).to be_eql('application.action_dispatch')
expect(level.to_s).to be_eql('error')
end

expect do
get '/cause_exception'
end.to raise_exception(NameError, 'Uncaught Rails error')
end

it 'uncaught exception should not report an error when capture_uncaught is not set' do
logger_mock.should_receive(:info).with('[Rollbar] Success').never

Rollbar.configure do |config|
config.capture_uncaught = false
end

expect(Rollbar).to receive(:log).never

expect do
get '/cause_exception'
end.to raise_exception(NameError, 'Uncaught Rails error')
end
end

context 'when Rails Error Subscriber is enabled in unsupported Rails', if: ::Rails.gem_version < ::Gem::Version.new('7.1.0') do
it 'uncaught exception should raise an error and report via middleware' do
logger_mock.should_receive(:info).with('[Rollbar] Success').once

expect do
get '/cause_exception'
end.to raise_exception(NameError, 'Uncaught Rails error')
end
end
end

describe 'configuration.locals', :type => 'request',
:if => RUBY_VERSION >= '2.3.0' &&
!(defined?(RUBY_ENGINE) &&
Expand Down
16 changes: 15 additions & 1 deletion spec/dummyapp/app/controllers/home_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,22 @@ def deprecated_report_exception
render :json => {}
end

def handle_rails_error
Rails.error.handle do
raise 'Handle Rails error'
end

render :json => {}
end

def record_rails_error
Rails.error.record do
raise 'Record Rails error'
end
end

def cause_exception
_foo = bar
raise NameError, 'Uncaught Rails error'
end

def cause_exception_with_locals
Expand Down
2 changes: 2 additions & 0 deletions spec/dummyapp/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
member { post :start_session }
end

match '/handle_rails_error' => 'home#handle_rails_error', :via => [:get, :post]
match '/record_rails_error' => 'home#record_rails_error', :via => [:get, :post]
match '/cause_exception' => 'home#cause_exception', :via => [:get, :post]
match '/cause_exception_with_locals' => 'home#cause_exception_with_locals',
:via => [:get, :post]
Expand Down
34 changes: 1 addition & 33 deletions spec/rollbar/logger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,45 +85,13 @@
end
end

describe '#formatter=' do
it 'fails with FormatterNotSupported' do
expect do
subject.formatter = double
end.to raise_error(Rollbar::Logger::FormatterNotSupported)
end
end

describe '#formatter' do
it 'fails with FormatterNotSupported' do
expect do
subject.formatter
end.to raise_error(Rollbar::Logger::FormatterNotSupported)
end
end

describe '#datetime_format=' do
it 'fails with DatetimeFormatNotSupported' do
expect do
subject.datetime_format = double
end.to raise_error(Rollbar::Logger::DatetimeFormatNotSupported)
end
end

describe '#datetime_format' do
it 'fails with DatetimeFormatNotSupported' do
expect do
subject.datetime_format
end.to raise_error(Rollbar::Logger::DatetimeFormatNotSupported)
end
end

describe '#rollbar' do
it 'returns a Rollbar notifier with a logger pointing to /dev/null' do
notifier = subject.rollbar
logger = notifier.configuration.logger
logdev = logger.instance_eval { @logdev }

if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('3.0')
if Gem::Version.new(Logger::VERSION) >= Gem::Version.new('1.4.3')
# The Logger class no longer creates a LogDevice when the device is `File::NULL`
# See: ruby/ruby@f3e12caa088cc893a54bc2810ff511e4c89b322b
expect(logdev).to be_nil
Expand Down
Loading