fix reset notifier on rails initialize

[nikolai-b]

Hi,

If an initializer calls Thread.new before Rollbar is initialized then the notifier doesn't clone the correct configuration. I've added a spec which can be called using
RESET=1 rake spec
because in the spec helper is calling Rollbar.reset_notifier! already.

[jondeandres]

@nikolai-b thanks for the PR, it looks great.

But I'd like to know exactly the behaviour you are having and the expected behaviour. This fix solves the problem only for Rails apps, but not for ruby applications. If you detail more the problem we can find a solution for all the use cases.

[nikolai-b]

The problem is when Thread.new is called before threads have been overloaded. If you comment out the fix then in the dummy app console try Rollbar.error 'test' you'll see that Rollbar is disabled.

Yep, this probably should be done somewhere like in the require hooks instead.

[jondeandres]

Nikolai,

we'll take a look into this soon. Thank you for the PR and find the bug :-D.

On Tue, Nov 11, 2014 at 4:49 PM, Nikolai notifications@github.com wrote:

The problem is when Thread.new is called before threads have been
overloaded
https://github.com/rollbar/rollbar-gem/blob/096c8fa360f8e094d0ebc9bb81cefd82422a3660/lib/rollbar/core_ext/thread.rb.
If you comment out the fix
https://github.com/nikolai-b/rollbar-gem/blob/master/lib/rollbar/rails.rb#L18
then in the dummy app console try Rollbar.error 'test' you'll see that
Rollbar is disabled.

Yep, this probably should be done somewhere like in the require hooks
instead
https://github.com/rollbar/rollbar-gem/blob/master/lib/rollbar.rb#L683.

—
Reply to this email directly or view it on GitHub
#168 (comment).

Jon

[nikolai-b]

Sorry, realized my comment above wasn't clear so I'll try again 😄

So you overloaded threads to create a new notifier but if Thread.new is called before Rollbar is configured then that notifier also won't be configured.

[jondeandres]

@nikolai-b I've tried to reproduce the problem you are having, without success.

I've executed Thread.new {} before rollbar is loaded and configured. After my initializer executes Rollbar.configure then Rollbar is configured and enabled.

Could you please provide us a full example of use?

If you execute Rollbar.configure then https://github.com/rollbar/rollbar-gem/blob/master/lib/rollbar.rb#L66 is executed. Only if Rollbar::Configuration#enabled at that point then Rollbar will be disabled, but I can't understand how it could return false.

[nikolai-b]

Its in the spec, let me know if that doesn't help

[jondeandres]

@nikolai-b I've reproduced the error :-D.

Well, in order to fix this for all ruby apps probably we'll need to require our Thread extension once Rollbar is configured. I'll make a fix for this soon.

Thanks for the bug report @nikolai-b, it's been really appreciated.

[jondeandres]

@nikolai-b I've used some ideas from you in #170, take a look if you want. I think we can close this PR then.

[nikolai-b]

yes, nice, sorry mine was a little rough, ran out of time

[brianr]

Thanks again for finding and debugging this, @nikolai-b. The fix for this (#170) will go out in the next release (1.2.8).

[brianr]

Released in 1.2.8