Rake integration gets into an infinite loop if anything else overrides execute

[DanielHeath]

Description

I'm using https://github.com/honeycombio/beeline-ruby alongside bugsnag-ruby. They aren't getting along, though. The overrides of Rake::Task#execute are mutually recursing.

Having had a read of the relevant code on both sides (honeycomb and bugsnag), I think the issue is on the bugsnag-ruby side.

Stack trace

gems/bugsnag-6.11.1/lib/bugsnag/integrations/rake.rb:20:in `execute_with_bugsnag'
gems/honeycomb-beeline-1.0.0/lib/honeycomb/integrations/rake.rb:14:in `execute'
gems/bugsnag-6.11.1/lib/bugsnag/integrations/rake.rb:20:in `execute_with_bugsnag'
gems/honeycomb-beeline-1.0.0/lib/honeycomb/integrations/rake.rb:14:in `execute'
gems/bugsnag-6.11.1/lib/bugsnag/integrations/rake.rb:20:in `execute_with_bugsnag'
gems/honeycomb-beeline-1.0.0/lib/honeycomb/integrations/rake.rb:14:in `execute'

The issue is that honeycomb defines execute in a module, which it adds using prepend. It calls super, which (thanks to aliasing) is execute_with_bugsnag, which calls execute_without_bugsnag, which (also thanks to the aliasing in bugsnag-ruby) is the execute implementation defined by honeycomb.

This is complex, because bugsnag supports ruby < 2, which doesn't have prepend. Otherwise it'd an easy fix (do it the ruby 2 way with prepend).

I think changing to use a module for the extra behavior added to Rake::Task would solve this immediate issue, but not being able to use prepend means you wouldn't be instrumenting other modules mixed into Rake::Task.

[tobyhs]

Hi @DanielHeath

After trying a few things, it looks like adding a require: false in the Gemfile (like gem 'honeycomb-beeline', '1.0.0', require: false) avoids the issue, and you can require 'honeycomb-beeline' somewhere later like in config/initializers/honeycomb.rb. Would this be an acceptable workaround for now?

[DanielHeath]

That lets me have both gems in the project, but once you actually use honeycomb the problem is back. I would prefer to have both error and timing instrumentation on my scheduled rake tasks, if possible. Thanks, Daniel Heath

…

On 8 Aug 2019, at 5:18 am, Toby Hsieh ***@***.***> wrote: Hi @DanielHeath After trying a few things, it looks like adding a require: false in the Gemfile (like gem 'honeycomb-beeline', '1.0.0', require: false) avoids the issue. Would this be an acceptable workaround for now? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[DanielHeath]

One (ugly but well supported) option is to set a thread variable on entry and check it before recursion. Thanks, Daniel Heath

…

On 8 Aug 2019, at 5:18 am, Toby Hsieh ***@***.***> wrote: Hi @DanielHeath After trying a few things, it looks like adding a require: false in the Gemfile (like gem 'honeycomb-beeline', '1.0.0', require: false) avoids the issue. Would this be an acceptable workaround for now? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[tobyhs]

I made an update to my original comment that may have been missed (about requiring 'honeycomb-beeline' in an initializer file).

I have the following, and it doesn't encounter an infinite loop/recursion:

$ grep 'bugsnag\|honeycomb' Gemfile
gem 'bugsnag', '6.11.1'
gem 'honeycomb-beeline', '1.0.0', require: false
$ cat config/initializers/honeycomb.rb
require 'honeycomb-beeline'

Honeycomb.configure do |config|
  config.write_key = "abc123"
  config.dataset = "rails"
  config.notification_events = %w[
    sql.active_record
    render_template.action_view
    render_partial.action_view
    render_collection.action_view
    process_action.action_controller
    send_file.action_controller
    send_data.action_controller
    deliver.action_mailer
  ].freeze
end
$ cat lib/tasks/mytest.rake
namespace :mytest do
  desc 'Testing'
  task test: :environment do
    puts Rake::Task.instance_method(:execute_with_bugsnag)
    puts Rake::Task.ancestors.inspect
  end
end
$ be rake mytest:test
#<UnboundMethod: Rake::Task#execute_with_bugsnag>
[Honeycomb::Rake::Task, Rake::Task, ActiveSupport::ToJsonWithActiveSupportEncoder, Object, PP::ObjectMixin, ActiveSupport::Dependencies::Loadable, JSON::Ext::Generator::GeneratorMethods::Object, ActiveSupport::Tryable, Kernel, BasicObject]

[DanielHeath]

Gave that a try, but there's a lot of situations in my app where a rake task is invoked after the environment has already been loaded, which still hits this error.

[tobyhs]

Can you give the branch at https://github.com/bugsnag/bugsnag-ruby/tree/tobyhs/rake_with_module_prepend a try to see if it works for your app?

[DanielHeath]

Tried switching to that branch, and the mutual recursion is gone :) thanks.

[tobyhs]

I have merged #559 (into the next branch) and plan on deleting the tobyhs/rake_with_module_prepend branch soon (to keep things tidy), so if you were relying on that branch and can't wait until the next bugsnag-ruby release for the fix, I would advise using the next branch at commit 1446466

[abigailbramble]

Released in https://github.com/bugsnag/bugsnag-ruby/releases/tag/v6.12.0

[danarnold]

It looks like the code introduced to fix this actually ends up breaking compatibility with other gems, such as elastic/apm-agent-ruby. This blog post from New Relic explains the incompatibility in some detail. In our application, we tried to get bugsnag to load after the elastic-apm gem, but we weren't able to get it working, and our workaround for now was to use a version of the bugsnag gem earlier than v6.12.0. There's also some discussion on this issue on the elastic-apm gem's tracker.

I'm not sure what the best solution is here, as this seems to be a language issue that has brought about these incompatibilities in libraries that implement things differently.

[xljones]

Hey @danarnold, thanks for bringing this to our attention. Much like the discussions in apm-agent-ruby, we're having the similar ones here. There's going to be some incompatibilities depending on what libraries the user is using in their project, and whether they have interfering methods.

For the benefit of future readers; the Ruby gems prior to v6.12.0, will be compatible with other libraries that use alias_method chains as they won't use Module#prepend, so I would suggest continuing with this method for now is a sensible approach. --Xander