Skip to content

Commit b51343f

Browse files
committed
Improve formatter efficiency and MultiErrorReporter customization
1 parent 0651e9e commit b51343f

File tree

6 files changed

+206
-39
lines changed

6 files changed

+206
-39
lines changed

lib/log_struct/formatter.rb

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,7 @@ def process_values(arg, recursion_depth: 0)
7373

7474
result
7575
when Array
76-
result = arg.map { |value| process_values(value, recursion_depth: recursion_depth + 1) }
77-
78-
# Filter large arrays, but don't truncate backtraces (arrays of strings that look like file:line)
79-
if result.size > 10 && !looks_like_backtrace?(result)
80-
result = result.take(10) + ["... and #{result.size - 10} more items"]
81-
end
82-
result
76+
process_array(arg, recursion_depth: recursion_depth)
8377
when GlobalID::Identification
8478
begin
8579
arg.to_global_id
@@ -91,7 +85,7 @@ def process_values(arg, recursion_depth: 0)
9185
else
9286
# For non-ActiveRecord objects that failed to_global_id, try to get a string representation
9387
# If this also fails, we want to catch it and return the error placeholder
94-
T.unsafe(arg).to_s
88+
String(T.cast(arg, Object))
9589
end
9690
rescue => e
9791
LogStruct.handle_exception(e, source: Source::Internal)
@@ -207,17 +201,35 @@ def generate_json(data)
207201
"#{data.to_json}\n"
208202
end
209203

204+
sig { params(array: T::Array[T.untyped], recursion_depth: Integer).returns(T::Array[T.untyped]) }
205+
def process_array(array, recursion_depth:)
206+
return [] if array.empty?
207+
208+
if looks_like_backtrace_array?(array)
209+
array.map { |value| process_values(value, recursion_depth: recursion_depth + 1) }
210+
else
211+
processed = []
212+
array.each_with_index do |value, index|
213+
break if index >= 10
214+
215+
processed << process_values(value, recursion_depth: recursion_depth + 1)
216+
end
217+
218+
if array.size > 10
219+
processed << "... and #{array.size - 10} more items"
220+
end
221+
222+
processed
223+
end
224+
end
225+
210226
# Check if an array looks like a backtrace (array of strings with file:line pattern)
211227
sig { params(array: T::Array[T.untyped]).returns(T::Boolean) }
212-
def looks_like_backtrace?(array)
213-
return false if array.empty?
214-
215-
# Check if most elements look like backtrace lines (file.rb:123 or similar patterns)
228+
def looks_like_backtrace_array?(array)
216229
backtrace_like_count = array.first(5).count do |element|
217230
element.is_a?(String) && element.match?(/\A[^:\s]+:\d+/)
218231
end
219232

220-
# If at least 3 out of the first 5 elements look like backtrace lines, treat as backtrace
221233
backtrace_like_count >= 3
222234
end
223235
end

lib/log_struct/log/shared/serialize_common_public.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ def serialize_common_public(strict = true)
2020
raise ArgumentError, "Public log struct must define #source and #event"
2121
end
2222

23-
src_val = T.unsafe(self).source
24-
evt_val = T.unsafe(self).event
25-
src = src_val.respond_to?(:serialize) ? T.unsafe(src_val).serialize.to_s : src_val.to_s
26-
evt = evt_val.respond_to?(:serialize) ? T.unsafe(evt_val).serialize.to_s : evt_val.to_s
23+
src_val = public_send(:source)
24+
evt_val = public_send(:event)
25+
src = src_val.respond_to?(:serialize) ? src_val.public_send(:serialize).to_s : src_val.to_s
26+
evt = evt_val.respond_to?(:serialize) ? evt_val.public_send(:serialize).to_s : evt_val.to_s
2727
lvl = level.serialize.to_s
2828
ts = timestamp.iso8601(3)
2929

lib/log_struct/multi_error_reporter.rb

Lines changed: 79 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
# frozen_string_literal: true
33

44
require_relative "enums/error_reporter"
5+
require_relative "handlers"
56

67
# Try to require all supported error reporting libraries
78
# Users may have multiple installed, so we should load all of them
@@ -19,33 +20,62 @@ module LogStruct
1920
# but the operation should be allowed to continue (e.g. scrubbing log data.)
2021
class MultiErrorReporter
2122
# Class variable to store the selected reporter
22-
@reporter = T.let(nil, T.nilable(ErrorReporter))
23+
class CallableReporterWrapper
24+
extend T::Sig
25+
26+
sig { params(callable: T.untyped).void }
27+
def initialize(callable)
28+
@callable = callable
29+
end
30+
31+
sig { returns(T.untyped) }
32+
attr_reader :callable
33+
alias_method :original, :callable
34+
35+
sig { params(error: StandardError, context: T.nilable(T::Hash[Symbol, T.untyped]), source: Source).void }
36+
def call(error, context, source)
37+
case callable_arity
38+
when 3
39+
callable.call(error, context, source)
40+
when 2
41+
callable.call(error, context)
42+
when 1
43+
callable.call(error)
44+
else
45+
callable.call(error, context, source)
46+
end
47+
end
48+
49+
private
50+
51+
sig { returns(Integer) }
52+
def callable_arity
53+
callable.respond_to?(:arity) ? callable.arity : -1
54+
end
55+
end
56+
57+
ReporterImpl = T.type_alias { T.any(ErrorReporter, CallableReporterWrapper) }
58+
59+
@reporter_impl = T.let(nil, T.nilable(ReporterImpl))
2360

2461
class << self
2562
extend T::Sig
2663

27-
sig { returns(ErrorReporter) }
64+
sig { returns(ReporterImpl) }
2865
def reporter
29-
@reporter ||= detect_reporter
66+
reporter_impl
3067
end
3168

3269
# Set the reporter to use (user-friendly API that accepts symbols)
33-
sig { params(reporter_type: T.any(ErrorReporter, Symbol)).returns(ErrorReporter) }
70+
sig { params(reporter_type: T.any(ErrorReporter, Symbol, Handlers::ErrorReporter)).returns(ReporterImpl) }
3471
def reporter=(reporter_type)
35-
@reporter = case reporter_type
72+
@reporter_impl = case reporter_type
3673
when ErrorReporter
3774
reporter_type
3875
when Symbol
39-
case reporter_type
40-
when :sentry then ErrorReporter::Sentry
41-
when :bugsnag then ErrorReporter::Bugsnag
42-
when :rollbar then ErrorReporter::Rollbar
43-
when :honeybadger then ErrorReporter::Honeybadger
44-
when :rails_logger then ErrorReporter::RailsLogger
45-
else
46-
valid_types = ErrorReporter.values.map { |v| ":#{v.serialize}" }.join(", ")
47-
raise ArgumentError, "Unknown reporter type: #{reporter_type}. Valid types are: #{valid_types}"
48-
end
76+
resolve_symbol_reporter(reporter_type)
77+
else
78+
wrap_callable_reporter(reporter_type)
4979
end
5080
end
5181

@@ -69,7 +99,9 @@ def detect_reporter
6999
sig { params(error: StandardError, context: T::Hash[T.untyped, T.untyped]).void }
70100
def report_error(error, context = {})
71101
# Call the appropriate reporter method based on what's available
72-
case reporter
102+
impl = reporter_impl
103+
104+
case impl
73105
when ErrorReporter::Sentry
74106
report_to_sentry(error, context)
75107
when ErrorReporter::Bugsnag
@@ -78,13 +110,43 @@ def report_error(error, context = {})
78110
report_to_rollbar(error, context)
79111
when ErrorReporter::Honeybadger
80112
report_to_honeybadger(error, context)
81-
else
113+
when ErrorReporter::RailsLogger
82114
fallback_logging(error, context)
115+
when CallableReporterWrapper
116+
impl.call(error, context, Source::Internal)
83117
end
84118
end
85119

86120
private
87121

122+
sig { returns(ReporterImpl) }
123+
def reporter_impl
124+
@reporter_impl ||= detect_reporter
125+
end
126+
127+
sig { params(symbol: Symbol).returns(ErrorReporter) }
128+
def resolve_symbol_reporter(symbol)
129+
case symbol
130+
when :sentry then ErrorReporter::Sentry
131+
when :bugsnag then ErrorReporter::Bugsnag
132+
when :rollbar then ErrorReporter::Rollbar
133+
when :honeybadger then ErrorReporter::Honeybadger
134+
when :rails_logger then ErrorReporter::RailsLogger
135+
else
136+
valid_types = ErrorReporter.values.map { |v| ":#{v.serialize}" }.join(", ")
137+
raise ArgumentError, "Unknown reporter type: #{symbol}. Valid types are: #{valid_types}"
138+
end
139+
end
140+
141+
sig { params(callable: T.untyped).returns(CallableReporterWrapper) }
142+
def wrap_callable_reporter(callable)
143+
unless callable.respond_to?(:call)
144+
raise ArgumentError, "Reporter must respond to #call"
145+
end
146+
147+
CallableReporterWrapper.new(callable)
148+
end
149+
88150
# Report to Sentry
89151
sig { params(error: StandardError, context: T::Hash[T.untyped, T.untyped]).void }
90152
def report_to_sentry(error, context = {})

lib/log_struct/shared/shared/serialize_common.rb

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
require_relative "../../enums/log_field"
55
require_relative "../interfaces/common_fields"
6+
require_relative "../../log/shared/merge_additional_data_fields"
67

78
module LogStruct
89
module Log
@@ -19,17 +20,19 @@ def serialize(strict = true)
1920
out = serialize_common(strict)
2021

2122
# Merge event/base fields from the struct-specific hash
22-
field_hash = T.unsafe(self).to_h
23+
kernel_self = T.cast(self, Kernel)
24+
field_hash = T.cast(kernel_self.public_send(:to_h), T::Hash[LogStruct::LogField, T.untyped])
2325
field_hash.each do |log_field, value|
2426
next if value.nil?
25-
key = T.cast(log_field, LogStruct::LogField).serialize
27+
key = log_field.serialize
2628
out[key] = value.is_a?(::Time) ? value.iso8601 : value
2729
end
2830

2931
# Merge any additional_data at top level if available
30-
if T.unsafe(self).respond_to?(:merge_additional_data_fields)
32+
if kernel_self.respond_to?(:merge_additional_data_fields)
3133
# merge_additional_data_fields expects symbol keys
32-
T.unsafe(self).merge_additional_data_fields(out)
34+
merge_target = T.cast(self, LogStruct::Log::Shared::MergeAdditionalDataFields)
35+
merge_target.merge_additional_data_fields(out)
3336
end
3437

3538
out

test/log_struct/formatter_test.rb

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,47 @@ def test_process_values_truncates_large_arrays
175175
assert_equal "... and 10 more items", result.last
176176
end
177177

178+
def test_process_values_does_not_process_beyond_limit_for_large_arrays
179+
exploding_class = Class.new do
180+
attr_reader :index
181+
182+
def initialize(index, raise_on_touch)
183+
@index = index
184+
@raise_on_touch = raise_on_touch
185+
end
186+
187+
def touch!
188+
raise "Should not touch index #{@index}" if @raise_on_touch
189+
end
190+
end
191+
192+
counting_formatter = Class.new(Formatter) do
193+
attr_reader :touched_indices
194+
195+
def initialize
196+
super
197+
@touched_indices = []
198+
end
199+
200+
def process_values(value, recursion_depth: 0)
201+
if value.respond_to?(:touch!)
202+
value.touch!
203+
@touched_indices << value.index if value.respond_to?(:index)
204+
end
205+
206+
super
207+
end
208+
end.new
209+
210+
values = (0...10).map { |i| exploding_class.new(i, false) } +
211+
(10...20).map { |i| exploding_class.new(i, true) }
212+
213+
result = counting_formatter.process_values(values)
214+
215+
assert_equal 11, result.length
216+
assert_equal((0...10).to_a, counting_formatter.touched_indices)
217+
end
218+
178219
def test_process_values_handles_recursive_structures
179220
hash1 = {a: 1}
180221
hash2 = {b: 2, hash1: hash1}

test/log_struct/multi_error_reporter_test.rb

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ def setup
1111
@context = {user_id: 123, action: "test"}
1212

1313
# Reset the reporter before each test
14-
MultiErrorReporter.instance_variable_set(:@reporter, nil)
14+
MultiErrorReporter.instance_variable_set(:@reporter_impl, nil)
1515

1616
# Stub stdout to capture output
1717
@original_stdout = $stdout
@@ -210,6 +210,55 @@ def test_report_error_with_no_service
210210
end
211211
end
212212

213+
def test_reporter_accepts_callable
214+
calls = []
215+
custom_reporter = lambda do |error, context, source|
216+
calls << {
217+
error: error,
218+
context: context,
219+
source: source
220+
}
221+
end
222+
223+
MultiErrorReporter.reporter = custom_reporter
224+
225+
wrapper = MultiErrorReporter.reporter
226+
227+
assert_respond_to wrapper, :original
228+
assert_same custom_reporter, wrapper.original
229+
230+
MultiErrorReporter.report_error(@exception, @context)
231+
232+
assert_equal 1, calls.size
233+
payload = calls.first
234+
235+
assert_equal @exception, payload[:error]
236+
assert_equal @context, payload[:context]
237+
assert_equal Source::Internal, payload[:source]
238+
end
239+
240+
def test_callable_reporter_supports_lower_arity
241+
calls = []
242+
custom = ->(error, context) { calls << [error, context] }
243+
244+
MultiErrorReporter.reporter = custom
245+
wrapper = MultiErrorReporter.reporter
246+
247+
assert_kind_of Proc, wrapper.original
248+
249+
MultiErrorReporter.report_error(@exception, @context)
250+
251+
assert_equal [[@exception, @context]], calls
252+
end
253+
254+
def test_setting_invalid_reporter_raises
255+
error = assert_raises(TypeError) do
256+
MultiErrorReporter.reporter = T.unsafe(Object.new)
257+
end
258+
259+
assert_match(/Expected type/, error.message)
260+
end
261+
213262
private
214263

215264
def setup_mock_constants

0 commit comments

Comments
 (0)