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

Removing ActionDispatch::RemoteIp middleware crashes application when using AppSignal #155

Closed
tombruijn opened this issue Sep 16, 2016 · 5 comments

Comments

@tombruijn
Copy link
Member

tombruijn commented Sep 16, 2016

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 tombruijn added the bug label Sep 16, 2016
@tombruijn tombruijn modified the milestones: 2.3, 2.4 Jun 13, 2017
@tombruijn
Copy link
Member Author

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

@tombruijn
Copy link
Member Author

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 tombruijn changed the title Removing ActionDispatch::RemoteIp middleware crashes application when using AppSignal 🐞 Removing ActionDispatch::RemoteIp middleware crashes application when using AppSignal Oct 27, 2017
@thijsc
Copy link
Member

thijsc commented Dec 5, 2017

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 thijsc closed this as completed Dec 5, 2017
@tombruijn tombruijn reopened this Dec 7, 2017
@tombruijn
Copy link
Member Author

tombruijn commented Dec 7, 2017

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 tombruijn changed the title 🐞 Removing ActionDispatch::RemoteIp middleware crashes application when using AppSignal Removing ActionDispatch::RemoteIp middleware crashes application when using AppSignal May 30, 2018
@tombruijn
Copy link
Member Author

Will probably be fixed with #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 tombruijn removed this from the The next minor Ruby gem version milestone Jan 7, 2019
@jeffkreeftmeijer jeffkreeftmeijer closed this as not planned Won't fix, can't repro, duplicate, stale Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants