jgorset / facebook-messenger

Definitely the best way to make Bots on Facebook Messenger with Ruby
MIT License
962 stars 211 forks source link

Bot has been removed from the module tree but is still active. #100

Closed mijoharas closed 6 years ago

mijoharas commented 7 years ago

I've been coming across this error while trying to change the application to play nicely with autoloading in rails (it was hacked together with requires which started causing some problems).

ArgumentError (A copy of Bot::IncomingRequest::FacebookMessenger has been removed from the module tree but is still active!):

Have you got any advice on what I should do?

mijoharas commented 7 years ago

Hi.

So I dug into this a little more a while ago. Turns out that you cannot include the Bot.on commands inside a module, since the module gets unloaded by rails when it has finished executing, which calls an error when a symbol needs to get resolved inside the module after rails has unloaded it.

Do you have any suggestions to get around this? Should the Bot.on calls not be made inside a module, or is this something particular to my setup?

jgorset commented 7 years ago

Sorry for not getting around to this sooner, @mijoharas. I'm not sure I understand, though – would you provide some code or even a sample project to demonstrate this issue?

mijoharas commented 7 years ago

Hi, thanks for having a look at this. I created a minimal rails app to demonstrate the problem here.

Instructions to demonstrate the problem, first set the environment variables;

FB_MESSENGER_VERIFY_TOKEN
FB_MESSENGER_APP_SECRET
FB_MESSENGER_ACCESS_TOKEN

to your values. then start the server with rails s.

On messaging the bot it should respond with hello world.

if you then hit localhost:3000/main/index (send a get request to the index action of the main controller). the module will be unloaded. Because the my_bot is in the rails autoload path it is free to be unloaded by rails at any time after a request (which is what happens in my other larger application, this was the only reliable way to simulate it in a ssce.).

After this, subsequent requests to the bot will fail with the error message:

ArgumentError (A copy of MyBot::SomeModule::Example has been removed from the module tree but is still active!):

app/my_bot/my_bot/some_module/example.rb:6:in `block in <module:Example>'
  Rendering /home/michael/.rvm/gems/ruby-2.3.3/gems/actionpack-5.0.1/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb within rescues/layout
Error occurred while parsing request parameters.
mijoharas commented 7 years ago

This can be worked around in a few ways. firstly, you can explicitly qualify everything in the Bot.on method: in my example code:

diff --git a/app/my_bot/my_bot/some_module/example.rb b/app/my_bot/my_bot/some_module/example.rb
index 6910439..06b223e 100644
--- a/app/my_bot/my_bot/some_module/example.rb
+++ b/app/my_bot/my_bot/some_module/example.rb
@@ -3,7 +3,7 @@ module MyBot
     module Example
       include Facebook::Messenger
       Bot.on :message do |message|
-        message.reply(text: MyBot::SomeOtherModule::Example.foo)
+        message.reply(text: ::MyBot::SomeOtherModule::Example.foo)
         # message.reply(text: Something.foo)
         # message.reply(text: 'foo')
       end

Another workaround would be to make sure that we do not do the original Bot.on call inside a module: in my example that would be:

diff --git a/app/my_bot/my_bot/some_module/example.rb b/app/my_bot/my_bot/some_module/example.rb
index 556846c..43110be 100644
--- a/app/my_bot/my_bot/some_module/example.rb
+++ b/app/my_bot/my_bot/some_module/example.rb
@@ -1,12 +1,6 @@
-module MyBot
-  module SomeModule
-    module Example
-      include Facebook::Messenger
-      Bot.on :message do |message|
-        message.reply(text: MyBot::SomeOtherModule::OtherExample.foo)
-        # message.reply(text: Something.foo)
-        # message.reply(text: 'foo')
-      end
-    end
-  end
+include Facebook::Messenger
+Bot.on :message do |message|
+  message.reply(text: MyBot::SomeOtherModule::OtherExample.foo)
+  # message.reply(text: Something.foo)
+  # message.reply(text: 'foo')
 end
mijoharas commented 7 years ago

Note: my method of using Object.remove_const(MyBot) to simulate the rails unloading is going at this with a hammer to break it, but I couldn't figure out the right way of making rails unload the module as it does in my proper app in this small self-contained example.

mijoharas commented 7 years ago

Also, what throws the error is when Rails encounters a const_missing and then searches from the parent module (which has been unloaded by this point) to find where the const might be defined, c.f: https://github.com/rails/rails/blob/master/activesupport/lib/active_support/dependencies.rb#L493-L496

    def load_missing_constant(from_mod, const_name)
      unless qualified_const_defined?(from_mod.name) && Inflector.constantize(from_mod.name).equal?(from_mod)
        raise ArgumentError, "A copy of #{from_mod} has been removed from the module tree but is still active!"
      end
jgorset commented 7 years ago

Thanks for the thorough explanation, @mijoharas! That's a bummer. I think the approach you outlined here is the only way to make it work, but frankly I'm wondering if we should remove the bit about running it with Rails from the documentation. It just seems very easy to get it wrong, and it seems to me that it's usually a better idea to run the bot on its own.

To put that theory to the test, would it work for your use-case?

mijoharas commented 7 years ago

I'm sure we could have made it work somehow with an exclusively rack-based approach, the application we were building started in rails though, so there was a lot less friction in using the existing framework. The example in the documentation does work fine as long as you do not perform the Bot.on messages within a module.

One thing that confuses me, is that IIRC in general incoming requests in rails manage to make sure that some classes are ready loaded when the request comes in and are ready to respond to it. If we can figure out how to hook into that system and say "hey, if something comes in to this <url>/bot route then make sure that this module containing my Bot.on calls is loaded and ready to take the requests" then we'd have a solid rails solution.

mijoharas commented 6 years ago

didn't realise this was still open, we mitigated this by making sure we didn't call any of the Bot.on hooks while inside a module as it doesn't play well with rails autoload reloading of classes in development. Closing issue.