diff --git a/.changeset/rails-excluded-exceptions-chain.md b/.changeset/rails-excluded-exceptions-chain.md new file mode 100644 index 00000000..9019afa2 --- /dev/null +++ b/.changeset/rails-excluded-exceptions-chain.md @@ -0,0 +1,5 @@ +--- +"posthog-rails": minor +--- + +Excluded exceptions now also match subclasses of a listed class and any exception whose cause chain contains one, for both `excluded_exceptions` and the built-in defaults. For example an error raised while rescuing `ActiveRecord::RecordNotFound` is no longer captured. ActiveJob captures now respect the exclusion list too. diff --git a/posthog-rails/examples/posthog.rb b/posthog-rails/examples/posthog.rb index fc0b3515..b7290a93 100644 --- a/posthog-rails/examples/posthog.rb +++ b/posthog-rails/examples/posthog.rb @@ -48,7 +48,8 @@ # config.current_user_resolver = proc { |controller| controller.current_user } # Additional exception classes to exclude from reporting - # These are added to the default excluded exceptions + # These are added to the default excluded exceptions. Subclasses of a listed + # class are excluded too, and so is any exception whose cause chain contains one. # config.excluded_exceptions = [ # # 'MyCustom404Error', # # 'MyCustomValidationError' diff --git a/posthog-rails/lib/generators/posthog/templates/posthog.rb b/posthog-rails/lib/generators/posthog/templates/posthog.rb index b67573a3..03ccc2e8 100644 --- a/posthog-rails/lib/generators/posthog/templates/posthog.rb +++ b/posthog-rails/lib/generators/posthog/templates/posthog.rb @@ -49,7 +49,8 @@ # config.current_user_resolver = proc { |controller| controller.current_user } # Additional exception classes to exclude from reporting - # These are added to the default excluded exceptions + # These are added to the default excluded exceptions. Subclasses of a listed + # class are excluded too, and so is any exception whose cause chain contains one. # config.excluded_exceptions = [ # # 'MyCustom404Error', # # 'MyCustomValidationError' diff --git a/posthog-rails/lib/posthog/rails/active_job.rb b/posthog-rails/lib/posthog/rails/active_job.rb index 4a7f78fd..8b00f86d 100644 --- a/posthog-rails/lib/posthog/rails/active_job.rb +++ b/posthog-rails/lib/posthog/rails/active_job.rb @@ -178,7 +178,6 @@ def capture_posthog_retry(exception) config = PostHog::Rails.config return unless config&.auto_instrument_active_job && config.report_active_job_retries && exception return unless PostHog.initialized? && PostHog.client.enabled? - return if excluded_posthog_retry?(exception, config) Internal::Context.with_context( @posthog_execution_context || posthog_job_context, fresh: true, enrich_current_span: false @@ -189,20 +188,9 @@ def capture_posthog_retry(exception) PostHog::Logging.logger.error("Failed to capture job retry: #{e.class}") end - def excluded_posthog_retry?(exception, config) - seen = {}.compare_by_identity - current = exception - while current && seen.size < PostHog::ExceptionCapture::MAX_CHAINED_EXCEPTIONS && !seen.key?(current) - return true unless config.should_capture_exception?(current) - - seen[current] = true - current = current.respond_to?(:cause) ? current.cause : nil - end - false - end - def capture_job_exception(exception, retry_failure: false) return unless PostHog::Rails.config&.auto_instrument_active_job + return unless PostHog::Rails.config&.should_capture_exception?(exception) # Build distinct_id from job arguments if possible distinct_id = if @posthog_execution_context diff --git a/posthog-rails/lib/posthog/rails/configuration.rb b/posthog-rails/lib/posthog/rails/configuration.rb index 2b3bd025..090d3d8c 100644 --- a/posthog-rails/lib/posthog/rails/configuration.rb +++ b/posthog-rails/lib/posthog/rails/configuration.rb @@ -4,6 +4,8 @@ # Copyright (c) 2020 Sentry # Licensed under the MIT License: https://github.com/getsentry/sentry-ruby/blob/master/LICENSE +require 'posthog/exception_capture' + module PostHog module Rails class Configuration @@ -23,7 +25,8 @@ class Configuration # Defaults to false. attr_accessor :report_active_job_retries - # @return [Array] Exception class names to ignore in addition to the defaults. + # @return [Array] Exception class names to ignore in addition to the defaults. Subclasses are + # ignored too, and so is any exception whose cause chain contains a listed class. attr_accessor :excluded_exceptions # @return [Boolean] Whether to use PostHog tracing headers for request-scoped identity/session context. @@ -117,8 +120,10 @@ def default_excluded_exceptions # @param exception [Exception] The exception to check. # @return [Boolean] Whether the exception should be captured. def should_capture_exception?(exception) - exception_name = exception.class.name - !all_excluded_exceptions.include?(exception_name) + excluded = all_excluded_exceptions + exception_chain(exception).none? do |error| + error.class.ancestors.any? { |ancestor| excluded.include?(ancestor.name) } + end end private @@ -126,6 +131,15 @@ def should_capture_exception?(exception) def all_excluded_exceptions default_excluded_exceptions + excluded_exceptions end + + def exception_chain(exception) + chain = [] + while exception.is_a?(Exception) && chain.size < ExceptionCapture::MAX_CHAINED_EXCEPTIONS + chain << exception + exception = exception.cause + end + chain + end end end end diff --git a/spec/posthog/rails/configuration_spec.rb b/spec/posthog/rails/configuration_spec.rb index b7064bcc..41201b10 100644 --- a/spec/posthog/rails/configuration_spec.rb +++ b/spec/posthog/rails/configuration_spec.rb @@ -38,6 +38,20 @@ # defaults still apply expect(config.should_capture_exception?(ActionController::RoutingError.new('x'))).to be false end + + it 'excludes subclasses of an excluded class' do + config.excluded_exceptions = ['MyApp::IgnorableError'] + stub_const('MyApp::IgnorableError', Class.new(StandardError)) + expect(config.should_capture_exception?(Class.new(MyApp::IgnorableError).new)).to be false + end + + it 'excludes an exception whose cause chain contains an excluded class' do + config.excluded_exceptions = ['MyApp::IgnorableError'] + stub_const('MyApp::IgnorableError', Class.new(StandardError)) + wrapper = StandardError.new('retrying') + wrapper.define_singleton_method(:cause) { MyApp::IgnorableError.new('over limit') } + expect(config.should_capture_exception?(wrapper)).to be false + end end describe 'user context defaults' do diff --git a/spec/posthog/rails/exception_mechanism_spec.rb b/spec/posthog/rails/exception_mechanism_spec.rb index 59415c67..d6a1dc40 100644 --- a/spec/posthog/rails/exception_mechanism_spec.rb +++ b/spec/posthog/rails/exception_mechanism_spec.rb @@ -376,6 +376,15 @@ def perform_now ) end + it 'skips job exceptions listed in excluded_exceptions' do + PostHog::Rails.config.auto_instrument_active_job = true + PostHog::Rails.config.excluded_exceptions = ['StandardError'] + + expect { job_class.new.perform_now }.to raise_error(StandardError, 'job failed') + + expect(PostHog).not_to have_received(:capture_exception) + end + it 'prevents Rails error subscriber from capturing the same job exception again' do PostHog::Rails.config.auto_instrument_active_job = true error = nil