appsignal-ruby icon indicating copy to clipboard operation
appsignal-ruby copied to clipboard

Removing ActionDispatch::RemoteIp middleware crashes application when using AppSignal

Open tombruijn opened this issue 9 years ago • 5 comments

Adding the following to your application crashes makes appsignal crash your application without much explanation why.

# config/application.rb
config.middleware.delete ActionDispatch::RemoteIp
/Users/tom/.gem/ruby/2.3.1/gems/actionpack-4.2.5/lib/action_dispatch/middleware/stack.rb:125:in `assert_index': No such middleware to insert before: ActionDispatch::RemoteIp (RuntimeError)

What we should do is:

  1. check if the middleware is present before adding ourself to the middleware stack based on it.
  2. have fallbacks on other middleware or find another way to load middleware in the right place in the stack

Maybe pick up with #159? If we instrument all the middleware and are at the very front of the stack this isn't an issue.

As reported in: https://app.intercom.io/a/apps/yzor8gyw/inbox/conversation/5983896759

tombruijn avatar Sep 16 '16 07:09 tombruijn

This has been changed to the ActionDispatch::DebugExceptions middleware in #286. Same problem there. If you remove that middleware the same problem occurs.

tombruijn avatar Jun 13 '17 13:06 tombruijn

Timeboxed in an hour: look if we can detect which middleware is present at the time we add our middleware to the proxy. If not, close the issue and this becomes a requirement for AppSignal to work in Rails.

tombruijn avatar Aug 11 '17 07:08 tombruijn

We tried to reproduce this and could not with the latest appsignal gem and Rails 5. We now insert the middleware after ActionDispatch::DebugExceptions. We weren't able to break the insertion.

thijsc avatar Dec 05 '17 10:12 thijsc

Correct, the middleware we depend on now is ActionDispatch::DebugExceptions. Same issue exists when removing that middleware. This issue was from before that.

As discussed we should add a config option insert_after_rails_middleware or something, so that users can configure which middleware it should be added after themselves.

As issue week is already closed I'll move it to longterm.

tombruijn avatar Dec 07 '17 10:12 tombruijn

Will probably be fixed with https://github.com/appsignal/appsignal-ruby/issues/159 where we will be at the top of the middleware stack so that we don't depend on other middleware to be injected at a specific location in the stack

tombruijn avatar Jan 07 '19 14:01 tombruijn