Merge pull request #1366 from appsignal/deduplicate-error-causes

Deduplicate error wrappers and error causes

.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
+```

lib/appsignal/transaction.rb

@@ -213,7 +213,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
 
@@ -560,17 +560,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
@@ -632,6 +633,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

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" }