Skip to content

Commit

Permalink
Use Module#prepend for Rake integration if Ruby version is new enough
Browse files Browse the repository at this point in the history
This might fix #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.
  • Loading branch information
Toby Hsieh committed Aug 14, 2019
1 parent df85d64 commit c52a9a7
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 38 deletions.
82 changes: 56 additions & 26 deletions lib/bugsnag/integrations/rake.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
12 changes: 12 additions & 0 deletions spec/fixtures/tasks/Rakefile
Original file line number Diff line number Diff line change
@@ -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
Expand Down
21 changes: 9 additions & 12 deletions spec/integrations/rake_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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")

This comment has been minimized.

Copy link
@DanielHeath

DanielHeath Aug 14, 2019

The crash in my app would happen when using Rake::Task['test:crash'].invoke (which has already loaded everything).

This comment has been minimized.

Copy link
@tobyhs

tobyhs Aug 14, 2019

Contributor

Hi @DanielHeath

I'm having trouble reproducing the infinite mutual recursion issue with this commit/branch even when calling Rake::Task#invoke in a Rails console.

I have the following:

~/testing/myapp$ grep 'bugsnag\|honeycomb' Gemfile
gem 'bugsnag', github: 'bugsnag/bugsnag-ruby', branch: 'tobyhs/rake_with_module_prepend'
gem 'honeycomb-beeline', '1.0.0'
~/testing/myapp$ cat lib/tasks/mytest.rake
namespace :mytest do
  desc 'Testing'
  task test: :environment do
    puts Rake::Task.ancestors.inspect
  end
end
~/testing/myapp$ bundle exec rails c
Loading development environment (Rails 5.2.3)
irb(main):001:0> Myapp::Application.load_tasks; nil
=> nil
irb(main):002:0> Rake::Task['mytest:test'].invoke
[Bugsnag::RakeTask, Honeycomb::Rake::Task, Rake::Task, ActiveSupport::ToJsonWithActiveSupportEncoder, Object, PP::ObjectMixin, ActiveSupport::Dependencies::Loadable, JSON::Ext::Generator::GeneratorMethods::Object, ActiveSupport::Tryable, Kernel, BasicObject]
=> [#<Proc:0x007fec9b204268@/Users/toby/testing/myapp/lib/tasks/mytest.rake:3>]

Would it be possible for you to create a fairly minimal Ruby app (including a Gemfile, Gemfile.lock, and .ruby-version) in a GitHub repo that still has the infinite loop issue with this bugsnag-ruby branch? (and mention the repo at the original GitHub issue at #556)

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

0 comments on commit c52a9a7

Please sign in to comment.