rollbar-gem icon indicating copy to clipboard operation
rollbar-gem copied to clipboard

Undefined method `params' for #<Hash>

Open severin opened this issue 1 year ago • 8 comments

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:

Screenshot 2024-07-03 at 10 27 25

severin avatar Jul 03 '24 08:07 severin

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 avatar Aug 26 '24 18:08 waltjones

@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

severin avatar Aug 28 '24 14:08 severin

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 avatar Jun 05 '25 20:06 cenon-delrosario

@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.

brianr avatar Jun 05 '25 20:06 brianr

Hi, Thank you for looking into it 👍

cenon-delrosario avatar Jun 05 '25 21:06 cenon-delrosario

@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?

brianr avatar Jun 06 '25 16:06 brianr

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.

cenon-delrosario avatar Jun 06 '25 21:06 cenon-delrosario

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.

waltjones avatar Jul 03 '25 18:07 waltjones

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.

We ended up using version 3.4. Thank you very much for the assistance :)

cenon-delrosario avatar Jul 03 '25 20:07 cenon-delrosario

@cenon-delrosario, good to hear, and thank you for the update!

waltjones avatar Jul 03 '25 21:07 waltjones