Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reset the notifier when we use Rollbar.preconfigure #378

Merged
merged 1 commit into from
Feb 4, 2016

Conversation

jondeandres
Copy link
Contributor

This fix the problem we had with gems using Thread.new before our
railtie is loaded.

The order to reproduce the bug was:

  1. Load Rollbar gem and the Thread monkey patch

  2. Rollbar.configure is executed

  3. Another gem is loaded and executes Thread.new

    3.1 This sets Rollbar.notifier to the current configuration

  4. Our railtie is loaded

    4.1 The railtie uses preconfigure that changes Rollbar.configuration
    4.2 Because we already have a Rollbar.notifier, the new configuration
    is not propagated

So now, on Rollbar.preconfigure we reset the notifier, so for next
time it's used it will take the last configuration, included the railtie one.

This fix the problem we had with gems using `Thread.new` before our
railtie is loaded.

The order to reproduce the bug was:

1 Load Rollbar gem and the Thread monkey patch
2 Rollbar.configure is executed
3 Another gem is loaded and executes Thread.new
 3.1 This sets Rollbar.notifier to the current configuration
4 Our railtie is loaded
 4.1 The railtie uses preconfigure that changes Rollbar.configuration
 4.2 Because we already have a Rollbar.notifier, the new configuration
      is not propagated

So now, on `Rollbar.preconfigure` we reset the notifier, so for next
time it's used it will take the last configuration, included the railtie one.
@brianr
Copy link
Member

brianr commented Feb 3, 2016

👍

jondeandres added a commit that referenced this pull request Feb 4, 2016
Reset the notifier when we use Rollbar.preconfigure
@jondeandres jondeandres merged commit 06a11e3 into master Feb 4, 2016
@jondeandres jondeandres deleted the jon/5153/fix-thread-issues branch February 4, 2016 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants