Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sanfrecce-osaka
Copy link
Contributor

@sanfrecce-osaka sanfrecce-osaka commented May 24, 2025

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

…vel` when using ActiveSupport::ErrorReporter

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/blame/5.24.0/sentry-ruby/lib/sentry/event.rb#L100-L102
@solnic
Copy link
Collaborator

solnic commented May 28, 2025

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 solnic self-requested a review May 28, 2025 12:34
@sanfrecce-osaka
Copy link
Contributor Author

sanfrecce-osaka commented Jun 15, 2025

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 rails/rails#43625 (comment) 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

@solnic
Copy link
Collaborator

solnic commented Jun 16, 2025

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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants