diff --git a/.changesets/do-not-report-error-causes.md b/.changesets/do-not-report-error-causes.md new file mode 100644 index 00000000..9857b3a5 --- /dev/null +++ b/.changesets/do-not-report-error-causes.md @@ -0,0 +1,24 @@ +--- +bump: minor +type: change +--- + +Do not report error causes if the wrapper error has already been reported. This deduplicates errors and prevents the error wrapper and error cause to be reported separately, as long as the error wrapper is reported first. + +```ruby +error_wrapper = nil +error_cause = nil +begin + begin + raise StandardError, "error cause" + rescue => e + error_cause = e + raise Exception, "error wrapper" + end +rescue Exception => e + error_wrapper = e +end + +Appsignal.report_error(error_wrapper) # Reports error +Appsignal.report_error(error_cause) # Doesn't report error +``` diff --git a/lib/appsignal/transaction.rb b/lib/appsignal/transaction.rb index e3dfa284..78c87c0f 100644 --- a/lib/appsignal/transaction.rb +++ b/lib/appsignal/transaction.rb @@ -177,7 +177,7 @@ def complete # In the duplicate transaction for each error, set an error # with a block that calls all the blocks set for that error # in the original transaction. - transaction.set_error(error) do + transaction.internal_set_error(error) do blocks.each { |block| block.call(transaction) } end @@ -520,17 +520,18 @@ def add_error(error, &block) return unless error return unless Appsignal.active? - _set_error(error) if @error_blocks.empty? - - if !@error_blocks.include?(error) && @error_blocks.length >= ERRORS_LIMIT - Appsignal.internal_logger.warn "Appsignal::Transaction#add_error: Transaction has more " \ - "than #{ERRORS_LIMIT} distinct errors. Only the first " \ - "#{ERRORS_LIMIT} distinct errors will be reported." + if error.instance_variable_get(:@__appsignal_error_reported) && !@error_blocks.include?(error) return end - @error_blocks[error] << block - @error_blocks[error].compact! + internal_set_error(error, &block) + + # Mark errors and their causes as tracked so we don't report duplicates, + # but also not error causes if the wrapper error is already reported. + while error + error.instance_variable_set(:@__appsignal_error_reported, true) unless error.frozen? + error = error.cause + end end alias :set_error :add_error alias_method :add_exception, :add_error @@ -592,6 +593,19 @@ def to_h attr_writer :is_duplicate, :tags, :custom_data, :breadcrumbs, :params, :session_data, :headers + def internal_set_error(error, &block) + _set_error(error) if @error_blocks.empty? + + if !@error_blocks.include?(error) && @error_blocks.length >= ERRORS_LIMIT + Appsignal.internal_logger.warn "Appsignal::Transaction#add_error: Transaction has more " \ + "than #{ERRORS_LIMIT} distinct errors. Only the first " \ + "#{ERRORS_LIMIT} distinct errors will be reported." + return + end + @error_blocks[error] << block + @error_blocks[error].compact! + end + private attr_reader :breadcrumbs diff --git a/spec/lib/appsignal/transaction_spec.rb b/spec/lib/appsignal/transaction_spec.rb index fd5ffaf1..870de13b 100644 --- a/spec/lib/appsignal/transaction_spec.rb +++ b/spec/lib/appsignal/transaction_spec.rb @@ -1568,6 +1568,43 @@ def to_s end end + context "when the error is already reported" do + it "does not add the error" do + # Report it on another transaction first + transaction1 = new_transaction + transaction1.add_error(error) + expect(transaction1).to have_error + + transaction2 = new_transaction + transaction2.add_error(error) + expect(transaction2).to_not have_error + end + end + + context "when the error cause is already reported" do + it "does not add the error" do + error = + begin + begin + raise ExampleStandardError, "error cause" + rescue + raise ExampleException, "error wrapper" + end + rescue ExampleException => e + e + end + + # Report the wrapper on another transaction first + transaction1 = new_transaction + transaction1.add_error(error) + expect(transaction1).to have_error + + transaction2 = new_transaction + transaction2.add_error(error.cause) + expect(transaction).to_not have_error + end + end + context "when a block is given" do it "stores the block in the error blocks" do block = proc { "block" }