getsentry / sentry-ruby

Sentry SDK for Ruby
https://sentry.io/for/ruby
MIT License
928 stars 494 forks source link

Injecting integrations implicitly depends on gem load order #235

Closed nfm closed 9 years ago

nfm commented 9 years ago

Raven.inject checks if a number of other constants are defined before requiring the integation files. While this is necessary, it adds an implicit dependency on the order of gems in your Gemfile.

I discovered we weren't logging exceptions from Delayed::Job failures, and tracked it down to the fact that we're requiring sentry-raven in our Gemfile before delayed_job. As a result, Delayed::Plugin isn't defined by the time Raven.inject is called.

I'm not sure what the best thing to do about this is, maybe just document this requirement.

As a workaround, I copied the require into my Rails initializer for Raven:

# config/initializers/raven.rb
Raven.configure do |config|
  ...
end
require 'raven/integrations/delayed_job' if defined?(::Delayed::Plugin)

All the gems will have been loaded by the time Rails starts running initializers, so that workaround does the trick, regardless of the order of gems in your Gemfile.

I'd be in favour of not implicitly injecting requires, and documenting that you need to do so in your initializer, but that would be a breaking change.

Anyway, food for thought, and just thought I'd document this here in case anyone else has the same problem.

nateberkopec commented 9 years ago

I've been struggling with this issue when it comes to #217 and have yet to come up with a satisfactory API. This implicit-load-order-API goes back a long way and I still haven't found a satisfactory solution that:

nateberkopec commented 9 years ago

@nfm Can you try #236?

nfm commented 9 years ago

@nateberkopec Just tried #236, and that worked for me with Delayed::Job. Didn't hit any of the other integrations though.

I'm not really familiar with any Gem or Bundler internals so I can't offer much more feedback on the patch. Looks like an interesting approach though!