feat(sentry-rails): Allow `severity` to pass what is available at `level` when using ActiveSupport::ErrorReporter
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
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 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
levelvia context parameter - Propose API to change severity restriction to Rails
- Propose to remove severity restriction to Rails
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.