Skip to content

Commit

Permalink
Keep the first ten errors
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
unflxw committed Jul 26, 2024
1 parent 17123e2 commit a187589
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 9 deletions.
17 changes: 10 additions & 7 deletions lib/appsignal/transaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
33 changes: 31 additions & 2 deletions spec/lib/appsignal/transaction_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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])
Expand All @@ -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
Expand Down

0 comments on commit a187589

Please sign in to comment.