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

feat(sentry-rails): Allow `severity` to pass what is available at `level` when using ActiveSupport::ErrorReporter

Open sanfrecce-osaka opened this issue 7 months ago • 3 comments

Description

The following are available for level in sentry-rails.

  • :fatal
  • :error
  • :warning
  • :log
  • :info
  • :debug

cf. https://docs.sentry.io/platforms/ruby/guides/rails/usage/set-level/

But in the case of severity in ActiveSupport::ErrorReporter#report, ArgumentError is raised if anything other than the following is passed.

  • :error
  • :warning
  • :info

cf. https://github.com/rails/rails/blob/v8.0.2/activesupport/lib/active_support/error_reporter.rb#L220-L222 cf. https://github.com/rails/rails/blob/v8.0.2/activesupport/lib/active_support/error_reporter.rb#L27

So I added symbols to ActiveSupport::ErrorReporter::SEVERITIES that is allowed as level when registering Sentry::Rails::ErrorSubscriber.

It also supports the behavior that warn is considered as warning.

cf. https://github.com/getsentry/sentry-ruby/blob/5.24.0/sentry-ruby/lib/sentry/event.rb#L100-L102

sanfrecce-osaka avatar May 24 '25 07:05 sanfrecce-osaka

Thank you for the PR - one question though: how does it happen that you end up with the ArgumentError? Is there a place in the SDK where the wrong level is used or is it about usage of the SDK that it allows passing invalid levels?

I need to understand better what the actual problem is.

solnic avatar May 28 '25 12:05 solnic

Thank you for the PR - one question though: how does it happen that you end up with the ArgumentError? Is there a place in the SDK where the wrong level is used or is it about usage of the SDK that it allows passing invalid levels?

I need to understand better what the actual problem is.

Thank you for the reply. ActiveSupport::ErrorReporter#report checks severity parameter before calling subscriber's report. If anything other than error, warning, or info is passed, ArgumentError is raised. In other words, ArgumentError will be raised before the SDK implemented report is called.

cf. https://github.com/rails/rails/blame/v8.0.2/activesupport/lib/active_support/error_reporter.rb#L216-L229

The severity parameter was discussed at https://github.com/rails/rails/pull/43625#issuecomment-966173208 where ActiveSupport::ErrorReporter was introduced. From the discussion in the pull request, I think three options. What do you think?

  • Change to pass level via context parameter
  • Propose API to change severity restriction to Rails
  • Propose to remove severity restriction to Rails

sanfrecce-osaka avatar Jun 15 '25 05:06 sanfrecce-osaka

OK but this means that it's outside of the SDK control, right? It's not the SDK that calls Rails.error.report with incorrect severity leading to the ArgumentError.

solnic avatar Jun 16 '25 09:06 solnic