From a1875895a97f86674cb623972216f8400def3af9 Mon Sep 17 00:00:00 2001
From: Noemi <45180344+unflxw@users.noreply.github.com>
Date: Fri, 26 Jul 2024 11:50:18 +0200
Subject: [PATCH] Keep the first ten errors

Before this change, the last ten additional errors would be kept,
alongside with the first (non-additional) error. This was a strange
mix of behaviours that would have been hard to document. Instead,
we can now document that we keep the first ten errors.

The language emitted in the warning has been modified to avoid any
mention of "additional" errors.
---
 lib/appsignal/transaction.rb           | 17 +++++++------
 spec/lib/appsignal/transaction_spec.rb | 33 ++++++++++++++++++++++++--
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/lib/appsignal/transaction.rb b/lib/appsignal/transaction.rb
index 615a86b99..94c988cff 100644
--- a/lib/appsignal/transaction.rb
+++ b/lib/appsignal/transaction.rb
@@ -20,7 +20,7 @@ class Transaction
     BREADCRUMB_LIMIT = 20
     # @api private
     ERROR_CAUSES_LIMIT = 10
-    ADDITIONAL_ERRORS_LIMIT = 10
+    ERRORS_LIMIT = 10
 
     class << self
       # Create a new transaction and set it as the currently active
@@ -469,13 +469,16 @@ def add_error(error)
 
       return _set_error(error) unless @has_error
 
-      @errors << error
-      return unless @errors.length > ADDITIONAL_ERRORS_LIMIT
+      # Subtract one from the limit to account for the first error,
+      # which is not stored in the errors array.
+      if @errors.length >= (ERRORS_LIMIT - 1)
+        Appsignal.internal_logger.warn "Appsignal::Transaction#add_error: Transaction has more " \
+          "than #{ERRORS_LIMIT} errors. Only the first " \
+          "#{ERRORS_LIMIT} errors will be reported."
+        return
+      end
 
-      Appsignal.internal_logger.debug "Appsignal::Transaction#add_error: Transaction has more " \
-        "than #{ADDITIONAL_ERRORS_LIMIT} additional errors. Only the last " \
-        "#{ADDITIONAL_ERRORS_LIMIT} will be reported."
-      @errors = @errors.last(ADDITIONAL_ERRORS_LIMIT)
+      @errors << error
     end
 
     alias :set_error :add_error
diff --git a/spec/lib/appsignal/transaction_spec.rb b/spec/lib/appsignal/transaction_spec.rb
index dab345146..55c5609d8 100644
--- a/spec/lib/appsignal/transaction_spec.rb
+++ b/spec/lib/appsignal/transaction_spec.rb
@@ -1323,7 +1323,7 @@ def to_s
         )
       end
 
-      it "does not store the error in the errors array" do
+      it "does not store the error in the additional errors array" do
         transaction.add_error(error)
 
         expect(transaction.errors).to be_empty
@@ -1339,7 +1339,7 @@ def to_s
 
       before { transaction.set_error(other_error) }
 
-      it "stores an error in the errors array" do
+      it "stores an error in the additional errors array" do
         transaction.add_error(error)
 
         expect(transaction.errors).to eq([error])
@@ -1355,6 +1355,35 @@ def to_s
         )
       end
     end
+
+    context "when the additional errors array is at the limit" do
+      before do
+        10.times do |i|
+          transaction.add_error(ExampleStandardError.new("error #{i}"))
+        end
+      end
+
+      it "does not add the error to the additional errors array" do
+        expect(transaction).to have_error("ExampleStandardError", "error 0", [])
+        expect(transaction.errors.length).to eq(9)
+        expected_errors = transaction.errors.dup
+
+        transaction.add_error(error)
+
+        expect(transaction).to have_error("ExampleStandardError", "error 0", [])
+        expect(transaction.errors).to eq(expected_errors)
+      end
+
+      it "logs a debug message" do
+        logs = capture_logs { transaction.add_error(error) }
+
+        expect(logs).to contains_log(
+          :warn,
+          "Appsignal::Transaction#add_error: Transaction has more than 10 errors. " \
+            "Only the first 10 errors will be reported."
+        )
+      end
+    end
   end
 
   describe "#_set_error" do