Merge pull request #559 from bugsnag/tobyhs/rake_with_module_prepend

Use Module#prepend for Rake integration if Ruby version is new enough
bugsnag · Aug 16, 2019 · 1446466 · 1446466
2 parents df85d64 + 26c350f
commit 1446466
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,12 @@ Changelog
 
 ### Fixes
 
+* Use `Module#prepend` for Rake integration when on a new enough Ruby version
+  to avoid infinite mutual recursion issues when something else monkey patches
+  `Rake::Task`.
+  | [#556](https://github.com/bugsnag/bugsnag-ruby/issues/556)
+  | [#559](https://github.com/bugsnag/bugsnag-ruby/issues/559)
+
 * Handle `nil` values for the `job` block parameter for the Que error notifier.
   This occurs under some conditions such as database connection failures.
   | [#545](https://github.com/bugsnag/bugsnag-ruby/issues/545)

diff --git 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
@@ -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
@@ -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
+end