rollbar-gem
rollbar-gem copied to clipboard
Undefined method `params' for #<Hash>
Hi there,
Looking at our NewRelic errors dashboard I notice quite a lot of exceptions that we don't see in Rollbar. Digging a bit deeper they are all raised from requests with non-existent paths that raise a RoutingError.
The relevant code is here: https://github.com/rollbar/rollbar-gem/blob/master/lib/rollbar/middleware/rails/show_exceptions.rb#L10
I'm not too familiar with how rails integrates with rack but to me this code looks intuitively wrong: env is supposed to be a hash and so that line should probably be:
key = 'action_dispatch.show_detailed_exceptions'
if exception.is_a?(ActionController::RoutingError) && env[key]
scope = ...
(The to_s call on key is also not necessary, since key is always a String)
Here is more context from NewRelic:
env is expected to be a Rack::Request instance, and the method to call [] directly on it has been removed.
https://github.com/rack/rack/blob/v3.0.8/lib/rack/request.rb#L608-L613
What is the situation that leads to having a hash here instead?
@waltjones I dug a bit into it: We are still using Rails 4.2, and the signature of the render_exception method has changed in Rails 5.0: https://github.com/rails/rails/commit/9503e65b9718e4f01860cf017c1cdcdccdfffde7
We are still on Rails 3.2.22 and we just upgraded to the latest version (3.6.2) and are now seeing a lot of this error in NewRelic. The problem has been reported for some time so would appreciate it if someone can offer at least a workaround or get this fixed soon.
We also cannot revert back to an older version of this gem because it causes a different problem.
Thank you.
@cenon-delrosario thank you for the nudge. We will review this and follow up this week with at least a timeline if not a fix.
Hi, Thank you for looking into it 👍
@cenon-delrosario, looking into this, here's what we've found:
- The latest version (3.6.2) of rollbar-gem is tested with rails 5+ and ruby 2.5+.
- The most recent version of rollbar-gem that is tested with rails 3 is rollbar-gem 3.4.0.
- The code change that introduced this specific issue is https://github.com/rollbar/rollbar-gem/pull/1142 , which removed a deprecation warning in versions of rack that are no longer supported.
We are reticent to introduce code that makes rollbar-gem work with old rack in this one code path, because it wouldn't be tested by CI.
A few ideas on how to solve this:
- For a workaround within the current version: . You could monkeypatch to revert the change in #1142
- Try if version 3.4.0 solves your problem
- You mentioned another issue with the prior version you used. What is that issue?
Hi @brianr,
Not sure but the Rack specification here https://github.com/rack/rack/blob/main/SPEC.rdoc#label-The+Request+Environment indicates that env is expected to be a Hash and there are parts in https://github.com/rollbar/rollbar-gem/blob/master/lib/rollbar/middleware/rails/show_exceptions.rb#L10 that still use [] unless those parts are meant to be called in a different context?
In reference to You mentioned another issue with the prior version you used. What is that issue?
We were on an older version of Rollbar (version 2.8.1) and had to upgrade due to upgrading Ruby from 2.2 to 2.3.1. After the Ruby upgrade we started getting this issue https://github.com/rollbar/rollbar-gem/issues/545 which prompted us to upgrade Rollbar.
I guess we can try version 3.4 for now and if not try the monkey patch solution.
Hi @cenon-delrosario , render_exception is called with a Rack::Request instance rather than a hash, and the [] method is implemented on the instance for back compatibility with a deprecation warning:
https://github.com/rack/rack/blob/v3.0.8/lib/rack/request.rb#L608-L613
See here for the render_exception call signature and use of the request instance.
https://apidock.com/rails/v5.2.3/ActionDispatch/ShowExceptions/render_exception
there are parts that still use
[]
This is an oversight and those should be updated. The intent is for people to not get the Rack deprecation warning when using rollbar-gem.
Hi @cenon-delrosario ,
render_exceptionis called with aRack::Requestinstance rather than a hash, and the[]method is implemented on the instance for back compatibility with a deprecation warning: https://github.com/rack/rack/blob/v3.0.8/lib/rack/request.rb#L608-L613See here for the
render_exceptioncall signature and use of the request instance. https://apidock.com/rails/v5.2.3/ActionDispatch/ShowExceptions/render_exceptionthere are parts that still use
[]This is an oversight and those should be updated. The intent is for people to not get the Rack deprecation warning when using rollbar-gem.
We ended up using version 3.4. Thank you very much for the assistance :)
@cenon-delrosario, good to hear, and thank you for the update!