From c52a9a73bd47752cb7064d697256b326261b451a Mon Sep 17 00:00:00 2001 From: Toby Hsieh Date: Tue, 13 Aug 2019 17:56:40 -0700 Subject: [PATCH] Use Module#prepend for Rake integration if Ruby version is new enough This might fix https://github.com/bugsnag/bugsnag-ruby/issues/556 If another gem uses `Module#prepend` on `Rake::Task`, then it can lead to infinite mutual recursion if we use alias_method to monkey patch the `#execute` method. --- lib/bugsnag/integrations/rake.rb | 82 ++++++++++++++++++++++---------- spec/fixtures/tasks/Rakefile | 12 +++++ spec/integrations/rake_spec.rb | 21 ++++---- 3 files changed, 77 insertions(+), 38 deletions(-) diff --git a/lib/bugsnag/integrations/rake.rb b/lib/bugsnag/integrations/rake.rb index a1cfbd397..9c7df1ef9 100644 --- a/lib/bugsnag/integrations/rake.rb +++ b/lib/bugsnag/integrations/rake.rb @@ -2,36 +2,66 @@ Rake::TaskManager.record_task_metadata = true -class Rake::Task - - FRAMEWORK_ATTRIBUTES = { - :framework => "Rake" - } - - ## - # Executes the rake task with Bugsnag setup with contextual data. - def execute_with_bugsnag(args=nil) - Bugsnag.configuration.app_type ||= "rake" - old_task = Bugsnag.configuration.request_data[:bugsnag_running_task] - Bugsnag.configuration.set_request_data :bugsnag_running_task, self - - execute_without_bugsnag(args) - - rescue Exception => ex - Bugsnag.notify(ex, true) do |report| - report.severity = "error" - report.severity_reason = { - :type => Bugsnag::Report::UNHANDLED_EXCEPTION_MIDDLEWARE, - :attributes => FRAMEWORK_ATTRIBUTES +if Gem::Version.new(RUBY_VERSION.dup) >= Gem::Version.new('2.0') + module Bugsnag + module RakeTask + FRAMEWORK_ATTRIBUTES = { + framework: 'Rake' } + + # Executes the rake task with Bugsnag setup with contextual data. + def execute(args = nil) + Bugsnag.configuration.app_type ||= "rake" + old_task = Bugsnag.configuration.request_data[:bugsnag_running_task] + Bugsnag.configuration.set_request_data :bugsnag_running_task, self + + super + rescue Exception => ex + Bugsnag.notify(ex, true) do |report| + report.severity = "error" + report.severity_reason = { + type: Bugsnag::Report::UNHANDLED_EXCEPTION_MIDDLEWARE, + attributes: FRAMEWORK_ATTRIBUTES + } + end + raise + ensure + Bugsnag.configuration.set_request_data :bugsnag_running_task, old_task + end end - raise - ensure - Bugsnag.configuration.set_request_data :bugsnag_running_task, old_task end - alias_method :execute_without_bugsnag, :execute - alias_method :execute, :execute_with_bugsnag + Rake::Task.send(:prepend, Bugsnag::RakeTask) +else + class Rake::Task + FRAMEWORK_ATTRIBUTES = { + framework: 'Rake' + } + + ## + # Executes the rake task with Bugsnag setup with contextual data. + def execute_with_bugsnag(args=nil) + Bugsnag.configuration.app_type ||= "rake" + old_task = Bugsnag.configuration.request_data[:bugsnag_running_task] + Bugsnag.configuration.set_request_data :bugsnag_running_task, self + + execute_without_bugsnag(args) + rescue Exception => ex + Bugsnag.notify(ex, true) do |report| + report.severity = "error" + report.severity_reason = { + type: Bugsnag::Report::UNHANDLED_EXCEPTION_MIDDLEWARE, + attributes: FRAMEWORK_ATTRIBUTES + } + end + raise + ensure + Bugsnag.configuration.set_request_data :bugsnag_running_task, old_task + end + + alias_method :execute_without_bugsnag, :execute + alias_method :execute, :execute_with_bugsnag + end end Bugsnag.configuration.internal_middleware.use(Bugsnag::Middleware::Rake) diff --git a/spec/fixtures/tasks/Rakefile b/spec/fixtures/tasks/Rakefile index 385b11049..236b9e290 100644 --- a/spec/fixtures/tasks/Rakefile +++ b/spec/fixtures/tasks/Rakefile @@ -1,3 +1,15 @@ +if Gem::Version.new(RUBY_VERSION.dup) >= Gem::Version.new('2.0') + # Mixing Module#prepend with alias_method can sometimes lead to infinite + # mutual recursion, so this is to test that it doesn't happen. + mod = Module.new do + def execute(args = nil) + puts 'In module prepended to Rake::Task' + super + end + end + Rake::Task.send(:prepend, mod) +end + require "bugsnag/integrations/rake" namespace :test do diff --git a/spec/integrations/rake_spec.rb b/spec/integrations/rake_spec.rb index e97dd45fb..45a884fee 100644 --- a/spec/integrations/rake_spec.rb +++ b/spec/integrations/rake_spec.rb @@ -55,18 +55,15 @@ let(:request) { JSON.parse(queue.pop) } it 'should run the rake middleware when rake tasks crash' do - #Skips this test in ruby 1.9.3 with travis - unless ENV['TRAVIS'] && RUBY_VERSION == "1.9.3" - ENV['BUGSNAG_TEST_SERVER_PORT'] = server.config[:Port].to_s - task_fixtures_path = File.join(File.dirname(__FILE__), '../fixtures', 'tasks') - Dir.chdir(task_fixtures_path) do - system("bundle exec rake test:crash > /dev/null 2>&1") - end - - result = request() - expect(result["events"][0]["metaData"]["rake_task"]).not_to be_nil - expect(result["events"][0]["metaData"]["rake_task"]["name"]).to eq("test:crash") + ENV['BUGSNAG_TEST_SERVER_PORT'] = server.config[:Port].to_s + task_fixtures_path = File.join(File.dirname(__FILE__), '../fixtures', 'tasks') + Dir.chdir(task_fixtures_path) do + system("bundle exec rake test:crash > /dev/null 2>&1") end + + result = request() + expect(result["events"][0]["metaData"]["rake_task"]).not_to be_nil + expect(result["events"][0]["metaData"]["rake_task"]["name"]).to eq("test:crash") end end -end \ No newline at end of file +end